From 4cd696002c2dc78a7271506c0f35970d97d9e921 Mon Sep 17 00:00:00 2001 From: Guido Bouman Date: Sat, 5 Jun 2021 16:29:29 +0200 Subject: [PATCH 1/4] Replace state on repeated Link navigations --- packages/react-router-dom/modules/Link.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/react-router-dom/modules/Link.js b/packages/react-router-dom/modules/Link.js index 25e9de5411..44b4c33b31 100644 --- a/packages/react-router-dom/modules/Link.js +++ b/packages/react-router-dom/modules/Link.js @@ -1,5 +1,6 @@ import React from "react"; import { __RouterContext as RouterContext } from "react-router"; +import { createPath } from 'history'; import PropTypes from "prop-types"; import invariant from "tiny-invariant"; import { @@ -100,7 +101,8 @@ const Link = forwardRef( href, navigate() { const location = resolveToLocation(to, context.location); - const method = replace ? history.replace : history.push; + const isDuplicateNavigation = createPath(context.location) === createPath(location); + const method = (replace || isDuplicateNavigation) ? history.replace : history.push; method(location); } From 8d5442584d10ff3acada818f27f0ad81b748cdb7 Mon Sep 17 00:00:00 2001 From: Guido Bouman Date: Sat, 5 Jun 2021 16:38:53 +0200 Subject: [PATCH 2/4] Normalise possible string location to object --- packages/react-router-dom/modules/Link.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-router-dom/modules/Link.js b/packages/react-router-dom/modules/Link.js index 44b4c33b31..df158ff028 100644 --- a/packages/react-router-dom/modules/Link.js +++ b/packages/react-router-dom/modules/Link.js @@ -101,7 +101,7 @@ const Link = forwardRef( href, navigate() { const location = resolveToLocation(to, context.location); - const isDuplicateNavigation = createPath(context.location) === createPath(location); + const isDuplicateNavigation = createPath(context.location) === createPath(normalizeToLocation(location)); const method = (replace || isDuplicateNavigation) ? history.replace : history.push; method(location); From cc7b01b9a3d616e9a2bb921b35f1240cff7ce692 Mon Sep 17 00:00:00 2001 From: Guido Bouman Date: Sat, 5 Jun 2021 16:45:56 +0200 Subject: [PATCH 3/4] Add test for duplicate navigation detection --- .../modules/__tests__/Link-click-test.js | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/react-router-dom/modules/__tests__/Link-click-test.js b/packages/react-router-dom/modules/__tests__/Link-click-test.js index e33b7d6eec..10cf2952e6 100644 --- a/packages/react-router-dom/modules/__tests__/Link-click-test.js +++ b/packages/react-router-dom/modules/__tests__/Link-click-test.js @@ -44,6 +44,38 @@ describe(" click events", () => { expect(memoryHistory.push).toBeCalledWith(to); }); + it("calls history.replace on duplicate navigation", () => { + const clickHandler = jest.fn(); + const to = "/the/path?the=query#the-hash"; + + renderStrict( + + + link + + , + node + ); + + const a = node.querySelector("a"); + TestUtils.Simulate.click(a, { + defaultPrevented: false, + button: 0 + }); + + TestUtils.Simulate.click(a, { + defaultPrevented: false, + button: 0 + }); + + + expect(clickHandler).toBeCalledTimes(2); + expect(memoryHistory.push).toBeCalledTimes(1); + expect(memoryHistory.push).toBeCalledWith(to); + expect(memoryHistory.replace).toBeCalledTimes(1); + expect(memoryHistory.replace).toBeCalledWith(to); + }); + it("calls onClick eventhandler and history.push with function `to` prop", () => { const memoryHistoryFoo = createMemoryHistory({ initialEntries: ["/foo"] From 941993ebfa53e30963654d4442beb3777a35b11f Mon Sep 17 00:00:00 2001 From: Guido Bouman Date: Wed, 9 Jun 2021 09:13:18 +0200 Subject: [PATCH 4/4] Spy on memoryRouter instead of mocking --- .../modules/__tests__/Link-click-test.js | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/packages/react-router-dom/modules/__tests__/Link-click-test.js b/packages/react-router-dom/modules/__tests__/Link-click-test.js index 10cf2952e6..6be55188d4 100644 --- a/packages/react-router-dom/modules/__tests__/Link-click-test.js +++ b/packages/react-router-dom/modules/__tests__/Link-click-test.js @@ -9,15 +9,19 @@ import renderStrict from "./utils/renderStrict.js"; describe(" click events", () => { const node = document.createElement("div"); - afterEach(() => { - ReactDOM.unmountComponentAtNode(node); + let memoryHistory, pushSpy, replaceSpy; + + beforeEach(() => { + memoryHistory = createMemoryHistory(); + pushSpy = jest.spyOn(memoryHistory, "push"); + replaceSpy = jest.spyOn(memoryHistory, "push"); }); - const memoryHistory = createMemoryHistory(); - memoryHistory.push = jest.fn(); + afterEach(() => { + ReactDOM.unmountComponentAtNode(node); - beforeEach(() => { - memoryHistory.push.mockReset(); + pushSpy.mockRestore(); + replaceSpy.mockRestore(); }); it("calls onClick eventhandler and history.push", () => { @@ -40,13 +44,13 @@ describe(" click events", () => { }); expect(clickHandler).toBeCalledTimes(1); - expect(memoryHistory.push).toBeCalledTimes(1); - expect(memoryHistory.push).toBeCalledWith(to); + expect(pushSpy).toBeCalledTimes(1); + expect(pushSpy).toBeCalledWith(to); }); it("calls history.replace on duplicate navigation", () => { const clickHandler = jest.fn(); - const to = "/the/path?the=query#the-hash"; + const to = "/duplicate/path?the=query#the-hash"; renderStrict( @@ -68,19 +72,17 @@ describe(" click events", () => { button: 0 }); - expect(clickHandler).toBeCalledTimes(2); - expect(memoryHistory.push).toBeCalledTimes(1); - expect(memoryHistory.push).toBeCalledWith(to); - expect(memoryHistory.replace).toBeCalledTimes(1); - expect(memoryHistory.replace).toBeCalledWith(to); + expect(pushSpy).toBeCalledTimes(1); + expect(pushSpy).toBeCalledWith(to); + expect(replaceSpy).toBeCalledTimes(1); + expect(replaceSpy).toBeCalledWith(to); }); it("calls onClick eventhandler and history.push with function `to` prop", () => { - const memoryHistoryFoo = createMemoryHistory({ - initialEntries: ["/foo"] - }); - memoryHistoryFoo.push = jest.fn(); + // Make push a no-op so key IDs do not change + pushSpy.mockImplementation(); + const clickHandler = jest.fn(); let to = null; const toFn = location => { @@ -93,7 +95,7 @@ describe(" click events", () => { }; renderStrict( - + link @@ -108,8 +110,8 @@ describe(" click events", () => { }); expect(clickHandler).toBeCalledTimes(1); - expect(memoryHistoryFoo.push).toBeCalledTimes(1); - expect(memoryHistoryFoo.push).toBeCalledWith(to); + expect(pushSpy).toBeCalledTimes(1); + expect(pushSpy).toBeCalledWith(to); }); it("does not call history.push on right click", () => { @@ -128,7 +130,7 @@ describe(" click events", () => { button: 1 }); - expect(memoryHistory.push).toBeCalledTimes(0); + expect(pushSpy).toBeCalledTimes(0); }); it("does not call history.push on prevented event.", () => { @@ -147,7 +149,7 @@ describe(" click events", () => { button: 0 }); - expect(memoryHistory.push).toBeCalledTimes(0); + expect(pushSpy).toBeCalledTimes(0); }); it("does not call history.push target not specifying 'self'", () => { @@ -168,12 +170,10 @@ describe(" click events", () => { button: 0 }); - expect(memoryHistory.push).toBeCalledTimes(0); + expect(pushSpy).toBeCalledTimes(0); }); it("prevents the default event handler if an error occurs", () => { - const memoryHistory = createMemoryHistory(); - memoryHistory.push = jest.fn(); const error = new Error(); const clickHandler = () => { throw error; @@ -205,6 +205,6 @@ describe(" click events", () => { console.error.mockRestore(); expect(clickHandler).toThrow(error); expect(mockPreventDefault).toHaveBeenCalled(); - expect(memoryHistory.push).toBeCalledTimes(0); + expect(pushSpy).toBeCalledTimes(0); }); });