Improve empty path and param handling (#4671)

* Overhaul Path handling

* Another test for deep link

src/routers/StackRouter.js

@@ -107,13 +107,7 @@ export default (routeConfigs, stackConfig = {}) => {
   const {
     getPathAndParamsForRoute,
     getActionForPathAndParams,
-  } = createPathParser(
-    childRouters,
-    routeConfigs,
-    stackConfig.paths,
-    initialRouteName,
-    initialRouteParams
-  );
+  } = createPathParser(childRouters, routeConfigs, stackConfig.paths);
 
   return {
     childRouters,

src/routers/SwitchRouter.js

@@ -47,13 +47,7 @@ export default (routeConfigs, config = {}) => {
   const {
     getPathAndParamsForRoute,
     getActionForPathAndParams,
-  } = createPathParser(
-    childRouters,
-    routeConfigs,
-    config.paths,
-    initialRouteName,
-    initialRouteParams
-  );
+  } = createPathParser(childRouters, routeConfigs, config.paths);
 
   if (initialRouteIndex === -1) {
     throw new Error(

src/routers/__tests__/PathHandling-test.js

@@ -13,50 +13,50 @@ beforeEach(() => {
   _TESTING_ONLY_normalize_keys();
 });
 
-const ListScreen = () => <div />;
+const performRouterTest = createTestRouter => {
+  const ListScreen = () => <div />;
 
-const ProfileNavigator = () => <div />;
-ProfileNavigator.router = StackRouter({
-  list: {
-    path: 'list/:id',
-    screen: ListScreen,
-  },
-});
-
-const MainNavigator = () => <div />;
-MainNavigator.router = StackRouter({
-  profile: {
-    path: 'p/:id',
-    screen: ProfileNavigator,
-  },
-});
-
-const LoginScreen = () => <div />;
-
-const AuthNavigator = () => <div />;
-AuthNavigator.router = StackRouter({
-  login: {
-    screen: LoginScreen,
-  },
-});
-
-const BarScreen = () => <div />;
-
-class FooNavigator extends React.Component {
-  static router = StackRouter({
-    bar: {
-      path: 'b/:barThing',
-      screen: BarScreen,
+  const ProfileNavigator = () => <div />;
+  ProfileNavigator.router = StackRouter({
+    list: {
+      path: 'list/:id',
+      screen: ListScreen,
     },
   });
-  render() {
-    return <div />;
+
+  const MainNavigator = () => <div />;
+  MainNavigator.router = StackRouter({
+    profile: {
+      path: 'p/:id',
+      screen: ProfileNavigator,
+    },
+  });
+
+  const LoginScreen = () => <div />;
+
+  const AuthNavigator = () => <div />;
+  AuthNavigator.router = StackRouter({
+    login: {
+      screen: LoginScreen,
+    },
+  });
+
+  const BarScreen = () => <div />;
+
+  class FooNavigator extends React.Component {
+    static router = StackRouter({
+      bar: {
+        path: 'b/:barThing',
+        screen: BarScreen,
+      },
+    });
+    render() {
+      return <div />;
+    }
   }
-}
 
-const PersonScreen = () => <div />;
+  const PersonScreen = () => <div />;
 
-const performRouterTest = createTestRouter => {
   const testRouter = createTestRouter({
     main: {
       screen: MainNavigator,
@@ -78,7 +78,7 @@ const performRouterTest = createTestRouter => {
     },
   });
 
-  test('Handles empty URIs', () => {
+  test('Handles empty URIs with empty action', () => {
     const router = createTestRouter(
       {
         Foo: {
@@ -91,12 +91,8 @@ const performRouterTest = createTestRouter => {
       { initialRouteName: 'Bar', initialRouteParams: { foo: 42 } }
     );
     const action = router.getActionForPathAndParams('');
-    expect(action).toEqual({
-      type: NavigationActions.NAVIGATE,
-      routeName: 'Bar',
-      params: { foo: 42 },
-    });
-    const state = router.getStateForAction(action);
+    expect(action).toEqual(null);
+    const state = router.getStateForAction(action || NavigationActions.init());
     expect(state.routes[state.index]).toEqual(
       expect.objectContaining({
         routeName: 'Bar',
@@ -105,6 +101,244 @@ const performRouterTest = createTestRouter => {
     );
   });
 
+  test('Handles paths with several params', () => {
+    const router = createTestRouter({
+      Person: {
+        path: 'people/:person',
+        screen: () => <div />,
+      },
+      Task: {
+        path: 'people/:person/tasks/:task',
+        screen: () => <div />,
+      },
+      ThingA: {
+        path: 'things/:good',
+        screen: () => <div />,
+      },
+      Thing: {
+        path: 'things/:good/:thing',
+        screen: () => <div />,
+      },
+    });
+    const action = router.getActionForPathAndParams(
+      'people/brent/tasks/everything'
+    );
+    expect(action).toEqual({
+      type: NavigationActions.NAVIGATE,
+      routeName: 'Task',
+      params: { person: 'brent', task: 'everything' },
+    });
+
+    const action1 = router.getActionForPathAndParams('people/lucy');
+    expect(action1).toEqual({
+      type: NavigationActions.NAVIGATE,
+      routeName: 'Person',
+      params: { person: 'lucy' },
+    });
+
+    const action2 = router.getActionForPathAndParams('things/foo/bar');
+    expect(action2).toEqual({
+      type: NavigationActions.NAVIGATE,
+      routeName: 'Thing',
+      params: { good: 'foo', thing: 'bar' },
+    });
+
+    const action3 = router.getActionForPathAndParams('things/foo');
+    expect(action3).toEqual({
+      type: NavigationActions.NAVIGATE,
+      routeName: 'ThingA',
+      params: { good: 'foo' },
+    });
+  });
+
+  test('Handles empty path configuration', () => {
+    const router = createTestRouter({
+      Foo: {
+        screen: () => <div />,
+      },
+      Bar: {
+        screen: () => <div />,
+        path: '',
+      },
+    });
+    const action = router.getActionForPathAndParams('');
+    expect(action).toEqual({
+      type: NavigationActions.NAVIGATE,
+      routeName: 'Bar',
+      params: {},
+    });
+  });
+
+  test('Handles wildcard path configuration', () => {
+    const router = createTestRouter({
+      Foo: {
+        screen: () => <div />,
+      },
+      Bar: {
+        screen: () => <div />,
+        path: ':something',
+      },
+    });
+    const action = router.getActionForPathAndParams('');
+    expect(action).toEqual(null);
+
+    const action1 = router.getActionForPathAndParams('Foo');
+    expect(action1).toEqual({
+      type: NavigationActions.NAVIGATE,
+      routeName: 'Foo',
+      params: {},
+    });
+    const action2 = router.getActionForPathAndParams('asdf');
+    expect(action2).toEqual({
+      type: NavigationActions.NAVIGATE,
+      routeName: 'Bar',
+      params: { something: 'asdf' },
+    });
+  });
+
+  test('Null path behavior', () => {
+    const ScreenA = () => <div />;
+    const router = createTestRouter({
+      Bar: {
+        screen: ScreenA,
+      },
+      Foo: {
+        path: null,
+        screen: ScreenA,
+      },
+      Baz: {
+        path: '',
+        screen: ScreenA,
+      },
+    });
+    const action0 = router.getActionForPathAndParams('test/random', {});
+    expect(action0).toBe(null);
+
+    const action1 = router.getActionForPathAndParams('', {});
+    expect(action1.routeName).toBe('Baz');
+    const state1 = router.getStateForAction(action1);
+    expect(state1.routes[state1.index].routeName).toBe('Baz');
+  });
+
+  test('Multiple null path sub routers path behavior', () => {
+    const ScreenA = () => <div />;
+    const ScreenB = () => <div />;
+    ScreenB.router = createTestRouter({
+      Foo: ScreenA,
+    });
+    const ScreenC = () => <div />;
+    ScreenC.router = createTestRouter({
+      Bar: {
+        path: 'bar/:id',
+        screen: ScreenA,
+      },
+      Empty: {
+        path: '',
+        screen: ScreenA,
+      },
+    });
+    const router = createTestRouter({
+      A: {
+        screen: ScreenA,
+      },
+      B: {
+        path: null,
+        screen: ScreenB,
+      },
+      C: {
+        path: null,
+        screen: ScreenC,
+      },
+    });
+    const action0 = router.getActionForPathAndParams('Foo', {});
+    expect(action0.routeName).toBe('B');
+    expect(action0.action.routeName).toBe('Foo');
+
+    const action1 = router.getActionForPathAndParams('', {});
+    expect(action1.routeName).toBe('C');
+    expect(action1.action.routeName).toBe('Empty');
+
+    const action2 = router.getActionForPathAndParams('A', {});
+    expect(action2.routeName).toBe('A');
+
+    const action3 = router.getActionForPathAndParams('bar/asdf', {});
+    expect(action3.routeName).toBe('C');
+    expect(action3.action.routeName).toBe('Bar');
+    expect(action3.action.params.id).toBe('asdf');
+  });
+
+  test('Null and empty string path sub routers behavior', () => {
+    const ScreenA = () => <div />;
+    const ScreenB = () => <div />;
+    ScreenB.router = createTestRouter({
+      Foo: ScreenA,
+      Baz: {
+        screen: ScreenA,
+        path: '',
+      },
+    });
+    const ScreenC = () => <div />;
+    ScreenC.router = createTestRouter({
+      Boo: ScreenA,
+      Bar: ScreenA,
+      Baz: {
+        screen: ScreenA,
+        path: '',
+      },
+    });
+    const router = createTestRouter({
+      B: {
+        path: null,
+        screen: ScreenB,
+      },
+      C: {
+        path: '',
+        screen: ScreenC,
+      },
+    });
+    const action0 = router.getActionForPathAndParams('', {});
+    expect(action0.routeName).toBe('C');
+    expect(action0.action.routeName).toBe('Baz');
+
+    const action1 = router.getActionForPathAndParams('Foo', {});
+    expect(action1.routeName).toBe('B');
+    expect(action1.action.routeName).toBe('Foo');
+
+    const action2 = router.getActionForPathAndParams('Bar', {});
+    expect(action2.routeName).toBe('C');
+    expect(action2.action.routeName).toBe('Bar');
+
+    const action3 = router.getActionForPathAndParams('unknown', {});
+    expect(action3).toBe(null);
+  });
+
+  test('Empty path acts as wildcard for nested router', () => {
+    const ScreenA = () => <div />;
+    const Foo = () => <div />;
+    const ScreenC = () => <div />;
+    ScreenC.router = createTestRouter({
+      Boo: ScreenA,
+      Bar: ScreenA,
+    });
+    Foo.router = createTestRouter({
+      Quo: ScreenA,
+      Qux: {
+        screen: ScreenC,
+        path: '',
+      },
+    });
+    const router = createTestRouter({
+      Bar: {
+        screen: ScreenA,
+      },
+      Foo,
+    });
+    const action0 = router.getActionForPathAndParams('Foo/Bar', {});
+    expect(action0.routeName).toBe('Foo');
+    expect(action0.action.routeName).toBe('Qux');
+    expect(action0.action.action.routeName).toBe('Bar');
+  });
+
   test('Gets deep path with pure wildcard match', () => {
     const ScreenA = () => <div />;
     const ScreenB = () => <div />;
@@ -153,7 +387,6 @@ const performRouterTest = createTestRouter => {
       const { path, params } = router.getPathAndParamsForState(state);
       expect(path).toEqual('baz/321');
       expect(params.id).toEqual('123');
-      expect(params.bazId).toEqual('321');
     }
 
     {
@@ -208,6 +441,31 @@ const performRouterTest = createTestRouter => {
     });
   });
 
+  test('URI encoded path param gets parsed and correctly printed', () => {
+    const router = createTestRouter({
+      main: {
+        screen: () => <div />,
+      },
+      person: {
+        path: 'people/:name',
+        screen: () => <div />,
+      },
+    });
+
+    const action = testRouter.getActionForPathAndParams('people/Henry%20L');
+    expect(action).toEqual({
+      routeName: 'person',
+      params: {
+        id: 'Henry L',
+      },
+      type: NavigationActions.NAVIGATE,
+    });
+    const s = testRouter.getStateForAction(action);
+    const out = testRouter.getPathAndParamsForState(s);
+    expect(out.path).toEqual('people/Henry%20L');
+    expect(out.params).toEqual({});
+  });
+
   test('Querystring params get passed to nested deep link', () => {
     const action = testRouter.getActionForPathAndParams(
       'main/p/4/list/10259959195',
@@ -297,3 +555,24 @@ describe('Path handling for stack router', () => {
 describe('Path handling for switch router', () => {
   performRouterTest(SwitchRouter);
 });
+
+test('Handles nested switch routers', () => {
+  const AScreen = () => <div />;
+  const DocsNavigator = () => <div />;
+  DocsNavigator.router = SwitchRouter({
+    A: AScreen,
+    B: AScreen,
+    C: AScreen,
+  });
+  DocsNavigator.path = 'docs';
+  const router = SwitchRouter({
+    Docs: DocsNavigator,
+    D: AScreen,
+  });
+  const action = router.getActionForPathAndParams('docs/B', {});
+
+  expect(action.type).toEqual(NavigationActions.NAVIGATE);
+  expect(action.routeName).toEqual('Docs');
+  expect(action.action.type).toEqual(NavigationActions.NAVIGATE);
+  expect(action.action.routeName).toEqual('B');
+});

src/routers/__tests__/StackRouter-test.js

@@ -319,7 +319,7 @@ describe('StackRouter', () => {
   });
 
   test('Correctly returns action chain for partially matched path', () => {
-    const uri = 'auth/login/2';
+    const uri = 'auth/login';
     const action = TestStackRouter.getActionForPathAndParams(uri);
     expect(action).toEqual({
       type: NavigationActions.NAVIGATE,
@@ -1206,8 +1206,7 @@ describe('StackRouter', () => {
     };
     const { path, params } = router.getPathAndParamsForState(state);
     expect(path).toEqual('f/123/baz/321');
-    expect(params.id).toEqual('123');
-    expect(params.bazId).toEqual('321');
+    expect(params).toEqual({});
   });
 
   test('Handle goBack identified by key', () => {

src/routers/pathUtils.js

@@ -1,5 +1,7 @@
 import pathToRegexp from 'path-to-regexp';
 import NavigationActions from '../NavigationActions';
+import invariant from '../utils/invariant';
+
 const queryString = require('query-string');
 
 function isEmpty(obj) {
@@ -10,6 +12,37 @@ function isEmpty(obj) {
   return true;
 }
 
+const getParamsFromPath = (inputParams, pathMatch, pathMatchKeys) => {
+  const params = pathMatch.slice(1).reduce(
+    // iterate over matched path params
+    (paramsOut, matchResult, i) => {
+      const key = pathMatchKeys[i];
+      if (!key || key.asterisk) {
+        return paramsOut;
+      }
+      const paramName = key.name;
+
+      let decodedMatchResult;
+      try {
+        decodedMatchResult = decodeURIComponent(matchResult);
+      } catch (e) {
+        // ignore `URIError: malformed URI`
+      }
+
+      paramsOut[paramName] = decodedMatchResult || matchResult;
+      return paramsOut;
+    },
+    {
+      // start with the input(query string) params, which will get overridden by path params
+      ...inputParams,
+    }
+  );
+  return params;
+};
+const getRestOfPath = (pathMatch, pathMatchKeys) => {
+  const rest = pathMatch[pathMatchKeys.findIndex(k => k.asterisk) + 1];
+  return rest;
+};
 export const urlToPathAndParams = (url, uriPrefix) => {
   const searchMatch = url.match(/^(.*)\?(.*)$/);
   const params = searchMatch ? queryString.parse(searchMatch[2]) : {};
@@ -34,138 +67,145 @@ export const urlToPathAndParams = (url, uriPrefix) => {
 export const createPathParser = (
   childRouters,
   routeConfigs,
-  pathConfigs = {},
-  initialRouteName,
-  initialRouteParams
+  pathConfigs = {}
 ) => {
   const pathsByRouteNames = {};
   let paths = [];
 
-  // Build paths for each route
+  // Build pathsByRouteNames, which includes a regex to match paths for each route. Keep in mind, the regex will pass for the route and all child routes. The code that uses pathsByRouteNames will need to also verify that the child router produces an action, in the case of isPathMatchable false (a null path).
   Object.keys(childRouters).forEach(routeName => {
     let pathPattern = pathConfigs[routeName] || routeConfigs[routeName].path;
-    let matchExact = !!pathPattern && !childRouters[routeName];
+
     if (pathPattern === undefined) {
+      // If the user hasn't specified a path at all, then we assume the routeName is an appropriate path
       pathPattern = routeName;
     }
-    const keys = [];
-    let re, toPath, priority;
-    if (typeof pathPattern === 'string') {
-      // pathPattern may be either a string or a regexp object according to path-to-regexp docs.
-      re = pathToRegexp(pathPattern, keys);
-      toPath = pathToRegexp.compile(pathPattern);
-      priority = 0;
-    } else if (pathPattern === null) {
-      // for wildcard match
-      re = pathToRegexp('*', keys);
-      toPath = () => '';
-      matchExact = true;
-      priority = -1;
-    }
-    if (!matchExact) {
-      const wildcardRe = pathToRegexp(`${pathPattern}/*`, keys);
-      re = new RegExp(`(?:${re.source})|(?:${wildcardRe.source})`);
-    }
-    pathsByRouteNames[routeName] = { re, keys, toPath, priority, pathPattern };
+
+    invariant(
+      pathPattern === null || typeof pathPattern === 'string',
+      `Route path for ${routeName} must be specified as a string, or null.`
+    );
+
+    // the path may be specified as null, which is similar to empty string because it allows child routers to handle the action, but it will not match empty paths
+    const isPathMatchable = pathPattern !== null;
+    // pathPattern is a string with inline params, such as people/:id/*foo
+    const exactReKeys = [];
+    const exactRe = isPathMatchable
+      ? pathToRegexp(pathPattern, exactReKeys)
+      : null;
+    const extendedPathReKeys = [];
+    const isWildcard = pathPattern === '' || !isPathMatchable;
+    const extendedPathRe = pathToRegexp(
+      isWildcard ? '*' : `${pathPattern}/*`,
+      extendedPathReKeys
+    );
+
+    pathsByRouteNames[routeName] = {
+      exactRe,
+      exactReKeys,
+      extendedPathRe,
+      extendedPathReKeys,
+      isWildcard,
+      toPath:
+        pathPattern === null ? () => '' : pathToRegexp.compile(pathPattern),
+    };
   });
 
   paths = Object.entries(pathsByRouteNames);
-  paths.sort((a, b) => b[1].priority - a[1].priority);
 
-  const getActionForPathAndParams = (pathToResolve, inputParams = {}) => {
-    // If the path is empty (null or empty string)
-    // just return the initial route action
-    if (!pathToResolve) {
-      return NavigationActions.navigate({
-        routeName: initialRouteName,
-        params: { ...inputParams, ...initialRouteParams },
-      });
-    }
+  const getActionForPathAndParams = (pathToResolve = '', inputParams = {}) => {
+    // Attempt to match `pathToResolve` with a route in this router's routeConfigs, deferring to child routers
 
-    // Attempt to match `pathToResolve` with a route in this router's
-    // routeConfigs
-    let matchedRouteName;
-    let pathMatch;
-    let pathMatchKeys;
+    let matchedAction = null;
 
     // eslint-disable-next-line no-restricted-syntax
     for (const [routeName, path] of paths) {
-      const { re, keys } = path;
-      pathMatch = re.exec(pathToResolve);
-      if (pathMatch && pathMatch.length) {
-        pathMatchKeys = keys;
-        matchedRouteName = routeName;
-        break;
-      }
-    }
+      const { exactRe, exactReKeys, extendedPathRe, extendedPathReKeys } = path;
+      const childRouter = childRouters[routeName];
 
-    // We didn't match -- return null to signify no action available
-    if (!matchedRouteName) {
-      return null;
-    }
+      const exactMatch = exactRe && exactRe.exec(pathToResolve);
 
-    // Determine nested actions:
-    // If our matched route for this router is a child router,
-    // get the action for the path AFTER the matched path for this
-    // router
-    let nestedAction;
-    if (childRouters[matchedRouteName]) {
-      nestedAction = childRouters[matchedRouteName].getActionForPathAndParams(
-        pathMatch.slice(pathMatchKeys.length).join('/'),
-        inputParams
-      );
-      if (!nestedAction) {
-        return null;
-      }
-    }
-
-    const params = pathMatch.slice(1).reduce(
-      // iterate over matched path params
-      (paramsOut, matchResult, i) => {
-        const key = pathMatchKeys[i];
-        if (!key || key.asterisk) {
-          return paramsOut;
-        }
-        const paramName = key.name;
-
-        let decodedMatchResult;
-        try {
-          decodedMatchResult = decodeURIComponent(matchResult);
-        } catch (e) {
-          // ignore `URIError: malformed URI`
+      if (exactMatch && exactMatch.length) {
+        const extendedMatch =
+          extendedPathRe && extendedPathRe.exec(pathToResolve);
+        let childAction = null;
+        if (extendedMatch && childRouter) {
+          const restOfPath = getRestOfPath(extendedMatch, extendedPathReKeys);
+          childAction = childRouter.getActionForPathAndParams(
+            restOfPath,
+            inputParams
+          );
         }
 
-        paramsOut[paramName] = decodedMatchResult || matchResult;
-        return paramsOut;
-      },
-      {
-        // start with the input(query string) params, which will get overridden by path params
-        ...inputParams,
+        return NavigationActions.navigate({
+          routeName,
+          params: getParamsFromPath(inputParams, exactMatch, exactReKeys),
+          action: childAction,
+        });
       }
-    );
+    }
 
-    return NavigationActions.navigate({
-      routeName: matchedRouteName,
-      ...(params ? { params } : {}),
-      ...(nestedAction ? { action: nestedAction } : {}),
-    });
+    // eslint-disable-next-line no-restricted-syntax
+    for (const [routeName, path] of paths) {
+      const { extendedPathRe, extendedPathReKeys } = path;
+      const childRouter = childRouters[routeName];
+
+      const extendedMatch =
+        extendedPathRe && extendedPathRe.exec(pathToResolve);
+
+      if (extendedMatch && extendedMatch.length) {
+        const restOfPath = getRestOfPath(extendedMatch, extendedPathReKeys);
+        let childAction = null;
+        if (childRouter) {
+          childAction = childRouter.getActionForPathAndParams(
+            restOfPath,
+            inputParams
+          );
+        }
+        if (!childAction) {
+          continue;
+        }
+        return NavigationActions.navigate({
+          routeName,
+          params: getParamsFromPath(
+            inputParams,
+            extendedMatch,
+            extendedPathReKeys
+          ),
+          action: childAction,
+        });
+      }
+    }
+
+    return null;
   };
   const getPathAndParamsForRoute = route => {
     const { routeName, params } = route;
     const childRouter = childRouters[routeName];
-    const subPath = pathsByRouteNames[routeName].toPath(params);
+    const { toPath, exactReKeys } = pathsByRouteNames[routeName];
+    const subPath = toPath(params);
+    const nonPathParams = {};
+    if (params) {
+      Object.keys(params)
+        .filter(paramName => !exactReKeys.find(k => k.name === paramName))
+        .forEach(paramName => {
+          nonPathParams[paramName] = params[paramName];
+        });
+    }
     if (childRouter) {
       // If it has a router it's a navigator.
       // If it doesn't have router it's an ordinary React component.
       const child = childRouter.getPathAndParamsForState(route);
       return {
         path: subPath ? `${subPath}/${child.path}` : child.path,
-        params: child.params ? { ...params, ...child.params } : params,
+        params: child.params
+          ? { ...nonPathParams, ...child.params }
+          : nonPathParams,
       };
     }
     return {
       path: subPath,
-      params,
+      params: nonPathParams,
     };
   };
   return { getActionForPathAndParams, getPathAndParamsForRoute };