From 3c53f44f0b91e06146df0b7c3a9d46f945d8a86b Mon Sep 17 00:00:00 2001 From: Pierre Poupin Date: Thu, 21 Nov 2024 11:46:50 +0100 Subject: [PATCH] perf: remove unnecessary state changes for isActive if not used (#161) --- .../components/FocusableView.tsx | 40 ++++------- .../spatial-navigation/components/Node.tsx | 70 ++++++++++++------- 2 files changed, 58 insertions(+), 52 deletions(-) diff --git a/packages/lib/src/spatial-navigation/components/FocusableView.tsx b/packages/lib/src/spatial-navigation/components/FocusableView.tsx index cfb0ecc7..3287acbf 100644 --- a/packages/lib/src/spatial-navigation/components/FocusableView.tsx +++ b/packages/lib/src/spatial-navigation/components/FocusableView.tsx @@ -1,4 +1,8 @@ -import { SpatialNavigationNode, SpatialNavigationNodeDefaultProps } from './Node'; +import { + FocusableNodeState, + SpatialNavigationNode, + SpatialNavigationNodeDefaultProps, +} from './Node'; import { Platform, View, ViewStyle, ViewProps } from 'react-native'; import { forwardRef, useImperativeHandle, useMemo, useRef } from 'react'; import { SpatialNavigationNodeRef } from '../types/SpatialNavigationNodeRef'; @@ -7,18 +11,7 @@ import { useSpatialNavigatorFocusableAccessibilityProps } from '../hooks/useSpat type FocusableViewProps = { style?: ViewStyle; - children: - | React.ReactElement - | ((props: { - /** Returns whether the root is focused or not. */ - isFocused: boolean; - /** Returns whether the root is active or not. An active node is active if one of its children is focused. */ - isActive: boolean; - /** Returns whether the root is active or not. - * This is very handy if you want to hide the focus on your page elements when - * the side-menu is focused (since it is a different root navigator) */ - isRootActive: boolean; - }) => React.ReactElement); + children: React.ReactElement | ((props: FocusableNodeState) => React.ReactElement); viewProps?: ViewProps & { onMouseEnter?: () => void; }; @@ -58,14 +51,12 @@ export const SpatialNavigationFocusableView = forwardRef - {({ isFocused, isActive, isRootActive }) => ( + {(nodeState) => ( {children} @@ -86,15 +77,16 @@ type InnerFocusableViewProps = FocusableViewProps & { onMouseEnter?: undefined; onClick?: undefined; }; - isActive: boolean; - isFocused: boolean; - isRootActive: boolean; + nodeState: FocusableNodeState; }; const InnerFocusableView = forwardRef( - ({ viewProps, webProps, children, isActive, isFocused, isRootActive, style }, ref) => { + ({ viewProps, webProps, children, nodeState, style }, ref) => { const accessibilityProps = useSpatialNavigatorFocusableAccessibilityProps(); - const accessibilityState = useMemo(() => ({ selected: isFocused }), [isFocused]); + const accessibilityState = useMemo( + () => ({ selected: nodeState.isFocused }), + [nodeState.isFocused], + ); return ( ( {...viewProps} {...webProps} > - {typeof children === 'function' - ? children({ isFocused, isActive, isRootActive }) - : children} + {typeof children === 'function' ? children(nodeState) : children} ); }, diff --git a/packages/lib/src/spatial-navigation/components/Node.tsx b/packages/lib/src/spatial-navigation/components/Node.tsx index cda4a17a..4c372246 100644 --- a/packages/lib/src/spatial-navigation/components/Node.tsx +++ b/packages/lib/src/spatial-navigation/components/Node.tsx @@ -10,31 +10,27 @@ import { NodeIndexRange } from '@bam.tech/lrud'; import { SpatialNavigationNodeRef } from '../types/SpatialNavigationNodeRef'; import { useIsRootActive } from '../context/IsRootActiveContext'; +type NonFocusableNodeState = { + /** Returns whether the root is active or not. An active node is active if one of its children is focused. */ + isActive: boolean; + /** Returns whether the root is active or not. + * This is very handy if you want to hide the focus on your page elements when + * the side-menu is focused (since it is a different root navigator) */ + isRootActive: boolean; +}; + +export type FocusableNodeState = NonFocusableNodeState & { + /** Returns whether the root is focused or not. */ + isFocused: boolean; +}; + type FocusableProps = { isFocusable: true; - children: (props: { - /** Returns whether the root is focused or not. */ - isFocused: boolean; - /** Returns whether the root is active or not. An active node is active if one of its children is focused. */ - isActive: boolean; - /** Returns whether the root is active or not. - * This is very handy if you want to hide the focus on your page elements when - * the side-menu is focused (since it is a different root navigator) */ - isRootActive: boolean; - }) => React.ReactElement; + children: (props: FocusableNodeState) => React.ReactElement; }; type NonFocusableProps = { isFocusable?: false; - children: - | React.ReactElement - | ((props: { - /** Returns whether the root is active or not. An active node is active if one of its children is focused. */ - isActive: boolean; - /** Returns whether the root is active or not. - * This is very handy if you want to hide the focus on your page elements when - * the side-menu is focused (since it is a different root navigator) */ - isRootActive: boolean; - }) => React.ReactElement); + children: React.ReactElement | ((props: NonFocusableNodeState) => React.ReactElement); }; type DefaultProps = { onFocus?: () => void; @@ -166,17 +162,23 @@ export const SpatialNavigationNode = forwardRef const shouldHaveDefaultFocus = useSpatialNavigatorDefaultFocus(); + const accessedPropertiesRef = useRef>(new Set()); + useEffect(() => { spatialNavigator.registerNode(id, { parent: parentId, isFocusable, onBlur: () => { currentOnBlur.current?.(); - setIsFocused(false); + if (accessedPropertiesRef.current.has('isFocused')) { + setIsFocused(false); + } }, onFocus: () => { currentOnFocus.current?.(); - setIsFocused(true); + if (accessedPropertiesRef.current.has('isFocused')) { + setIsFocused(true); + } }, onSelect: () => currentOnSelect.current?.(), onLongSelect: () => currentOnLongSelect.current?.(), @@ -185,11 +187,15 @@ export const SpatialNavigationNode = forwardRef indexRange, onActive: () => { currentOnActive.current?.(); - setIsActive(true); + if (accessedPropertiesRef.current.has('isActive')) { + setIsActive(true); + } }, onInactive: () => { currentOnInactive.current?.(); - setIsActive(false); + if (accessedPropertiesRef.current.has('isActive')) { + setIsActive(false); + } }, }); @@ -203,11 +209,21 @@ export const SpatialNavigationNode = forwardRef } }, [id, isFocusable, shouldHaveDefaultFocus, spatialNavigator]); + // This proxy allows to track whether a property is used or not + // hence allowing to ignore re-renders for unused properties + const proxyObject = new Proxy( + { isFocused, isActive, isRootActive }, + { + get(target, prop: keyof FocusableNodeState) { + accessedPropertiesRef.current.add(prop); + return target[prop]; + }, + }, + ); + return ( - {typeof children === 'function' - ? bindRefToChild(children({ isFocused, isActive, isRootActive })) - : children} + {typeof children === 'function' ? bindRefToChild(children(proxyObject)) : children} ); },