diff --git a/contributingGuides/NAVIGATION.md b/contributingGuides/NAVIGATION.md index 9cee8c3b9f0a9..40fbc3dfcd451 100644 --- a/contributingGuides/NAVIGATION.md +++ b/contributingGuides/NAVIGATION.md @@ -27,6 +27,7 @@ The navigation in the app is built on top of the `react-navigation` library. To - [Entry screens (access control)](#entry-screens-access-control) - [Current limitations (work in progress)](#current-limitations-work-in-progress) - [Multi-segment dynamic routes](#multi-segment-dynamic-routes) + - [Suffix layering (stacking dynamic routes)](#suffix-layering-stacking-dynamic-routes) - [Dynamic routes with query parameters](#dynamic-routes-with-query-parameters) - [How to add a new dynamic route](#how-to-add-a-new-dynamic-route) - [Migrating from backTo to dynamic routes](#migrating-from-backto-to-dynamic-routes) @@ -701,7 +702,7 @@ A dynamic route is a URL suffix (e.g. `verify-account`) that can be appended to Do not use dynamic routes when: - Your use case falls under the [current limitations](#current-limitations-work-in-progress): - - You need to stack multiple dynamic route suffixes (e.g. `/a/verify-account/another-flow`). + - You need path parameters in dynamic suffixes (e.g. `a/:reportID`). - The screen has a single, fixed entry and a fixed back destination. In this case, use a normal static route instead. ### Dynamic routes configuration @@ -732,10 +733,9 @@ When adding or extending a dynamic route, list every screen that should be able ### Current limitations (work in progress) -- **Stacking:** Multiple dynamic route suffixes on top of each other (e.g. `/a/verify-account/another-flow`) are not supported. Only one dynamic suffix per path is allowed. - **Path parameters:** Suffixes must not include path params (e.g. `a/:reportID`). Query parameters are supported - see [Dynamic routes with query parameters](#dynamic-routes-with-query-parameters). -If you try to use dynamic routes for these cases now, you will either fail to navigate to the page at all or end up on a non-existent page, and the navigation will be broken. +If you try to use dynamic routes for this case now, you will either fail to navigate to the page at all or end up on a non-existent page, and the navigation will be broken. ### Multi-segment dynamic routes @@ -751,6 +751,61 @@ For instance, if both `verify-account` and `add-bank-account/verify-account` are registered, a path ending with `/add-bank-account/verify-account` will always match the longer, more specific suffix. +### Suffix layering (stacking dynamic routes) + +Dynamic route suffixes can be stacked on top of each other, +producing URLs like `/base-path/suffix-a/suffix-b`. +Each suffix in the chain is resolved recursively: the parser strips the outermost suffix first, +resolves the remaining path (which may itself contain another dynamic suffix), +and repeats until it reaches a static base path. + +For example, given the path `/settings/wallet/verify-account/country`: + +1. The outermost suffix `country` is identified and stripped, leaving `/settings/wallet/verify-account`. +2. `/settings/wallet/verify-account` still contains a dynamic suffix `verify-account`, which is stripped to get `/settings/wallet`. +3. `/settings/wallet` is a static path - standard React Navigation parsing returns the base state. +4. The parser walks back up: it checks that the focused screen of `/settings/wallet` is listed in `VERIFY_ACCOUNT.entryScreens`. +5. Then it checks that the focused screen of the resolved `/settings/wallet/verify-account` state is listed in `COUNTRY.entryScreens`. +6. If all authorization checks pass, the final navigation state is built for the full path. + +#### Authorization per layer + +Each suffix independently validates access via its own `entryScreens` array. +The focused screen resolved from the layer directly beneath must be listed +in the current suffix's `entryScreens`. If any layer fails authorization, +the path falls back to standard React Navigation parsing and a warning is logged. + +#### Configuration example + +```ts +DYNAMIC_ROUTES: { + VERIFY_ACCOUNT: { + path: 'verify-account', + entryScreens: [SCREENS.SETTINGS.WALLET.ROOT, SCREENS.TRAVEL.MY_TRIPS], + }, + ADDRESS_COUNTRY: { + path: 'country', + entryScreens: [SCREENS.SETTINGS.DYNAMIC_VERIFY_ACCOUNT], + getRoute: (country = '') => `country${country ? `?country=${country}` : ''}`, + queryParams: ['country'], + }, +}, +``` + +With this configuration, `country` can be opened on top of `verify-account` +because `DYNAMIC_VERIFY_ACCOUNT` is listed in `ADDRESS_COUNTRY.entryScreens`. +Back navigation removes one suffix at a time: +`/settings/wallet/verify-account/country` → `/settings/wallet/verify-account` → `/settings/wallet`. + +#### Multi-segment suffixes in layered paths + +Suffix layering works with multi-segment suffixes as well. +For example, if `deep/verify-account` and `country` are both registered, +the path `/settings/wallet/deep/verify-account/country` will first strip `country`, +then strip `deep/verify-account`, and resolve `/settings/wallet` as the base. +The matching algorithm always tests the longest candidate suffix first, +so overlapping registrations are resolved deterministically. + ### Dynamic routes with query parameters Dynamic route suffixes can carry query parameters diff --git a/src/libs/Navigation/Navigation.ts b/src/libs/Navigation/Navigation.ts index c1eec1948b1e5..41db04f1d9e08 100644 --- a/src/libs/Navigation/Navigation.ts +++ b/src/libs/Navigation/Navigation.ts @@ -4,6 +4,7 @@ import {CommonActions, StackActions} from '@react-navigation/native'; import {Str} from 'expensify-common'; // eslint-disable-next-line you-dont-need-lodash-underscore/omit import omit from 'lodash/omit'; +import {nanoid} from 'nanoid/non-secure'; import {Dimensions} from 'react-native'; import type {OnyxEntry} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; @@ -26,6 +27,7 @@ import SCREENS, {PROTECTED_SCREENS} from '@src/SCREENS'; import type {Account, SidePanel} from '@src/types/onyx'; import getInitialSplitNavigatorState from './AppNavigator/createSplitNavigator/getInitialSplitNavigatorState'; import originalCloseRHPFlow from './helpers/closeRHPFlow'; +import findMatchingDynamicSuffix from './helpers/dynamicRoutesUtils/findMatchingDynamicSuffix'; import getPathFromState from './helpers/getPathFromState'; import getStateFromPath from './helpers/getStateFromPath'; import getTopmostReportParams from './helpers/getTopmostReportParams'; @@ -457,6 +459,22 @@ function goUp(backToRoute: Route, options?: GoBackOptions) { // If we need to pop more than one route from rootState, we replace the current route to not lose visited routes from the navigation state if (indexOfBackToRoute === -1 || (isRootNavigatorState(targetState) && distanceToPop > 1)) { + const actionPayload = minimalAction.payload as NavigationRoute; + + // StackRouter's REPLACE drops `path`, use a targeted RESET for dynamic routes to preserve it. + if (actionPayload?.path && findMatchingDynamicSuffix(backToRoute)) { + const routes = targetState.routes.with(targetState.index ?? targetState.routes.length - 1, { + key: `${actionPayload.name}-${nanoid()}`, + name: actionPayload.name, + params: actionPayload.params, + path: actionPayload.path, + }); + + const resetAction = {type: CONST.NAVIGATION_ACTIONS.RESET, payload: {index: targetState.index, routes}, target: targetState.key} as NavigationAction; + navigationRef.current.dispatch(resetAction); + return; + } + const replaceAction = {...minimalAction, type: CONST.NAVIGATION.ACTION_TYPE.REPLACE} as NavigationAction; navigationRef.current.dispatch(replaceAction); return; diff --git a/tests/navigation/getMatchingFullScreenRouteTests.ts b/tests/navigation/getMatchingFullScreenRouteTests.ts index 45213a3a9bc8d..004b47786b400 100644 --- a/tests/navigation/getMatchingFullScreenRouteTests.ts +++ b/tests/navigation/getMatchingFullScreenRouteTests.ts @@ -34,8 +34,9 @@ jest.mock('@libs/Navigation/linkingConfig/RELATIONS', () => { jest.mock('@src/ROUTES', () => ({ DYNAMIC_ROUTES: { - VERIFY_ACCOUNT: {path: 'verify-account', entryScreens: []}, - CUSTOM_FLOW: {path: 'custom-flow', entryScreens: []}, + SUFFIX_A: {path: 'suffix-a', entryScreens: []}, + SUFFIX_B: {path: 'suffix-b', entryScreens: []}, + MULTI_SEG: {path: 'deep/suffix-a', entryScreens: []}, }, HOME: 'home', })); @@ -48,37 +49,77 @@ describe('getMatchingFullScreenRoute - dynamic suffix', () => { jest.clearAllMocks(); }); - it('should return last route when path has dynamic suffix and base path state has full screen as last route', () => { + it('should return last route when path has a single dynamic suffix and base path state has full screen as last route', () => { const route = { name: 'DynamicScreen', - path: '/settings/wallet/verify-account', + path: '/base/suffix-a', }; const fullScreenRoute = {name: SCREENS.HOME}; const basePathState = { - routes: [{name: 'Settings'}, fullScreenRoute], + routes: [{name: 'BaseScreen'}, fullScreenRoute], index: 1, }; - mockGetStateFromPath.mockReturnValue(basePathState); + mockGetStateFromPath.mockImplementation((path: string) => (path === '/base' ? basePathState : undefined)); const result = getMatchingFullScreenRoute(route); - expect(mockGetStateFromPath).toHaveBeenCalledWith('/settings/wallet'); + expect(mockGetStateFromPath).toHaveBeenCalledWith('/base'); expect(result).toEqual(fullScreenRoute); }); - it('should recursively find full screen route when base path has nested state with non-full-screen last route', () => { + it('should strip the outermost suffix from a layered path before resolving the matching full screen route', () => { const route = { name: 'DynamicScreen', - path: '/workspace/123/custom-flow', + path: '/base/suffix-a/suffix-b', + }; + const fullScreenRoute = {name: SCREENS.HOME}; + const basePathState = { + routes: [{name: 'BaseScreen'}, fullScreenRoute], + index: 1, + }; + + mockGetStateFromPath.mockImplementation((path: string) => (path === '/base/suffix-a' ? basePathState : undefined)); + + const result = getMatchingFullScreenRoute(route); + + expect(mockGetStateFromPath).toHaveBeenCalledTimes(1); + expect(mockGetStateFromPath).toHaveBeenCalledWith('/base/suffix-a'); + expect(result).toEqual(fullScreenRoute); + }); + + it('should strip the outermost suffix when the inner suffix is multi-segment', () => { + const route = { + name: 'DynamicScreen', + path: '/base/deep/suffix-a/suffix-b', + }; + const fullScreenRoute = {name: SCREENS.HOME}; + const basePathState = { + routes: [{name: 'BaseScreen'}, fullScreenRoute], + index: 1, + }; + + mockGetStateFromPath.mockImplementation((path: string) => (path === '/base/deep/suffix-a' ? basePathState : undefined)); + + const result = getMatchingFullScreenRoute(route); + + expect(mockGetStateFromPath).toHaveBeenCalledTimes(1); + expect(mockGetStateFromPath).toHaveBeenCalledWith('/base/deep/suffix-a'); + expect(result).toEqual(fullScreenRoute); + }); + + it('should recursively find full screen route when the stripped base path has nested state with non-full-screen last route', () => { + const route = { + name: 'DynamicScreen', + path: '/base/suffix-a', }; const nestedFocusedRoute = {name: SCREENS.HOME}; const basePathState = { routes: [ { - name: 'Workspace', + name: 'BaseNavigator', state: { - routes: [{name: 'SomeNestedScreen', path: '/workspace/123'}], + routes: [{name: 'SomeNestedScreen', path: '/base'}], index: 0, }, }, @@ -86,12 +127,12 @@ describe('getMatchingFullScreenRoute - dynamic suffix', () => { index: 0, }; - mockGetStateFromPath.mockReturnValue(basePathState); + mockGetStateFromPath.mockImplementation((path: string) => (path === '/base' ? basePathState : undefined)); mockFindFocusedRoute.mockReturnValue(nestedFocusedRoute); const result = getMatchingFullScreenRoute(route); - expect(mockGetStateFromPath).toHaveBeenCalledWith('/workspace/123'); + expect(mockGetStateFromPath).toHaveBeenCalledWith('/base'); expect(mockFindFocusedRoute).toHaveBeenCalledWith(basePathState); expect(result).toBeDefined(); expect(result?.name).toBe(SCREENS.HOME); @@ -100,18 +141,18 @@ describe('getMatchingFullScreenRoute - dynamic suffix', () => { it('should return undefined when path has dynamic suffix but base path resolves to NOT_FOUND', () => { const route = { name: 'DynamicScreen', - path: '/invalid/base/verify-account', + path: '/invalid/base/suffix-a/suffix-b', }; const invalidRouteState = { - routes: [{name: SCREENS.NOT_FOUND, path: '/invalid/base'}], + routes: [{name: SCREENS.NOT_FOUND, path: '/invalid/base/suffix-a'}], index: 0, }; - mockGetStateFromPath.mockReturnValue(invalidRouteState); + mockGetStateFromPath.mockImplementation((path: string) => (path === '/invalid/base/suffix-a' ? invalidRouteState : undefined)); const result = getMatchingFullScreenRoute(route); - expect(mockGetStateFromPath).toHaveBeenCalledWith('/invalid/base'); + expect(mockGetStateFromPath).toHaveBeenCalledWith('/invalid/base/suffix-a'); expect(result).toBeUndefined(); }); @@ -130,14 +171,14 @@ describe('getMatchingFullScreenRoute - dynamic suffix', () => { it('should return undefined when path has dynamic suffix but base path state is undefined', () => { const route = { name: 'DynamicScreen', - path: '/broken/path/verify-account', + path: '/broken/path/suffix-a/suffix-b', }; mockGetStateFromPath.mockReturnValue(undefined); const result = getMatchingFullScreenRoute(route); - expect(mockGetStateFromPath).toHaveBeenCalledWith('/broken/path'); + expect(mockGetStateFromPath).toHaveBeenCalledWith('/broken/path/suffix-a'); expect(result).toBeUndefined(); }); }); diff --git a/tests/navigation/getStateFromPathTests.ts b/tests/navigation/getStateFromPathTests.ts index 5808b6f164e4d..39548f069a16f 100644 --- a/tests/navigation/getStateFromPathTests.ts +++ b/tests/navigation/getStateFromPathTests.ts @@ -21,9 +21,25 @@ jest.mock('@libs/Navigation/linkingConfig', () => ({ jest.mock('@src/ROUTES', () => ({ DYNAMIC_ROUTES: { - VERIFY_ACCOUNT: { - path: 'verify-account', - entryScreens: ['Settings', 'Wallet'], + SUFFIX_A: { + path: 'suffix-a', + entryScreens: ['BaseScreen'], + }, + SUFFIX_B: { + path: 'suffix-b', + entryScreens: ['DynamicSuffixAScreen'], + }, + SUFFIX_B_UNAUTHORIZED: { + path: 'suffix-b-unauth', + entryScreens: ['SomeOtherScreen'], + }, + MULTI_SEG: { + path: 'deep/suffix-a', + entryScreens: ['BaseScreen'], + }, + MULTI_SEG_LAYER: { + path: 'suffix-b-from-multi', + entryScreens: ['DynamicMultiSegScreen'], }, }, })); @@ -38,15 +54,48 @@ describe('getStateFromPath', () => { const mockGetStateForDynamicRoute = getStateForDynamicRoute as jest.Mock; const mockLogWarn = jest.spyOn(Log, 'warn'); + const baseRouteState = {routes: [{name: 'BaseScreen'}]}; + const dynamicSuffixAState = {routes: [{name: 'DynamicSuffixAScreen'}]}; + const dynamicSuffixBState = {routes: [{name: 'DynamicSuffixBScreen'}]}; + const dynamicMultiSegState = {routes: [{name: 'DynamicMultiSegScreen'}]}; + const dynamicMultiSegLayerState = {routes: [{name: 'DynamicMultiSegLayerScreen'}]}; + const focusedRouteParams = {baseParam: '123'}; + beforeEach(() => { jest.clearAllMocks(); - mockRNGetStateFromPath.mockReturnValue({}); + mockRNGetStateFromPath.mockReturnValue(baseRouteState); + mockGetStateForDynamicRoute.mockImplementation((_path: string, dynamicRouteKey: string) => { + if (dynamicRouteKey === 'SUFFIX_A') { + return dynamicSuffixAState; + } + if (dynamicRouteKey === 'SUFFIX_B') { + return dynamicSuffixBState; + } + if (dynamicRouteKey === 'MULTI_SEG') { + return dynamicMultiSegState; + } + if (dynamicRouteKey === 'MULTI_SEG_LAYER') { + return dynamicMultiSegLayerState; + } + return {routes: [{name: 'UnknownDynamic'}]}; + }); + mockFindFocusedRoute.mockImplementation((state: unknown) => { + if (state === baseRouteState) { + return {name: 'BaseScreen', params: focusedRouteParams}; + } + if (state === dynamicSuffixAState) { + return {name: 'DynamicSuffixAScreen', params: focusedRouteParams}; + } + if (state === dynamicMultiSegState) { + return {name: 'DynamicMultiSegScreen', params: focusedRouteParams}; + } + return undefined; + }); }); it('should delegate to RN getStateFromPath for standard routes (non-dynamic)', () => { - const path = '/settings/profile'; - - const expectedState = {routes: [{name: 'Settings'}]}; + const path = '/base/profile'; + const expectedState = {routes: [{name: 'BaseProfile'}]}; mockRNGetStateFromPath.mockReturnValue(expectedState); const result = getStateFromPath(path as unknown as Route); @@ -55,34 +104,70 @@ describe('getStateFromPath', () => { }); it('should generate dynamic state when authorized screen is focused', () => { - const fullPath = '/settings/wallet/verify-account'; - const baseRouteState = {routes: [{name: 'Wallet'}]}; - const focusedRouteParams = {walletID: '456'}; - - mockRNGetStateFromPath.mockReturnValue(baseRouteState); - mockFindFocusedRoute.mockReturnValue({name: 'Wallet', params: focusedRouteParams}); - - const expectedDynamicState = {routes: [{name: 'DynamicRoot'}]}; - mockGetStateForDynamicRoute.mockReturnValue(expectedDynamicState); + const fullPath = '/base/suffix-a'; const result = getStateFromPath(fullPath as unknown as Route); - expect(result).toBe(expectedDynamicState); - expect(mockGetStateForDynamicRoute).toHaveBeenCalledWith(fullPath, 'VERIFY_ACCOUNT', focusedRouteParams); + expect(result).toBe(dynamicSuffixAState); + expect(mockGetStateForDynamicRoute).toHaveBeenCalledWith(fullPath, 'SUFFIX_A', focusedRouteParams); }); it('should fallback to standard RN parsing if focused screen is NOT authorized for dynamic route', () => { - const fullPath = '/chat/verify-account'; - - mockRNGetStateFromPath.mockReturnValue({}); - mockFindFocusedRoute.mockReturnValue({name: 'ChatScreen'}); - - const standardState = {routes: [{name: 'Chat', params: {screen: 'verify-account'}}]}; + const fullPath = '/unknown/suffix-b-unauth'; + const standardState = {routes: [{name: 'FallbackRoute'}]}; mockRNGetStateFromPath.mockReturnValue(standardState); + mockFindFocusedRoute.mockReturnValue({name: 'UnknownScreen'}); const result = getStateFromPath(fullPath as unknown as Route); expect(result).toBe(standardState); expect(mockLogWarn).toHaveBeenCalledWith(expect.stringContaining('is not allowed to access dynamic route')); }); + + describe('layered dynamic suffixes', () => { + it('should authorize a layered suffix when the inner dynamic screen is listed in entryScreens', () => { + const fullPath = '/base/suffix-a/suffix-b'; + + const result = getStateFromPath(fullPath as unknown as Route); + + expect(result).toBe(dynamicSuffixBState); + expect(mockGetStateForDynamicRoute).toHaveBeenCalledWith('/base/suffix-a', 'SUFFIX_A', focusedRouteParams); + expect(mockGetStateForDynamicRoute).toHaveBeenCalledWith(fullPath, 'SUFFIX_B', focusedRouteParams); + }); + + it('should fallback to RN parsing when the outer suffix entryScreens does not include the inner dynamic screen', () => { + const fullPath = '/base/suffix-a/suffix-b-unauth'; + const standardState = {routes: [{name: 'FallbackRoute'}]}; + mockRNGetStateFromPath.mockImplementation((path: string) => { + if (path === fullPath) { + return standardState; + } + return baseRouteState; + }); + + const result = getStateFromPath(fullPath as unknown as Route); + + expect(result).toBe(standardState); + expect(mockGetStateForDynamicRoute).toHaveBeenCalledWith('/base/suffix-a', 'SUFFIX_A', focusedRouteParams); + expect(mockLogWarn).toHaveBeenCalledWith(expect.stringContaining('is not allowed to access dynamic route')); + }); + + it('should pass the full layered path including query params to the outer dynamic route builder', () => { + const fullPath = '/base/suffix-a/suffix-b?param=val'; + + getStateFromPath(fullPath as unknown as Route); + + expect(mockGetStateForDynamicRoute).toHaveBeenCalledWith(fullPath, 'SUFFIX_B', focusedRouteParams); + }); + + it('should support a multi-segment inner suffix inside the layered path', () => { + const fullPath = '/base/deep/suffix-a/suffix-b-from-multi'; + + const result = getStateFromPath(fullPath as unknown as Route); + + expect(result).toBe(dynamicMultiSegLayerState); + expect(mockGetStateForDynamicRoute).toHaveBeenCalledWith('/base/deep/suffix-a', 'MULTI_SEG', focusedRouteParams); + expect(mockGetStateForDynamicRoute).toHaveBeenCalledWith(fullPath, 'MULTI_SEG_LAYER', focusedRouteParams); + }); + }); });