Skip to content

Commit 24b9fff

Browse files
authored
Merge pull request Expensify#85538 from software-mansion-labs/collectioneur/dynamic-routes-suffix-layering
2 parents 66c8c1d + 777c64d commit 24b9fff

4 files changed

Lines changed: 245 additions & 46 deletions

File tree

contributingGuides/NAVIGATION.md

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ The navigation in the app is built on top of the `react-navigation` library. To
2727
- [Entry screens (access control)](#entry-screens-access-control)
2828
- [Current limitations (work in progress)](#current-limitations-work-in-progress)
2929
- [Multi-segment dynamic routes](#multi-segment-dynamic-routes)
30+
- [Suffix layering (stacking dynamic routes)](#suffix-layering-stacking-dynamic-routes)
3031
- [Dynamic routes with query parameters](#dynamic-routes-with-query-parameters)
3132
- [How to add a new dynamic route](#how-to-add-a-new-dynamic-route)
3233
- [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
701702

702703
Do not use dynamic routes when:
703704
- Your use case falls under the [current limitations](#current-limitations-work-in-progress):
704-
- You need to stack multiple dynamic route suffixes (e.g. `/a/verify-account/another-flow`).
705+
- You need path parameters in dynamic suffixes (e.g. `a/:reportID`).
705706
- The screen has a single, fixed entry and a fixed back destination. In this case, use a normal static route instead.
706707

707708
### Dynamic routes configuration
@@ -732,10 +733,9 @@ When adding or extending a dynamic route, list every screen that should be able
732733

733734
### Current limitations (work in progress)
734735

735-
- **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.
736736
- **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).
737737

738-
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.
738+
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.
739739

740740
### Multi-segment dynamic routes
741741

@@ -751,6 +751,61 @@ For instance, if both `verify-account` and `add-bank-account/verify-account`
751751
are registered, a path ending with `/add-bank-account/verify-account`
752752
will always match the longer, more specific suffix.
753753

754+
### Suffix layering (stacking dynamic routes)
755+
756+
Dynamic route suffixes can be stacked on top of each other,
757+
producing URLs like `/base-path/suffix-a/suffix-b`.
758+
Each suffix in the chain is resolved recursively: the parser strips the outermost suffix first,
759+
resolves the remaining path (which may itself contain another dynamic suffix),
760+
and repeats until it reaches a static base path.
761+
762+
For example, given the path `/settings/wallet/verify-account/country`:
763+
764+
1. The outermost suffix `country` is identified and stripped, leaving `/settings/wallet/verify-account`.
765+
2. `/settings/wallet/verify-account` still contains a dynamic suffix `verify-account`, which is stripped to get `/settings/wallet`.
766+
3. `/settings/wallet` is a static path - standard React Navigation parsing returns the base state.
767+
4. The parser walks back up: it checks that the focused screen of `/settings/wallet` is listed in `VERIFY_ACCOUNT.entryScreens`.
768+
5. Then it checks that the focused screen of the resolved `/settings/wallet/verify-account` state is listed in `COUNTRY.entryScreens`.
769+
6. If all authorization checks pass, the final navigation state is built for the full path.
770+
771+
#### Authorization per layer
772+
773+
Each suffix independently validates access via its own `entryScreens` array.
774+
The focused screen resolved from the layer directly beneath must be listed
775+
in the current suffix's `entryScreens`. If any layer fails authorization,
776+
the path falls back to standard React Navigation parsing and a warning is logged.
777+
778+
#### Configuration example
779+
780+
```ts
781+
DYNAMIC_ROUTES: {
782+
VERIFY_ACCOUNT: {
783+
path: 'verify-account',
784+
entryScreens: [SCREENS.SETTINGS.WALLET.ROOT, SCREENS.TRAVEL.MY_TRIPS],
785+
},
786+
ADDRESS_COUNTRY: {
787+
path: 'country',
788+
entryScreens: [SCREENS.SETTINGS.DYNAMIC_VERIFY_ACCOUNT],
789+
getRoute: (country = '') => `country${country ? `?country=${country}` : ''}`,
790+
queryParams: ['country'],
791+
},
792+
},
793+
```
794+
795+
With this configuration, `country` can be opened on top of `verify-account`
796+
because `DYNAMIC_VERIFY_ACCOUNT` is listed in `ADDRESS_COUNTRY.entryScreens`.
797+
Back navigation removes one suffix at a time:
798+
`/settings/wallet/verify-account/country``/settings/wallet/verify-account``/settings/wallet`.
799+
800+
#### Multi-segment suffixes in layered paths
801+
802+
Suffix layering works with multi-segment suffixes as well.
803+
For example, if `deep/verify-account` and `country` are both registered,
804+
the path `/settings/wallet/deep/verify-account/country` will first strip `country`,
805+
then strip `deep/verify-account`, and resolve `/settings/wallet` as the base.
806+
The matching algorithm always tests the longest candidate suffix first,
807+
so overlapping registrations are resolved deterministically.
808+
754809
### Dynamic routes with query parameters
755810

756811
Dynamic route suffixes can carry query parameters

src/libs/Navigation/Navigation.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {CommonActions, StackActions} from '@react-navigation/native';
44
import {Str} from 'expensify-common';
55
// eslint-disable-next-line you-dont-need-lodash-underscore/omit
66
import omit from 'lodash/omit';
7+
import {nanoid} from 'nanoid/non-secure';
78
import {Dimensions} from 'react-native';
89
import type {OnyxEntry} from 'react-native-onyx';
910
import Onyx from 'react-native-onyx';
@@ -26,6 +27,7 @@ import SCREENS, {PROTECTED_SCREENS} from '@src/SCREENS';
2627
import type {Account, SidePanel} from '@src/types/onyx';
2728
import getInitialSplitNavigatorState from './AppNavigator/createSplitNavigator/getInitialSplitNavigatorState';
2829
import originalCloseRHPFlow from './helpers/closeRHPFlow';
30+
import findMatchingDynamicSuffix from './helpers/dynamicRoutesUtils/findMatchingDynamicSuffix';
2931
import getPathFromState from './helpers/getPathFromState';
3032
import getStateFromPath from './helpers/getStateFromPath';
3133
import getTopmostReportParams from './helpers/getTopmostReportParams';
@@ -457,6 +459,22 @@ function goUp(backToRoute: Route, options?: GoBackOptions) {
457459

458460
// 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
459461
if (indexOfBackToRoute === -1 || (isRootNavigatorState(targetState) && distanceToPop > 1)) {
462+
const actionPayload = minimalAction.payload as NavigationRoute;
463+
464+
// StackRouter's REPLACE drops `path`, use a targeted RESET for dynamic routes to preserve it.
465+
if (actionPayload?.path && findMatchingDynamicSuffix(backToRoute)) {
466+
const routes = targetState.routes.with(targetState.index ?? targetState.routes.length - 1, {
467+
key: `${actionPayload.name}-${nanoid()}`,
468+
name: actionPayload.name,
469+
params: actionPayload.params,
470+
path: actionPayload.path,
471+
});
472+
473+
const resetAction = {type: CONST.NAVIGATION_ACTIONS.RESET, payload: {index: targetState.index, routes}, target: targetState.key} as NavigationAction;
474+
navigationRef.current.dispatch(resetAction);
475+
return;
476+
}
477+
460478
const replaceAction = {...minimalAction, type: CONST.NAVIGATION.ACTION_TYPE.REPLACE} as NavigationAction;
461479
navigationRef.current.dispatch(replaceAction);
462480
return;

tests/navigation/getMatchingFullScreenRouteTests.ts

Lines changed: 60 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@ jest.mock('@libs/Navigation/linkingConfig/RELATIONS', () => {
3434

3535
jest.mock('@src/ROUTES', () => ({
3636
DYNAMIC_ROUTES: {
37-
VERIFY_ACCOUNT: {path: 'verify-account', entryScreens: []},
38-
CUSTOM_FLOW: {path: 'custom-flow', entryScreens: []},
37+
SUFFIX_A: {path: 'suffix-a', entryScreens: []},
38+
SUFFIX_B: {path: 'suffix-b', entryScreens: []},
39+
MULTI_SEG: {path: 'deep/suffix-a', entryScreens: []},
3940
},
4041
HOME: 'home',
4142
}));
@@ -48,50 +49,90 @@ describe('getMatchingFullScreenRoute - dynamic suffix', () => {
4849
jest.clearAllMocks();
4950
});
5051

51-
it('should return last route when path has dynamic suffix and base path state has full screen as last route', () => {
52+
it('should return last route when path has a single dynamic suffix and base path state has full screen as last route', () => {
5253
const route = {
5354
name: 'DynamicScreen',
54-
path: '/settings/wallet/verify-account',
55+
path: '/base/suffix-a',
5556
};
5657
const fullScreenRoute = {name: SCREENS.HOME};
5758
const basePathState = {
58-
routes: [{name: 'Settings'}, fullScreenRoute],
59+
routes: [{name: 'BaseScreen'}, fullScreenRoute],
5960
index: 1,
6061
};
6162

62-
mockGetStateFromPath.mockReturnValue(basePathState);
63+
mockGetStateFromPath.mockImplementation((path: string) => (path === '/base' ? basePathState : undefined));
6364

6465
const result = getMatchingFullScreenRoute(route);
6566

66-
expect(mockGetStateFromPath).toHaveBeenCalledWith('/settings/wallet');
67+
expect(mockGetStateFromPath).toHaveBeenCalledWith('/base');
6768
expect(result).toEqual(fullScreenRoute);
6869
});
6970

70-
it('should recursively find full screen route when base path has nested state with non-full-screen last route', () => {
71+
it('should strip the outermost suffix from a layered path before resolving the matching full screen route', () => {
7172
const route = {
7273
name: 'DynamicScreen',
73-
path: '/workspace/123/custom-flow',
74+
path: '/base/suffix-a/suffix-b',
75+
};
76+
const fullScreenRoute = {name: SCREENS.HOME};
77+
const basePathState = {
78+
routes: [{name: 'BaseScreen'}, fullScreenRoute],
79+
index: 1,
80+
};
81+
82+
mockGetStateFromPath.mockImplementation((path: string) => (path === '/base/suffix-a' ? basePathState : undefined));
83+
84+
const result = getMatchingFullScreenRoute(route);
85+
86+
expect(mockGetStateFromPath).toHaveBeenCalledTimes(1);
87+
expect(mockGetStateFromPath).toHaveBeenCalledWith('/base/suffix-a');
88+
expect(result).toEqual(fullScreenRoute);
89+
});
90+
91+
it('should strip the outermost suffix when the inner suffix is multi-segment', () => {
92+
const route = {
93+
name: 'DynamicScreen',
94+
path: '/base/deep/suffix-a/suffix-b',
95+
};
96+
const fullScreenRoute = {name: SCREENS.HOME};
97+
const basePathState = {
98+
routes: [{name: 'BaseScreen'}, fullScreenRoute],
99+
index: 1,
100+
};
101+
102+
mockGetStateFromPath.mockImplementation((path: string) => (path === '/base/deep/suffix-a' ? basePathState : undefined));
103+
104+
const result = getMatchingFullScreenRoute(route);
105+
106+
expect(mockGetStateFromPath).toHaveBeenCalledTimes(1);
107+
expect(mockGetStateFromPath).toHaveBeenCalledWith('/base/deep/suffix-a');
108+
expect(result).toEqual(fullScreenRoute);
109+
});
110+
111+
it('should recursively find full screen route when the stripped base path has nested state with non-full-screen last route', () => {
112+
const route = {
113+
name: 'DynamicScreen',
114+
path: '/base/suffix-a',
74115
};
75116
const nestedFocusedRoute = {name: SCREENS.HOME};
76117
const basePathState = {
77118
routes: [
78119
{
79-
name: 'Workspace',
120+
name: 'BaseNavigator',
80121
state: {
81-
routes: [{name: 'SomeNestedScreen', path: '/workspace/123'}],
122+
routes: [{name: 'SomeNestedScreen', path: '/base'}],
82123
index: 0,
83124
},
84125
},
85126
],
86127
index: 0,
87128
};
88129

89-
mockGetStateFromPath.mockReturnValue(basePathState);
130+
mockGetStateFromPath.mockImplementation((path: string) => (path === '/base' ? basePathState : undefined));
90131
mockFindFocusedRoute.mockReturnValue(nestedFocusedRoute);
91132

92133
const result = getMatchingFullScreenRoute(route);
93134

94-
expect(mockGetStateFromPath).toHaveBeenCalledWith('/workspace/123');
135+
expect(mockGetStateFromPath).toHaveBeenCalledWith('/base');
95136
expect(mockFindFocusedRoute).toHaveBeenCalledWith(basePathState);
96137
expect(result).toBeDefined();
97138
expect(result?.name).toBe(SCREENS.HOME);
@@ -100,18 +141,18 @@ describe('getMatchingFullScreenRoute - dynamic suffix', () => {
100141
it('should return undefined when path has dynamic suffix but base path resolves to NOT_FOUND', () => {
101142
const route = {
102143
name: 'DynamicScreen',
103-
path: '/invalid/base/verify-account',
144+
path: '/invalid/base/suffix-a/suffix-b',
104145
};
105146
const invalidRouteState = {
106-
routes: [{name: SCREENS.NOT_FOUND, path: '/invalid/base'}],
147+
routes: [{name: SCREENS.NOT_FOUND, path: '/invalid/base/suffix-a'}],
107148
index: 0,
108149
};
109150

110-
mockGetStateFromPath.mockReturnValue(invalidRouteState);
151+
mockGetStateFromPath.mockImplementation((path: string) => (path === '/invalid/base/suffix-a' ? invalidRouteState : undefined));
111152

112153
const result = getMatchingFullScreenRoute(route);
113154

114-
expect(mockGetStateFromPath).toHaveBeenCalledWith('/invalid/base');
155+
expect(mockGetStateFromPath).toHaveBeenCalledWith('/invalid/base/suffix-a');
115156
expect(result).toBeUndefined();
116157
});
117158

@@ -130,14 +171,14 @@ describe('getMatchingFullScreenRoute - dynamic suffix', () => {
130171
it('should return undefined when path has dynamic suffix but base path state is undefined', () => {
131172
const route = {
132173
name: 'DynamicScreen',
133-
path: '/broken/path/verify-account',
174+
path: '/broken/path/suffix-a/suffix-b',
134175
};
135176

136177
mockGetStateFromPath.mockReturnValue(undefined);
137178

138179
const result = getMatchingFullScreenRoute(route);
139180

140-
expect(mockGetStateFromPath).toHaveBeenCalledWith('/broken/path');
181+
expect(mockGetStateFromPath).toHaveBeenCalledWith('/broken/path/suffix-a');
141182
expect(result).toBeUndefined();
142183
});
143184
});

0 commit comments

Comments
 (0)