From 26463c8a6ee3814e0166b312bb8df398216604c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= <62747088+kacper-mikolajczak@users.noreply.github.com> Date: Fri, 30 Aug 2024 09:18:13 +0200 Subject: [PATCH] refactor: memoize getStateFromPath and getPathFromState (#12120) **_Problem_** When `getStateFromPath` receives big _linking config_ ([example](https://github.com/Expensify/App/blob/071f11ce1012db7d07eb750d74f58b552eba6144/src/libs/Navigation/linkingConfig/config.ts#L1152)) as an option parameter, it tends to slow down quite drastically. **_Investigations_** The config-related data is created every time`getStateFromPath` is called, resulting in noticeable overhead. Often times, configs provided to `getStateFromPath` are going to be static (e.g. initialised once at the app setup and remained untouched throughout its lifetime) and the data derived from it could also be static. **_Solution_** PR tries to improve `getStateFromPath` helper performance by extracting and caching calculations related to last linking config provided (due to its static nature, keeping reference to the latest value seems an OK heuristic). Here is an example of potential gains for Exfy app and its config: https://github.com/Expensify/App/issues/48150 _**Test plan**_ No manual testing required. _**PR introduces**_ - [x] `getStateFromPath` config data caching - [x] `setTimeout` typings fixes --- packages/core/src/getPathFromState.tsx | 15 +- packages/core/src/getStateFromPath.tsx | 184 ++++++++++++------ .../elements/src/SafeAreaProviderCompat.tsx | 2 +- packages/native-stack/src/utils/debounce.tsx | 2 +- .../native/src/createStaticNavigation.tsx | 64 +++--- .../react-native-tab-view/src/SceneView.tsx | 2 +- packages/stack/src/utils/throttle.tsx | 2 +- 7 files changed, 175 insertions(+), 96 deletions(-) diff --git a/packages/core/src/getPathFromState.tsx b/packages/core/src/getPathFromState.tsx index 85a9004686..0bc7a27a52 100644 --- a/packages/core/src/getPathFromState.tsx +++ b/packages/core/src/getPathFromState.tsx @@ -37,6 +37,11 @@ const getActiveRoute = (state: State): { name: string; params?: object } => { return route; }; +let cachedNormalizedConfigs: [ + PathConfigMap<{}> | undefined, + Record, +] = [undefined, {}]; + /** * Utility to serialize a navigation state object to a path string. * @@ -81,9 +86,13 @@ export function getPathFromState( } // Create a normalized configs object which will be easier to use - const configs: Record = options?.screens - ? createNormalizedConfigs(options?.screens) - : {}; + if (cachedNormalizedConfigs[0] !== options?.screens) { + cachedNormalizedConfigs = [ + options?.screens, + options?.screens ? createNormalizedConfigs(options.screens) : {}, + ]; + } + const configs: Record = cachedNormalizedConfigs[1]; let path = '/'; let current: State | undefined = state; diff --git a/packages/core/src/getStateFromPath.tsx b/packages/core/src/getStateFromPath.tsx index ca2db2f4e8..8611bf065c 100644 --- a/packages/core/src/getStateFromPath.tsx +++ b/packages/core/src/getStateFromPath.tsx @@ -42,6 +42,12 @@ type ParsedRoute = { params?: Record | undefined; }; +type ConfigResources = { + initialRoutes: InitialRouteConfig[]; + configs: RouteConfig[]; + configWithRegexes: RouteConfig[]; +}; + /** * Utility to parse a path string to initial state object accepted by the container. * This is useful for deep linking when we need to handle the incoming URL. @@ -67,18 +73,8 @@ export function getStateFromPath( path: string, options?: Options ): ResultState | undefined { - if (options) { - validatePathConfig(options); - } - - const initialRoutes: InitialRouteConfig[] = []; - - if (options?.initialRouteName) { - initialRoutes.push({ - initialRouteName: options.initialRouteName, - parentScreens: [], - }); - } + const { initialRoutes, configs, configWithRegexes } = + getConfigResources(options); const screens = options?.screens; @@ -122,8 +118,111 @@ export function getStateFromPath( return undefined; } + if (remaining === '/') { + // We need to add special handling of empty path so navigation to empty path also works + // When handling empty path, we should only look at the root level config + const match = configs.find( + (config) => + config.path === '' && + config.routeNames.every( + // Make sure that none of the parent configs have a non-empty path defined + (name) => !configs.find((c) => c.screen === name)?.path + ) + ); + + if (match) { + return createNestedStateObject( + path, + match.routeNames.map((name) => ({ name })), + initialRoutes, + configs + ); + } + + return undefined; + } + + let result: PartialState | undefined; + let current: PartialState | undefined; + + // We match the whole path against the regex instead of segments + // This makes sure matches such as wildcard will catch any unmatched routes, even if nested + const { routes, remainingPath } = matchAgainstConfigs( + remaining, + configWithRegexes + ); + + if (routes !== undefined) { + // This will always be empty if full path matched + current = createNestedStateObject(path, routes, initialRoutes, configs); + remaining = remainingPath; + result = current; + } + + if (current == null || result == null) { + return undefined; + } + + return result; +} + +/** + * Reference to the last used config resources. This is used to avoid recomputing the config resources when the options are the same. + */ +let cachedConfigResources: [Options<{}> | undefined, ConfigResources] = [ + undefined, + prepareConfigResources(), +]; + +function getConfigResources( + options: Options | undefined +) { + if (cachedConfigResources[0] !== options) { + cachedConfigResources = [options, prepareConfigResources(options)]; + } + + return cachedConfigResources[1]; +} + +function prepareConfigResources(options?: Options<{}>) { + if (options) { + validatePathConfig(options); + } + + const initialRoutes = getInitialRoutes(options); + + const configs = getNormalizedConfigs(initialRoutes, options?.screens); + + checkForDuplicatedConfigs(configs); + + const configWithRegexes = getConfigsWithRegexes(configs); + + return { + initialRoutes, + configs, + configWithRegexes, + }; +} + +function getInitialRoutes(options?: Options<{}>) { + const initialRoutes: InitialRouteConfig[] = []; + + if (options?.initialRouteName) { + initialRoutes.push({ + initialRouteName: options.initialRouteName, + parentScreens: [], + }); + } + + return initialRoutes; +} + +function getNormalizedConfigs( + initialRoutes: InitialRouteConfig[], + screens: PathConfigMap = {} +) { // Create a normalized configs array which will be easier to use - const configs = ([] as RouteConfig[]) + return ([] as RouteConfig[]) .concat( ...Object.keys(screens).map((key) => createNormalizedConfigs( @@ -185,7 +284,9 @@ export function getStateFromPath( } return bParts.length - aParts.length; }); +} +function checkForDuplicatedConfigs(configs: RouteConfig[]) { // Check for duplicate patterns in the config configs.reduce>((acc, config) => { if (acc[config.pattern]) { @@ -214,57 +315,14 @@ export function getStateFromPath( [config.pattern]: config, }); }, {}); +} - if (remaining === '/') { - // We need to add special handling of empty path so navigation to empty path also works - // When handling empty path, we should only look at the root level config - const match = configs.find( - (config) => - config.path === '' && - config.routeNames.every( - // Make sure that none of the parent configs have a non-empty path defined - (name) => !configs.find((c) => c.screen === name)?.path - ) - ); - - if (match) { - return createNestedStateObject( - path, - match.routeNames.map((name) => ({ name })), - initialRoutes, - configs - ); - } - - return undefined; - } - - let result: PartialState | undefined; - let current: PartialState | undefined; - - // We match the whole path against the regex instead of segments - // This makes sure matches such as wildcard will catch any unmatched routes, even if nested - const { routes, remainingPath } = matchAgainstConfigs( - remaining, - configs.map((c) => ({ - ...c, - // Add `$` to the regex to make sure it matches till end of the path and not just beginning - regex: c.regex ? new RegExp(c.regex.source + '$') : undefined, - })) - ); - - if (routes !== undefined) { - // This will always be empty if full path matched - current = createNestedStateObject(path, routes, initialRoutes, configs); - remaining = remainingPath; - result = current; - } - - if (current == null || result == null) { - return undefined; - } - - return result; +function getConfigsWithRegexes(configs: RouteConfig[]) { + return configs.map((c) => ({ + ...c, + // Add `$` to the regex to make sure it matches till end of the path and not just beginning + regex: c.regex ? new RegExp(c.regex.source + '$') : undefined, + })); } const joinPaths = (...paths: string[]): string => diff --git a/packages/elements/src/SafeAreaProviderCompat.tsx b/packages/elements/src/SafeAreaProviderCompat.tsx index cba24de83a..4b91d3cfe6 100644 --- a/packages/elements/src/SafeAreaProviderCompat.tsx +++ b/packages/elements/src/SafeAreaProviderCompat.tsx @@ -85,7 +85,7 @@ const SafeAreaFrameProvider = ({ height: rect.height, }); - let timeout: NodeJS.Timeout; + let timeout: ReturnType; const observer = new ResizeObserver((entries) => { const entry = entries[0]; diff --git a/packages/native-stack/src/utils/debounce.tsx b/packages/native-stack/src/utils/debounce.tsx index 27db55971b..699674a8ff 100644 --- a/packages/native-stack/src/utils/debounce.tsx +++ b/packages/native-stack/src/utils/debounce.tsx @@ -2,7 +2,7 @@ export function debounce void>( func: T, duration: number ): T { - let timeout: NodeJS.Timeout; + let timeout: ReturnType; return function (this: unknown, ...args) { clearTimeout(timeout); diff --git a/packages/native/src/createStaticNavigation.tsx b/packages/native/src/createStaticNavigation.tsx index 7bacf5ad58..88657584c5 100644 --- a/packages/native/src/createStaticNavigation.tsx +++ b/packages/native/src/createStaticNavigation.tsx @@ -51,19 +51,46 @@ export function createStaticNavigation(tree: StaticNavigation) { { linking, ...rest }: Props, ref: React.Ref> ) { - const screens = React.useMemo(() => { - if (tree.config.screens) { - return createPathConfigForStaticNavigation( - tree, - { initialRouteName: linking?.config?.initialRouteName }, - linking?.enabled === 'auto' - ); + const linkingConfig = React.useMemo(() => { + if (!tree.config.screens) return; + + const screens = createPathConfigForStaticNavigation( + tree, + { initialRouteName: linking?.config?.initialRouteName }, + linking?.enabled === 'auto' + ); + + if (!screens) return; + + return { + path: linking?.config?.path, + initialRouteName: linking?.config?.initialRouteName, + screens, + }; + }, [ + linking?.enabled, + linking?.config?.path, + linking?.config?.initialRouteName, + ]); + + const memoizedLinking = React.useMemo(() => { + if (!linking) { + return undefined; } - return undefined; - }, [linking?.config, linking?.enabled]); + const enabled = + typeof linking.enabled === 'boolean' + ? linking.enabled + : linkingConfig?.screens != null; + + return { + ...linking, + enabled, + config: linkingConfig, + }; + }, [linking, linkingConfig]); - if (linking?.enabled === true && screens == null) { + if (linking?.enabled === true && linkingConfig?.screens == null) { throw new Error( 'Linking is enabled but no linking configuration was found for the screens.\n\n' + 'To solve this:\n' + @@ -74,22 +101,7 @@ export function createStaticNavigation(tree: StaticNavigation) { } return ( - + ); diff --git a/packages/react-native-tab-view/src/SceneView.tsx b/packages/react-native-tab-view/src/SceneView.tsx index d31a1b15d3..5eb25d6025 100644 --- a/packages/react-native-tab-view/src/SceneView.tsx +++ b/packages/react-native-tab-view/src/SceneView.tsx @@ -54,7 +54,7 @@ export function SceneView({ }; let unsubscribe: (() => void) | undefined; - let timer: NodeJS.Timeout | undefined; + let timer: ReturnType | undefined; if (lazy && isLoading) { // If lazy mode is enabled, listen to when we enter screens diff --git a/packages/stack/src/utils/throttle.tsx b/packages/stack/src/utils/throttle.tsx index 41d0e59855..071275b669 100644 --- a/packages/stack/src/utils/throttle.tsx +++ b/packages/stack/src/utils/throttle.tsx @@ -2,7 +2,7 @@ export function throttle void>( func: T, duration: number ): T { - let timeout: NodeJS.Timeout | undefined; + let timeout: ReturnType | undefined; return function (this: unknown, ...args) { if (timeout == null) {