Allow Redirect component to accept Route object as to (#108)

* Allow Redirect component to accept Route object as 'to'

* Adjust flow types and remove 'any'

* Fix minor test issue

Author: jpqne

src/__tests__/unit/controllers/redirect/test.tsx

@@ -6,6 +6,7 @@ import { defaultRegistry } from 'react-sweet-state';
 import { Redirect } from '../../../../controllers/redirect';
 import { RedirectProps } from '../../../../controllers/redirect/types';
 import { Router } from '../../../../controllers/router';
+import type { Route } from '../../../../common/types';
 
 const MockLocation = {
   pathname: 'pathname',
@@ -32,6 +33,12 @@ const defaultArgs = {
 };
 
 const mockConsole = { ...global.console, warn: jest.fn() };
+const mockRoute: Route = {
+  name: 'test',
+  component: () => null,
+  path: '/:id',
+  query: ['foo'],
+};
 
 describe('Redirect', () => {
   const mountInRouter = (args: Partial<RedirectProps>) =>
@@ -81,13 +88,54 @@ describe('Redirect', () => {
     expect(MockHistory.push).toHaveBeenCalledWith(to);
   });
 
-  it('should navigate to given route correctly', () => {
-    const to = '/cool-page';
-
-    mountInRouter({ to, push: false });
-    expect(MockHistory.replace).toHaveBeenCalledWith(to);
-    expect(MockHistory.push).not.toHaveBeenCalled();
-  });
+  it.each([
+    [
+      'with `params` and `query`',
+      { name: 'a', path: '/:id', query: ['foo'] } as Route,
+      { id: '1' },
+      { foo: 'bar' },
+      '/1?foo=bar',
+    ],
+    [
+      'with `params`',
+      { name: 'b', path: '/:id', query: ['foo'] } as Route,
+      { id: '1' },
+      undefined,
+      '/1',
+    ],
+    [
+      'with `query`',
+      { name: 'c', path: '/home', query: ['page'] } as Route,
+      undefined,
+      { page: '1' },
+      '/home?page=1',
+    ],
+    [
+      'without `params` and `query`',
+      { name: 'd', path: '/menu', query: ['page'] } as Route,
+      undefined,
+      undefined,
+      '/menu',
+    ],
+  ])(
+    "doesn't break / throw when rendered with `to` as a Route object, %s",
+    (_, to, params, query, expected) => {
+      expect(() => mountInRouter({ to, params, query })).not.toThrow();
+      expect(MockHistory.push).toHaveBeenCalledWith(expected);
+    }
+  );
+
+  it.each([
+    ['string', '/cool-page', undefined, undefined, '/cool-page'],
+    ['object', mockRoute, { id: '2' }, { foo: 'bar' }, '/2?foo=bar'],
+  ])(
+    'should navigate to given route %s correctly',
+    (_, to, params, query, expected) => {
+      mountInRouter({ to, query, params, push: false });
+      expect(MockHistory.replace).toHaveBeenCalledWith(expected);
+      expect(MockHistory.push).not.toHaveBeenCalled();
+    }
+  );
 
   it('should navigate to absolute URLs', () => {
     jest.spyOn(window.location, 'assign').mockImplementation(() => jest.fn());
@@ -99,19 +147,29 @@ describe('Redirect', () => {
     expect(window.location.assign).toHaveBeenCalledWith(to);
   });
 
-  it('should not redirect if the location is equivalent to current', () => {
-    mountInRouter({ to: 'pathname' });
-
-    expect(mockConsole.warn).toHaveBeenCalledWith(expect.any(String));
-    expect(MockHistory.replace).not.toHaveBeenCalled();
-    expect(MockHistory.push).not.toHaveBeenCalled();
-  });
-
-  it('should use push history correctly', () => {
-    const to = '/cool-page';
-
-    mountInRouter({ to, push: true });
-    expect(MockHistory.push).toHaveBeenCalledWith(to);
-    expect(MockHistory.replace).not.toHaveBeenCalled();
-  });
+  it.each([
+    ['string', 'pathname'],
+    ['object', { name: 'a', path: 'pathname' } as Route],
+  ])(
+    'should not redirect if the given route %s is equivalent to current location',
+    (_, to) => {
+      mountInRouter({ to });
+
+      expect(mockConsole.warn).toHaveBeenCalledWith(expect.any(String));
+      expect(MockHistory.replace).not.toHaveBeenCalled();
+      expect(MockHistory.push).not.toHaveBeenCalled();
+    }
+  );
+
+  it.each([
+    ['string', '/cool-page', undefined, '/cool-page'],
+    ['object', mockRoute, { id: '3' }, '/3'],
+  ])(
+    'should use push history correctly with given route %s',
+    (_, to, params, expected) => {
+      mountInRouter({ to, params, push: true });
+      expect(MockHistory.push).toHaveBeenCalledWith(expected);
+      expect(MockHistory.replace).not.toHaveBeenCalled();
+    }
+  );
 });

src/controllers/redirect/index.tsx

@@ -6,6 +6,7 @@ import { RouterActionsType, RouterState } from '../router-store/types';
 import { RouterSubscriber } from '../subscribers/route';
 
 import { RedirectProps } from './types';
+import { generateLocationFromPath } from '../../common/utils';
 
 type RedirectorProps = RedirectProps & {
   actions: RouterActionsType;
@@ -18,8 +19,18 @@ class Redirector extends Component<RedirectorProps> {
   };
 
   componentDidMount() {
-    const { to, location, push, actions } = this.props;
-    const newPath = typeof to === 'object' ? createPath(to) : to;
+    const { to, location, push, params, query, actions } = this.props;
+    const routeAttributes = {
+      params,
+      query,
+      basePath: actions.getBasePath(),
+    };
+    const newPath =
+      typeof to === 'object'
+        ? 'path' in to
+          ? createPath(generateLocationFromPath(to.path, routeAttributes))
+          : createPath(to)
+        : to;
     const currentPath = createPath(location);
     const action = push ? actions.push : actions.replace;
 

src/controllers/redirect/types.ts

@@ -1,6 +1,8 @@
-import { Location } from '../../common/types';
+import { Location, MatchParams, Query, Route } from '../../common/types';
 
 export type RedirectProps = {
-  to: Location | string;
+  to: Location | Route | string;
   push?: boolean;
+  params?: MatchParams;
+  query?: Query;
 };

src/index.js.flow

@@ -7,7 +7,9 @@ import type {
   HistoryUpdateType,
   LinkProps,
   Location,
+  MatchParams,
   MemoryRouterProps,
+  Query,
   ResourceOptions,
   ResourceStoreContext,
   Route,
@@ -32,8 +34,10 @@ export * from './types.js.flow';
 
 declare export function Link(props: LinkProps): Node;
 declare export function Redirect(props: {
-  to: Location | string,
+  to: Location | Route | string,
   push?: boolean,
+  params?: MatchParams,
+  query?: Query,
 }): Node;
 
 declare export function RouterActions(props: {|
@@ -82,7 +86,7 @@ declare export function useRouterActions(): RouterActionsType;
 declare export function useResourceStoreContext(): ResourceStoreContext;
 declare export function createRouterSelector<T, U = void>(
   selector: (state: RouterState, props: U) => T
-): (U) => T;
+): U => T;
 
 declare export function useQueryParam(
   paramKey: string
@@ -149,4 +153,6 @@ type CreateResourceArg<T> =
       maxCache?: number,
     |};
 
-declare export function createResource<T>(args: CreateResourceArg<T>): RouteResource<T>;
+declare export function createResource<T>(
+  args: CreateResourceArg<T>
+): RouteResource<T>;

src/types.js.flow

@@ -16,13 +16,15 @@ export type Location = {|
 
 export type Query = { [string]: string };
 
+export type MatchParams = { [key: string]: string | null | typeof undefined };
+
 export type Match = {|
   /** TODO we are supporting `undefined` here because we are currently using both
    * this version of the `Match` type, and react-routers version (which allows for `undefined`)
    * To fix this we should move `matchPath` to our own util so we can apply our own types, then
    * decide if we want to support undefined types.
    */
-  params: { [string]: string | null | typeof undefined },
+  params: MatchParams,
   query: Query,
   isExact: boolean,
   path: string,
@@ -100,7 +102,9 @@ export type ResourceOptions = {
   routerContext?: RouterContext,
 };
 
-export type RouteResourceUpdater<T> = (data: RouteResourceData<T>) => RouteResourceData<T>;
+export type RouteResourceUpdater<T> = (
+  data: RouteResourceData<T>
+) => RouteResourceData<T>;
 
 export type InvariantRoute = {
   path: string,
@@ -259,10 +263,6 @@ export type RequestResourcesParams = {
   timeout?: number,
 };
 
-export type MatchParams = {
-  [key: string]: string | null | typeof undefined,
-};
-
 export type ResourceStoreData =
   | { [type: string]: RouteResourceDataForType }
   | {||};

src/ui/link/index.tsx

@@ -55,7 +55,7 @@ const Link = forwardRef<HTMLButtonElement | HTMLAnchorElement, LinkProps>(
     const routeAttributes = {
       params,
       query,
-      basePath: routerActions.getBasePath() as any,
+      basePath: routerActions.getBasePath(),
     };
     const linkDestination =
       href != null