From f4974191485cd8861548979a014065e174fe169c Mon Sep 17 00:00:00 2001 From: Santino Puleio Date: Wed, 12 Jul 2023 18:59:51 +0200 Subject: [PATCH] Improve skipRender fix for directional navigation introduced in v1.6.2 --- package.json | 2 +- src/__tests__/createRouter.test.js | 11 ++++------- src/createRouter.js | 27 ++++++++++----------------- 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/package.json b/package.json index 6f9b76c..89e22fe 100644 --- 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 index 6b760bc..398af09 100755 --- 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 index 20984d6..aa88d70 100755 --- 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))