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- authored Aug 27, 2024
1 parent ae99a1c commit 5651176
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-dom/__tests__/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
4 changes: 2 additions & 2 deletions packages/router/__tests__/navigation-blocking-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,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 @@ -455,7 +455,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/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,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;

// Initialize the router, all side effects should be kicked off from here.
// Implemented as a Fluent API for ease of:
Expand All @@ -1045,8 +1045,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 @@ -1068,7 +1069,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 @@ -1082,8 +1085,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 5651176

Please sign in to comment.