Improve skipRender fix for directional navigation introduced in v1.6.2

santino · Jul 12, 2023 · f497419 · f497419
1 parent 7c0aa53
commit f497419
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 25 deletions.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "react-concurrent-router",
-  "version": "1.6.2",
+  "version": "1.6.3",
   "description": "Performant routing embracing React concurrent UI patterns",
   "author": "Santino Puleio",
   "license": "MIT",

diff --git a/src/__tests__/createRouter.test.js b/src/__tests__/createRouter.test.js
@@ -282,7 +282,7 @@ describe('createRouter', () => {
       )
       expect(locationsMatch).toHaveBeenNthCalledWith(
         3,
-        { pathname: 'matchedLocation', state: null },
+        { pathname: 'matchedLocation' },
         { pathname: 'newLocation' },
         true
       )
@@ -321,8 +321,7 @@ describe('createRouter', () => {
       expect(prepareMatch).toHaveBeenCalledTimes(2)
       expect(defaultProps.history.replace).toHaveBeenCalledTimes(1)
       expect(defaultProps.history.replace).toHaveBeenCalledWith({
-        pathname: 'matchedLocation',
-        state: null
+        pathname: 'matchedLocation'
       })
       expect(mockSubscribeCallback).not.toHaveBeenCalled()
     })
@@ -380,8 +379,7 @@ describe('createRouter', () => {
       }
       const matchedRoute = {
         pathname: 'newLocation',
-        params: { foo: 'bar' },
-        state: null
+        params: { foo: 'bar' }
       }
       const router = createRouter(defaultProps)
       const mockSubscribeCallback = jest.fn()
@@ -410,8 +408,7 @@ describe('createRouter', () => {
         skipRenderLocation
       )
       expect(prepareMatch).toHaveBeenCalledTimes(1)
-      expect(defaultProps.history.replace).toHaveBeenCalledTimes(1)
-      expect(defaultProps.history.replace).toHaveBeenCalledWith(matchedRoute)
+      expect(defaultProps.history.replace).not.toHaveBeenCalled()
       expect(mockSubscribeCallback).toHaveBeenCalledTimes(1)
       expect(mockSubscribeCallback).toHaveBeenCalledWith({
         component: mockComponent,

diff --git a/src/createRouter.js b/src/createRouter.js
@@ -9,8 +9,8 @@ const createRouter = ({
   assistPrefetch = false,
   awaitComponent = false,
   awaitPrefetch = false,
-  routes,
-  history
+  history,
+  routes
 }) => {
   const routesMap = routesToMap(routes)
   const entryMatch = matchRoutes(routesMap, history.location)
@@ -27,33 +27,26 @@ const createRouter = ({
 
   // Listen for location changes, match route entry, prepare the entry and notify subscribers.
   // This pattern ensures that loading (js|data) occurs outside of, and before, rendering.
-  history.listen(({ location }) => {
+  history.listen(({ location, action }) => {
     if (locationsMatch(currentEntry.location, location, true)) return // still on the same route
 
+    // skip render must not affect directional (backward/forward) navigation
+    const skipRender = location.state && location.state.skipRender && action !== 'POP'
     const match = matchRoutes(routesMap, location)
-    // skip render is meant to be for current navigation action only and must not affect future backward/forward navigation
-    const { skipRender, ...locationState } = location.state || {}
-    const useLocation = {
-      ...match.location,
-      state: Object.keys(locationState).length ? locationState : null
-    }
-    const locationHasMatch = locationsMatch(useLocation, location, true)
     const nextEntry = skipRender
       ? {
           ...currentEntry,
-          location: useLocation,
+          location: match.location,
           params: match.params,
-          skipRender: true
+          skipRender
         }
       : prepareMatch(match, assistPrefetch, awaitPrefetch)
 
-    if (skipRender || !locationHasMatch) {
-      // When skipping render we need to override history entry to not break directional navigation (forward/backward). OR
-      // Requested route had redirectRules that have been applied, hence
+    if (!locationsMatch(match.location, location, true)) {
+      // requested route had redirectRules that have been applied, hence
       // the actual destination is not the requested location; update history
-      history.replace(useLocation)
+      return history.replace(match.location)
     }
-    if (!locationHasMatch) return // if redirectRules are aplied we don't notify subscribers
 
     currentEntry = nextEntry
     subscribers.forEach(callback => callback(nextEntry))