Skip to content

Commit

Permalink
fix: Wait for restore url navigation to complete before proceeding (#…
Browse files Browse the repository at this point in the history
…11930)

Co-authored-by: Artur Signell <artur@vaadin.com>
  • Loading branch information
brophdawg11 and Artur- committed Sep 5, 2024
1 parent 3908497 commit 5ff506e
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/serious-news-kick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/router": patch
---

Fix blocker usage when `blocker.proceed` is called quickly/syncronously
1 change: 1 addition & 0 deletions contributors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
- Armanio
- arnassavickas
- aroyan
- Artur-
- ashusnapx
- avipatel97
- awreese
Expand Down
19 changes: 14 additions & 5 deletions packages/react-router/__tests__/dom/use-blocker-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ import {

type Router = ReturnType<typeof createMemoryRouter>;

const LOADER_LATENCY_MS = 100;
const LOADER_LATENCY_MS = 200;

async function slowLoader() {
await sleep(LOADER_LATENCY_MS);
await sleep(LOADER_LATENCY_MS / 2);
return json(null);
}

Expand Down Expand Up @@ -1084,14 +1084,23 @@ describe("navigation blocking with useBlocker", () => {
act(() => {
click(node.querySelector("[data-action='back']"));
});
act(() => {
expect(node.innerHTML).toContain("<h1>Contact</h1>");
await act(async () => {
click(node.querySelector("[data-action='proceed']"));
expect([...router.state.blockers.values()][0]).toEqual({
state: "proceeding",
proceed: undefined,
reset: undefined,
location: expect.any(Object),
});
await sleep(LOADER_LATENCY_MS);
});
expect(node.innerHTML).toContain("<h1>About</h1>");
expect(blocker).toEqual({
state: "proceeding",
state: "unblocked",
proceed: undefined,
reset: undefined,
location: expect.any(Object),
location: undefined,
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ describe("navigation blocking", () => {
router.getBlocker("KEY", fn);
await router.navigate(-1);
router.getBlocker("KEY", fn).proceed?.();
await sleep(LOADER_LATENCY_MS);
await sleep(LOADER_LATENCY_MS + 10);
expect(router.getBlocker("KEY", fn)).toEqual({
state: "unblocked",
proceed: undefined,
Expand All @@ -456,7 +456,7 @@ describe("navigation blocking", () => {
router.getBlocker("KEY", fn);
await router.navigate(-1);
router.getBlocker("KEY", fn).proceed?.();
await sleep(LOADER_LATENCY_MS);
await sleep(LOADER_LATENCY_MS + 10);
expect(router.state.location.pathname).toBe("/about");
});
});
Expand Down
17 changes: 11 additions & 6 deletions packages/react-router/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1022,7 +1022,7 @@ export function createRouter(init: RouterInit): Router {

// Flag to ignore the next history update, so we can revert the URL change on
// a POP navigation that was blocked by the user without touching router state
let ignoreNextHistoryUpdate = false;
let unblockBlockerHistoryUpdate: (() => void) | undefined = undefined;

let pendingRevalidationDfd: ReturnType<typeof createDeferred<void>> | null =
null;
Expand All @@ -1037,8 +1037,9 @@ export function createRouter(init: RouterInit): Router {
({ action: historyAction, location, delta }) => {
// Ignore this event if it was just us resetting the URL from a
// blocked POP navigation
if (ignoreNextHistoryUpdate) {
ignoreNextHistoryUpdate = false;
if (unblockBlockerHistoryUpdate) {
unblockBlockerHistoryUpdate();
unblockBlockerHistoryUpdate = undefined;
return;
}

Expand All @@ -1060,7 +1061,9 @@ export function createRouter(init: RouterInit): Router {

if (blockerKey && delta != null) {
// Restore the URL to match the current UI, but don't update router state
ignoreNextHistoryUpdate = true;
let nextHistoryUpdatePromise = new Promise<void>((resolve) => {
unblockBlockerHistoryUpdate = resolve;
});
init.history.go(delta * -1);

// Put the blocker into a blocked state
Expand All @@ -1074,8 +1077,10 @@ export function createRouter(init: RouterInit): Router {
reset: undefined,
location,
});
// Re-do the same POP navigation we just blocked
init.history.go(delta);
// Re-do the same POP navigation we just blocked, after the url
// restoration is also complete. See:
// https://github.com/remix-run/react-router/issues/11613
nextHistoryUpdatePromise.then(() => init.history.go(delta));
},
reset() {
let blockers = new Map(state.blockers);
Expand Down

0 comments on commit 5ff506e

Please sign in to comment.