update routeHasChanged() to check route name and group instead of object reference (#125)

* update routeHasChanged() to check route name and group instead of object ref

* ignore group when checking routeHasChanged

* update test name

* remove group, canTransitionOut and canTransitionIn from types

* check path at routeHasChanged()

Co-authored-by: Liam Ma <[email protected]>

Author: liamqma

src/__tests__/unit/controllers/resource-store/utils/get-resources-for-next-location/test.ts

@@ -69,10 +69,12 @@ describe('getResourcesForNextLocation()', () => {
   describe('when the next route does not match the prev route', () => {
     it('should request all resources on the next route', async () => {
       const prevRoute = {
+        name: 'prev-route',
         path: '/prev-route',
         resources: [mockResource],
       };
       const nextRoute = {
+        name: 'next-route',
         path: '/next-route',
         resources: [
           mockResource,

src/__tests__/unit/controllers/resource-store/utils/route-checks/test.ts

@@ -3,43 +3,33 @@ import {
   routeHasResources,
 } from '../../../../../../controllers/resource-store/utils/route-checks';
 
+const mockRoute = {
+  name: 'foo',
+  path: '/some-path',
+  component: () => null,
+};
+
 describe('routeHasChanged()', () => {
-  it('should return true if the route objects do not match', () => {
+  it('should return true if the route name does not match', () => {
     expect(
-      routeHasChanged(
-        // @ts-ignore - not providing all properties on mock
-        {
-          path: '/some-path',
-          component: () => null,
-        },
-        {
-          path: '/another-path',
-          component: () => null,
-        }
-      )
+      routeHasChanged(mockRoute, {
+        ...mockRoute,
+        name: 'bar',
+      })
     ).toBeTruthy();
   });
 
-  it('should return true if the prev route is null', () => {
+  it('should return true if the route path does not match', () => {
     expect(
-      routeHasChanged(
-        null,
-        // @ts-ignore - not providing all properties on mock
-        {
-          path: '/another-path',
-          component: () => null,
-        }
-      )
+      routeHasChanged(mockRoute, {
+        ...mockRoute,
+        path: '/bar',
+      })
     ).toBeTruthy();
   });
 
-  it('should return false if the routes match', () => {
-    const route = {
-      path: '/some-path',
-      component: () => null,
-    };
-    // @ts-ignore - not providing all properties on mock
-    expect(routeHasChanged(route, route)).toBeFalsy();
+  it('should return false if the route name matches', () => {
+    expect(routeHasChanged(mockRoute, { ...mockRoute })).toBeFalsy();
   });
 });
 

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

@@ -737,6 +737,7 @@ describe('SPA Router store', () => {
         },
         {
           ...mockRoute,
+          name: 'bar',
           path: '/bar/a',
           component: ComponentB,
           resources: [resourceA, resourceB],

src/common/types.ts

@@ -175,9 +175,6 @@ export type InvariantRoute = {
   path: string;
   exact?: boolean;
 
-  /** Used to prevent transitions between app groups */
-  group?: string;
-
   /** Unique name for the route */
   name: string;
 
@@ -200,26 +197,6 @@ export type Route = InvariantRoute & {
   /** The component to render on match, typed explicitly */
   component: ComponentType<RouteContext>;
 
-  /**
-   * Triggered before leaving the route, can trigger full page reload if returns (or resolves) false.
-   * Defaults to true.
-   */
-  canTransitionOut?: (
-    currentRouteMatch: MatchedRoute,
-    nextRouteMatch: MatchedRoute,
-    props: any
-  ) => boolean | Promise<boolean>;
-
-  /**
-   * Triggered before entering the route, can trigger full page reload if returns (or resolves) false.
-   * Defaults to true.
-   */
-  canTransitionIn?: (
-    currentRouteMatch: MatchedRoute,
-    nextRouteMatch: MatchedRoute,
-    props: any
-  ) => boolean | Promise<boolean>;
-
   /**
    * The resources for the route
    */

src/controllers/resource-store/utils/route-checks/index.ts

@@ -3,7 +3,5 @@ import { Route } from '../../../../common/types';
 export const routeHasResources = (route: Route | null): boolean =>
   !!(route && route.resources && route.resources.length > 0);
 
-export const routeHasChanged = (
-  prev: Route | null,
-  next: Route | null
-): boolean => prev !== next;
+export const routeHasChanged = (prev: Route, next: Route): boolean =>
+  prev.name !== next.name || prev.path !== next.path;

src/types.js.flow

@@ -109,8 +109,6 @@ export type RouteResourceUpdater<T> = (
 export type InvariantRoute = {
   path: string,
   exact?: boolean,
-  /** Used to prevent transitions between app groups */
-  group?: string,
   /** Unique name for the route */
   name: string,
   /**