Allow router store push/replace to take a LocationShape (#171)

Author: yamadapc

docs/api/components.md

@@ -256,9 +256,9 @@ By using either of these you will gain access to the following actions
 
 | prop            | type       | arguments                                                | description                                                                                           |
 | --------------- | ---------- | -------------------------------------------------------- | ----------------------------------------------------------------------------------------------------- |
-| `push`          | `function` | `path: Href, state?: any`                                | Calls `history.push` with the supplied args                                                           |
+| `push`          | `function` | `path: Href | Location, state?: any`                     | Calls `history.push` with the supplied args                                                           |
 | `pushTo`        | `function` | `route: Route, attributes?: { params?: {}, query?: {} }` | Calls `history.push` generating the path from supplied route and attributes                           |
-| `replace`       | `function` | `path: Href, state?: any`                                | Calls `history.replace` with the supplied args                                                        |
+| `replace`       | `function` | `path: Href | Location, state?: any`                     | Calls `history.replace` with the supplied args                                                        |
 | `replaceTo`     | `function` | `route: Route, attributes?: { params?: {}, query?: {} }` | Calls `history.replace` generating the path from supplied route and attributes                        |
 | `goBack`        | `function` |                                                          | Goes to the previous route in history                                                                 |
 | `goForward`     | `function` |                                                          | Goes to the next route in history                                                                     |

src/__tests__/unit/common/utils/history/test.ts

@@ -117,21 +117,52 @@ describe('createLegacyHistory', () => {
       window.location = holdWindowLocation;
     });
 
+    it('should not throw when null', () => {
+      const history = createLegacyHistory();
+      history.push(null as any);
+      expect(spy).toHaveBeenCalledWith('/');
+    })
+
     it('should change location via page reload', async () => {
       const history = createLegacyHistory();
       history.push('/foo');
 
       expect(spy).toHaveBeenCalledWith('/foo');
     });
+
+    it('should change location if a Location object is received', async () => {
+      const history = createLegacyHistory();
+      history.push({ pathname: '/other', search: '?param=1', hash: '' });
+
+      expect(spy).toHaveBeenCalledWith('/other?param=1');
+    });
   });
 
   describe('replace()', () => {
+    it('should not throw when null', () => {
+      window.history.replaceState = jest.fn();
+      const history = createLegacyHistory();
+      history.replace(null as any);
+      expect(window.history.replaceState).toHaveBeenCalledWith({}, '', '/');
+    })
+
     it('should replace location via history', () => {
       window.history.replaceState = jest.fn();
       const history = createLegacyHistory();
       history.replace('/baz');
       expect(window.history.replaceState).toHaveBeenCalledWith({}, '', '/baz');
     });
+
+    it('should replace location if a Location object is received', async () => {
+      window.history.replaceState = jest.fn();
+      const history = createLegacyHistory();
+      history.replace({ pathname: '/other', search: '?param=1', hash: '' });
+      expect(window.history.replaceState).toHaveBeenCalledWith(
+        {},
+        '',
+        '/other?param=1'
+      );
+    });
   });
 
   describe('goBack()', () => {

src/__tests__/unit/controllers/router-store/test.tsx

@@ -369,6 +369,23 @@ describe('SPA Router store', () => {
         expect(mockHistory.push).toBeCalledWith('/board/123');
       });
 
+      it('should push a relative path a Location object is pushed', () => {
+        mount(
+          <MemoryRouter routes={[]}>
+            <RouterSubscriber>{children}</RouterSubscriber>
+          </MemoryRouter>
+        );
+        const { actions } = getRouterStore();
+
+        actions.push({ pathname: '/board/123', search: '', hash: '' });
+
+        expect(mockHistory.push).toBeCalledWith({
+          pathname: '/board/123',
+          search: '',
+          hash: '',
+        });
+      });
+
       it('should call window.location.assign with the absolute URL if it is on a different domain', () => {
         jest
           .spyOn(window.location, 'assign')

src/common/types.ts

@@ -26,8 +26,8 @@ export type BrowserHistory = (
   | Omit<History5, 'location' | 'go' | 'createHref' | 'push' | 'replace'>
 ) & {
   location: Location;
-  push: (path: string) => void;
-  replace: (path: string) => void;
+  push: (path: string | Location) => void;
+  replace: (path: string | Location) => void;
 };
 
 export type MatchParams = {

src/common/utils/history/index.ts

@@ -110,9 +110,16 @@ export const createLegacyHistory = (): BrowserHistory => {
     },
     ...(hasWindow()
       ? {
-          push: (path: string) => window.location.assign(path),
-          replace: (path: string) =>
-            window.history.replaceState({}, document.title, path),
+          push: (path: string | Location) =>
+            window.location.assign(
+              typeof path === 'string' ? path : createPath(path || {})
+            ),
+          replace: (path: string | Location) =>
+            window.history.replaceState(
+              {},
+              document.title,
+              typeof path === 'string' ? path : createPath(path || {})
+            ),
           goBack: () => window.history.back(),
           goForward: () => window.history.forward(),
           listen: createLegacyListener(updateExposedLocation),

src/controllers/router-store/index.tsx

@@ -255,7 +255,7 @@ const actions: AllRouterActions = {
       if (isExternalAbsolutePath(path)) {
         window.location.assign(path as string);
       } else {
-        history.push(getRelativePath(path, basePath) as any);
+        history.push(getRelativePath(path, basePath));
       }
     },
 

src/controllers/router-store/types.ts

@@ -104,9 +104,9 @@ type PrivateRouterActions = {
 };
 
 type PublicRouterActions = {
-  push: (path: Href, state?: any) => RouterAction;
+  push: (path: Href | Location, state?: any) => RouterAction;
   pushTo: (route: Route, attributes?: ToAttributes) => RouterAction;
-  replace: (path: Href) => RouterAction;
+  replace: (path: Href | Location) => RouterAction;
   replaceTo: (route: Route, attributes?: ToAttributes) => RouterAction;
   goBack: () => RouterAction;
   goForward: () => RouterAction;

src/types.js.flow

@@ -215,9 +215,9 @@ export type RouterAction = Action<EntireRouterState, ContainerProps, *>;
 type UnregisterCallback = () => void;
 
 export type RouterActionsType = {|
-  push: (path: Href, state?: any) => void,
+  push: (path: Href | Location, state?: any) => void,
   pushTo: (route: Route, attributes: ToAttributes) => void,
-  replace: (path: Href, state?: any) => void,
+  replace: (path: Href | Location, state?: any) => void,
   replaceTo: (route: Route, attributes: ToAttributes) => void,
   goBack: () => void,
   goForward: () => void,