Merge pull request Expensify#87870 from software-mansion-labs/collectioneur/dynamic-routes-backward-compatibility

Author: mjasikowski

contributingGuides/NAVIGATION.md

@@ -31,6 +31,7 @@ The navigation in the app is built on top of the `react-navigation` library. To
     - [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)
+    - [Backward compatibility for changed paths](#backward-compatibility-for-changed-paths)
   - [How to remove backTo from URL (Legacy)](#how-to-remove-backto-from-url)
     - [Separating routes for each screen instance](#separating-routes-for-each-screen-instance)
   - [Generating state from a path](#generating-state-from-a-path)
@@ -931,6 +932,46 @@ If you are migrating an existing flow that uses `backTo` (or multiple static rou
 
 For the legacy approach (e.g. separating routes per screen instance) and edge cases like RHP underlay, see [How to remove backTo from URL](#how-to-remove-backto-from-url) and [Separating routes for each screen instance](#separating-routes-for-each-screen-instance).
 
+### Backward compatibility for changed paths
+
+When migrating a static route to a dynamic route, the resulting URL often changes because dynamic routes build the target path on top of the entry screen's URL. Here is a concrete example:
+
+**Static routes** — the entry screen and the target screen have independent, unrelated paths:
+
+| | Path |
+|---|---|
+| Entry screen | `/r/:reportID/details` |
+| Target screen | `/r/:reportID/settings/name` |
+
+The target path `/r/:reportID/settings/name` stands on its own; it does not contain the entry screen's `/details` segment.
+
+**Dynamic routes** — the target URL is formed by appending a suffix to the entry screen's URL:
+
+| | Path |
+|---|---|
+| Entry screen (base path) | `/r/:reportID/details` |
+| Dynamic suffix | `settings/name` |
+| Target screen | `/r/:reportID/details/settings/name` |
+
+Because the suffix `settings/name` is appended to `/r/:reportID/details`, the resulting URL now includes the entry screen's path as a prefix. The old static URL (`/r/:reportID/settings/name`) no longer matches the new dynamic one (`/r/:reportID/details/settings/name`).
+
+If the new path does not match the old one, you **must** add a redirect mapping to [`src/libs/Navigation/linkingConfig/OldRoutes.ts`](../src/libs/Navigation/linkingConfig/OldRoutes.ts) so that bookmarks, shared links, and browser history still work.
+
+Each entry in `OldRoutes.ts` maps an old path pattern to the new path. Use `*` as a wildcard for dynamic segments and `$1`, `$2`, etc. in the replacement to reference captured segments:
+
+```ts
+const oldRoutes: Record<string, string> = {
+    '/r/*/settings/name': '/r/$1/details/settings/name',
+    '/workspaces/*/accounting/*/card-reconciliation/account': '/workspaces/$1/accounting/$2/card-reconciliation/account-reconciliation-settings',
+};
+```
+
+- A `*` in the **middle** of a pattern matches a single path segment (everything except `/`).
+- A `*` at the **end** of a pattern matches the entire remaining path (including `/`).
+- `$1`, `$2`, ... in the replacement string correspond to the captured wildcards in order.
+
+After adding a mapping, add test cases in [`tests/navigation/getMatchingNewRouteTest.ts`](../tests/navigation/getMatchingNewRouteTest.ts) to verify that the old path redirects to the new one and that query parameters are preserved.
+
 ## How to remove backTo from URL
 
 **Preferred approach:** For new flows where a screen is reused across multiple entry points, use [Dynamic Routes](#dynamic-routes) and the [Migrating from backTo to Dynamic Routes](#migrating-from-backto-to-dynamic-routes) guide instead of `backTo` or multiple static routes.

src/ROUTES.ts

@@ -1020,12 +1020,6 @@ const ROUTES = {
         // eslint-disable-next-line no-restricted-syntax -- Legacy route generation
         getRoute: (reportID: string, backTo?: string) => getUrlWithBackToParam(`r/${reportID}/settings` as const, backTo),
     },
-    REPORT_SETTINGS_NAME: {
-        route: 'r/:reportID/settings/name',
-
-        // eslint-disable-next-line no-restricted-syntax -- Legacy route generation
-        getRoute: (reportID: string, backTo?: string) => getUrlWithBackToParam(`r/${reportID}/settings/name` as const, backTo),
-    },
     REPORT_CHANGE_APPROVER: {
         route: 'r/:reportID/change-approver',
 

src/libs/Navigation/helpers/getAdaptedStateFromPath.ts

@@ -17,7 +17,6 @@ import isDynamicRouteScreen from './dynamicRoutesUtils/isDynamicRouteScreen';
 import findFocusedRouteWithOnyxTabGuard from './findFocusedRouteWithOnyxTabGuard';
 import getMatchingNewRoute from './getMatchingNewRoute';
 import getParamsFromRoute from './getParamsFromRoute';
-import getRedirectedPath from './getRedirectedPath';
 import getStateFromPath from './getStateFromPath';
 import {isFullScreenName} from './isNavigatorName';
 import normalizePath from './normalizePath';
@@ -346,7 +345,6 @@ function getAdaptedState(state: PartialState<NavigationState<RootNavigatorParamL
 // We keep `options` in the signature for `linkingConfig` compatibility with react-navigation.
 const getAdaptedStateFromPath: GetAdaptedStateFromPath = (path, options, shouldReplacePathInNestedState = true) => {
     let normalizedPath = !path.startsWith('/') ? `/${path}` : path;
-    normalizedPath = getRedirectedPath(normalizedPath);
     normalizedPath = getMatchingNewRoute(normalizedPath) ?? normalizedPath;
 
     // Bing search results still link to /signin when searching for “Expensify”, but the /signin route no longer exists in our repo, so we redirect it to the home page to avoid showing a Not Found page.

src/libs/Navigation/helpers/getMatchingNewRoute.ts

@@ -1,33 +1,69 @@
 import oldRoutes from '@navigation/linkingConfig/OldRoutes';
 
+const escapeRegExp = (str: string) => str.replaceAll(/[.*+?^${}()|[\]\\]/g, '\\$&');
+
+/**
+ * Converts an OldRoutes pattern string into a RegExp.
+ *
+ * A trailing `*` (last in the pattern) matches any remaining characters (multi-segment).
+ * A non-trailing `*` matches exactly one path segment.
+ *
+ * Captured groups can be referenced in the replacement via `$1`, `$2`, etc.
+ *
+ * @private - Internal helper. Do not export or use outside this file.
+ */
+function patternToRegex(pattern: string): RegExp {
+    const parts = pattern.split('*');
+    let regexStr = '^';
+
+    for (let i = 0; i < parts.length; i++) {
+        regexStr += escapeRegExp(parts.at(i) ?? '');
+        if (i < parts.length - 1) {
+            const isTrailing = i === parts.length - 2 && pattern.endsWith('*');
+            regexStr += isTrailing ? '(.*)' : '([^/]+)';
+        }
+    }
+
+    regexStr += '(?=[?#]|$)';
+
+    return new RegExp(regexStr);
+}
+
 /**
  * Maps an old route path to its corresponding new route based on the `oldRoutes` map.
  * It finds the best matching pattern (with wildcard `*` support) and replaces the matched
  * part of the path with the new route value.
  *
+ * Wildcards:
+ * - Trailing `*` matches any remaining path (including `/`).
+ * - Non-trailing `*` matches exactly one path segment.
+ * - Use `$1`, `$2`, … in the replacement string to reference captured wildcards.
+ *
  * @param path - The input URL path to match and transform.
  * @returns The new route path if a match is found, otherwise `undefined`.
  *
  * Related issue: https://github.com/Expensify/App/issues/64968
  */
 function getMatchingNewRoute(path: string) {
-    let bestMatch;
+    let bestMatch: string | undefined;
+    let bestRegex: RegExp | undefined;
     let maxLength = -1;
 
     for (const pattern of Object.keys(oldRoutes)) {
-        const regexStr = `^${pattern.replace('*', '.*')}`;
-        const regex = new RegExp(regexStr);
+        const regex = patternToRegex(pattern);
 
         if (regex.test(path) && pattern.length > maxLength) {
             bestMatch = pattern;
+            bestRegex = regex;
             maxLength = pattern.length;
         }
     }
-    if (!bestMatch) {
-        return bestMatch;
+
+    if (!bestMatch || !bestRegex) {
+        return undefined;
     }
 
-    const finalRegexp = bestMatch?.replace('*', '') ?? '';
-    return path.replace(finalRegexp, oldRoutes[bestMatch]);
+    return path.replace(bestRegex, oldRoutes[bestMatch]);
 }
+
 export default getMatchingNewRoute;

src/libs/Navigation/helpers/getRedirectedPath.ts

@@ -11,16 +11,14 @@ import getPathWithoutDynamicSuffix from './dynamicRoutesUtils/getPathWithoutDyna
 import getStateForDynamicRoute from './dynamicRoutesUtils/getStateForDynamicRoute';
 import findFocusedRouteWithOnyxTabGuard from './findFocusedRouteWithOnyxTabGuard';
 import getMatchingNewRoute from './getMatchingNewRoute';
-import getRedirectedPath from './getRedirectedPath';
 
 /**
  * @param path - The path to parse
  * @returns - It's possible that there is no navigation action for the given path
  */
 function getStateFromPath(path: Route): PartialState<NavigationState> {
     const normalizedPath = !path.startsWith('/') ? `/${path}` : path;
-    const redirectedPath = getRedirectedPath(normalizedPath);
-    const normalizedPathAfterRedirection = getMatchingNewRoute(redirectedPath) ?? redirectedPath;
+    const normalizedPathAfterRedirection = getMatchingNewRoute(normalizedPath) ?? normalizedPath;
 
     const suffixMatch = findMatchingDynamicSuffix(normalizedPathAfterRedirection);
     if (suffixMatch) {

src/libs/Navigation/helpers/getStateFromPath.ts

@@ -1,8 +1,13 @@
 const oldRoutes: Record<string, string> = {
-    // eslint-disable-next-line @typescript-eslint/naming-convention
-    '/settings/workspaces/*': '/workspaces/',
-    // eslint-disable-next-line @typescript-eslint/naming-convention
+    /* eslint-disable @typescript-eslint/naming-convention */
+    '/settings/workspaces/*': '/workspaces/$1',
     '/settings/workspaces': '/workspaces',
+    '/r/*/settings/name': '/r/$1/details/settings/name',
+    '/workspaces/*/overview/address': '/workspaces/$1/overview/workspace-address',
+    '/workspaces/*/accounting/*/card-reconciliation/account': '/workspaces/$1/accounting/$2/card-reconciliation/account-reconciliation-settings',
+    '/flag/*/*': '/r/$1/flag/$1/$2',
+    '/home-page': '/home',
+    /* eslint-enable @typescript-eslint/naming-convention */
 };
 
 export default oldRoutes;

src/libs/Navigation/linkingConfig/OldRoutes.ts

@@ -22,4 +22,46 @@ describe('getBestMatchingPath', () => {
         expect(getMatchingNewRoute('/anything/anything/')).toBe(undefined);
         expect(getMatchingNewRoute('/anything/anything/anything')).toBe(undefined);
     });
+
+    it('redirects old report settings name path to report details', () => {
+        expect(getMatchingNewRoute('/r/123/settings/name')).toBe('/r/123/details/settings/name');
+    });
+
+    it('preserves query params when redirecting report settings name', () => {
+        expect(getMatchingNewRoute('/r/123/settings/name?backTo=/home')).toBe('/r/123/details/settings/name?backTo=/home');
+    });
+
+    it('redirects old workspace overview address path', () => {
+        expect(getMatchingNewRoute('/workspaces/abc/overview/address')).toBe('/workspaces/abc/overview/workspace-address');
+    });
+
+    it('redirects old card reconciliation account path with two wildcards', () => {
+        expect(getMatchingNewRoute('/workspaces/abc/accounting/xero/card-reconciliation/account')).toBe(
+            '/workspaces/abc/accounting/xero/card-reconciliation/account-reconciliation-settings',
+        );
+        expect(getMatchingNewRoute('/workspaces/abc/accounting/netsuite/card-reconciliation/account')).toBe(
+            '/workspaces/abc/accounting/netsuite/card-reconciliation/account-reconciliation-settings',
+        );
+    });
+
+    it('redirects old flag comment path to report-based dynamic route', () => {
+        expect(getMatchingNewRoute('/flag/123/456')).toBe('/r/123/flag/123/456');
+    });
+
+    it('does not redirect paths that look similar but do not match migrated patterns', () => {
+        expect(getMatchingNewRoute('/r/123/settings/visibility')).toBe(undefined);
+        expect(getMatchingNewRoute('/workspaces/abc/overview/plan')).toBe(undefined);
+        expect(getMatchingNewRoute('/workspaces/abc/accounting/xero/card-reconciliation/settings')).toBe(undefined);
+    });
+
+    it('does not match extensions of exact patterns', () => {
+        expect(getMatchingNewRoute('/home-page2')).toBe(undefined);
+        expect(getMatchingNewRoute('/home-page/extra')).toBe(undefined);
+        expect(getMatchingNewRoute('/r/123/settings/name-extra')).toBe(undefined);
+        expect(getMatchingNewRoute('/workspaces/abc/overview/address/sub')).toBe(undefined);
+    });
+
+    it('preserves fragment when redirecting', () => {
+        expect(getMatchingNewRoute('/home-page?backTo=r/123')).toBe('/home?backTo=r/123');
+    });
 });

tests/navigation/getMatchingNewRouteTest.ts

@@ -52,7 +52,6 @@ jest.mock('@src/ROUTES', () => ({
 }));
 
 jest.mock('@libs/Navigation/helpers/getMatchingNewRoute', () => jest.fn());
-jest.mock('@libs/Navigation/helpers/getRedirectedPath', () => jest.fn((path: string) => path));
 jest.mock('@libs/Navigation/helpers/dynamicRoutesUtils/getStateForDynamicRoute', () => jest.fn());
 
 describe('getStateFromPath', () => {