Skip to content

Commit

Permalink
Fix bug with preventScrollReset when using concurrent redirecting fet…
Browse files Browse the repository at this point in the history
…chers (#11999)
  • Loading branch information
brophdawg11 authored Sep 13, 2024
1 parent 73fcb9b commit f941ddf
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 6 deletions.
5 changes: 5 additions & 0 deletions .changeset/chatty-impalas-pump.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/router": patch
---

Fix bug with fetchers not persisting `preventScrollReset` through redirects during concurrent fetches
41 changes: 41 additions & 0 deletions packages/router/__tests__/scroll-restoration-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,47 @@ describe("scroll restoration", () => {
expect(t.router.state.restoreScrollPosition).toBe(null);
expect(t.router.state.preventScrollReset).toBe(true);
});

it("persists through concurrent redirecting fetchers", async () => {
let t = setup({
routes: SCROLL_ROUTES,
initialEntries: ["/tasks"],
hydrationData: {
loaderData: {
tasks: "TASKS",
},
},
});

expect(t.router.state.restoreScrollPosition).toBe(false);
expect(t.router.state.preventScrollReset).toBe(false);

let positions = {};
let activeScrollPosition = 0;
t.router.enableScrollRestoration(positions, () => activeScrollPosition);

let nav1 = await t.fetch("/tasks", {
formMethod: "post",
formData: createFormData({}),
preventScrollReset: true,
});

let nav2 = await t.fetch("/tasks", {
formMethod: "post",
formData: createFormData({}),
preventScrollReset: true,
});

let nav3 = await nav1.actions.tasks.redirectReturn("/tasks");
await nav3.loaders.tasks.resolve("TASKS 2");
expect(t.router.state.restoreScrollPosition).toBe(null);
expect(t.router.state.preventScrollReset).toBe(true);

let nav4 = await nav2.actions.tasks.redirectReturn("/tasks");
await nav4.loaders.tasks.resolve("TASKS 3");
expect(t.router.state.restoreScrollPosition).toBe(null);
expect(t.router.state.preventScrollReset).toBe(true);
});
});
});
});
23 changes: 17 additions & 6 deletions packages/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2184,7 +2184,7 @@ export function createRouter(init: RouterInit): Router {

let match = getTargetMatch(matches, path);

pendingPreventScrollReset = (opts && opts.preventScrollReset) === true;
let preventScrollReset = (opts && opts.preventScrollReset) === true;

if (submission && isMutationMethod(submission.formMethod)) {
handleFetcherAction(
Expand All @@ -2195,6 +2195,7 @@ export function createRouter(init: RouterInit): Router {
matches,
fogOfWar.active,
flushSync,
preventScrollReset,
submission
);
return;
Expand All @@ -2211,6 +2212,7 @@ export function createRouter(init: RouterInit): Router {
matches,
fogOfWar.active,
flushSync,
preventScrollReset,
submission
);
}
Expand All @@ -2225,6 +2227,7 @@ export function createRouter(init: RouterInit): Router {
requestMatches: AgnosticDataRouteMatch[],
isFogOfWar: boolean,
flushSync: boolean,
preventScrollReset: boolean,
submission: Submission
) {
interruptActiveLoads();
Expand Down Expand Up @@ -2339,6 +2342,7 @@ export function createRouter(init: RouterInit): Router {
updateFetcherState(key, getLoadingFetcher(submission));
return startRedirectNavigation(fetchRequest, actionResult, false, {
fetcherSubmission: submission,
preventScrollReset,
});
}
}
Expand Down Expand Up @@ -2453,7 +2457,8 @@ export function createRouter(init: RouterInit): Router {
return startRedirectNavigation(
revalidationRequest,
redirect.result,
false
false,
{ preventScrollReset }
);
}

Expand All @@ -2466,7 +2471,8 @@ export function createRouter(init: RouterInit): Router {
return startRedirectNavigation(
revalidationRequest,
redirect.result,
false
false,
{ preventScrollReset }
);
}

Expand Down Expand Up @@ -2534,6 +2540,7 @@ export function createRouter(init: RouterInit): Router {
matches: AgnosticDataRouteMatch[],
isFogOfWar: boolean,
flushSync: boolean,
preventScrollReset: boolean,
submission?: Submission
) {
let existingFetcher = state.fetchers.get(key);
Expand Down Expand Up @@ -2630,7 +2637,9 @@ export function createRouter(init: RouterInit): Router {
return;
} else {
fetchRedirectIds.add(key);
await startRedirectNavigation(fetchRequest, result, false);
await startRedirectNavigation(fetchRequest, result, false, {
preventScrollReset,
});
return;
}
}
Expand Down Expand Up @@ -2673,10 +2682,12 @@ export function createRouter(init: RouterInit): Router {
{
submission,
fetcherSubmission,
preventScrollReset,
replace,
}: {
submission?: Submission;
fetcherSubmission?: Submission;
preventScrollReset?: boolean;
replace?: boolean;
} = {}
) {
Expand Down Expand Up @@ -2757,7 +2768,7 @@ export function createRouter(init: RouterInit): Router {
formAction: location,
},
// Preserve these flags across redirects
preventScrollReset: pendingPreventScrollReset,
preventScrollReset: preventScrollReset || pendingPreventScrollReset,
enableViewTransition: isNavigation
? pendingViewTransitionEnabled
: undefined,
Expand All @@ -2774,7 +2785,7 @@ export function createRouter(init: RouterInit): Router {
// Send fetcher submissions through for shouldRevalidate
fetcherSubmission,
// Preserve these flags across redirects
preventScrollReset: pendingPreventScrollReset,
preventScrollReset: preventScrollReset || pendingPreventScrollReset,
enableViewTransition: isNavigation
? pendingViewTransitionEnabled
: undefined,
Expand Down

0 comments on commit f941ddf

Please sign in to comment.