Skip to content

Commit 0c08bd0

Browse files
authored
Avoid fetcher 404 when lazy route discovery is racing against a navigation (#13564)
1 parent 98128db commit 0c08bd0

File tree

3 files changed

+92
-6
lines changed

3 files changed

+92
-6
lines changed

.changeset/purple-boats-bake.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"react-router": patch
3+
---
4+
5+
Avoid initial fetcher execution 404 error when Lazy Route Discovery is interrupted by a navigation

integration/fetcher-test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,3 +525,76 @@ test.describe("fetcher aborts and adjacent forms", () => {
525525
await page.waitForSelector("#idle", { timeout: 2000 });
526526
});
527527
});
528+
529+
test.describe("fetcher lazy route discovery", () => {
530+
let fixture: Fixture;
531+
let appFixture: AppFixture;
532+
533+
test.afterAll(() => {
534+
appFixture.close();
535+
});
536+
537+
test("skips revalidation of initial load fetchers performing lazy route discovery", async ({
538+
page,
539+
}) => {
540+
fixture = await createFixture({
541+
files: {
542+
"app/routes/parent.tsx": js`
543+
import * as React from "react";
544+
import { useFetcher, useNavigate, Outlet } from "react-router";
545+
546+
export default function Index() {
547+
const fetcher = useFetcher();
548+
const navigate = useNavigate();
549+
550+
React.useEffect(() => {
551+
fetcher.load('/api');
552+
}, []);
553+
554+
React.useEffect(() => {
555+
navigate('/parent/child');
556+
}, []);
557+
558+
return (
559+
<>
560+
<h1>Parent</h1>
561+
{fetcher.data ?
562+
<pre data-fetcher>{fetcher.data}</pre> :
563+
null}
564+
<Outlet/>
565+
</>
566+
);
567+
}
568+
`,
569+
"app/routes/parent.child.tsx": js`
570+
export default function Index() {
571+
return <h2>Child</h2>;
572+
}
573+
`,
574+
"app/routes/api.tsx": js`
575+
export async function loader() {
576+
return "FETCHED!"
577+
}
578+
`,
579+
},
580+
});
581+
582+
// Slow down the fetcher discovery a tiny bit so it doesn't resolve prior
583+
// to the navigation
584+
page.route(/\/__manifest/, async (route) => {
585+
console.log(route.request().url());
586+
if (route.request().url().includes(encodeURIComponent("/api"))) {
587+
await new Promise((r) => setTimeout(r, 100));
588+
}
589+
route.continue();
590+
});
591+
592+
appFixture = await createAppFixture(fixture);
593+
let app = new PlaywrightFixture(appFixture, page);
594+
await app.goto("/parent");
595+
await page.waitForSelector("h2", { timeout: 3000 });
596+
await expect(page.locator("h2")).toHaveText("Child");
597+
await page.waitForSelector("[data-fetcher]", { timeout: 3000 });
598+
await expect(page.locator("[data-fetcher]")).toHaveText("FETCHED!");
599+
});
600+
});

packages/react-router/lib/router/router.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1995,6 +1995,7 @@ export function createRouter(init: RouterInit): Router {
19951995
fetchRedirectIds,
19961996
routesToUse,
19971997
basename,
1998+
init.patchRoutesOnNavigation != null,
19981999
pendingActionResult
19992000
);
20002001

@@ -2441,6 +2442,7 @@ export function createRouter(init: RouterInit): Router {
24412442
fetchRedirectIds,
24422443
routesToUse,
24432444
basename,
2445+
init.patchRoutesOnNavigation != null,
24442446
[match.route.id, actionResult]
24452447
);
24462448

@@ -4583,6 +4585,7 @@ function getMatchesToLoad(
45834585
fetchRedirectIds: Set<string>,
45844586
routesToUse: AgnosticDataRouteObject[],
45854587
basename: string | undefined,
4588+
hasPatchRoutesOnNavigation: boolean,
45864589
pendingActionResult?: PendingActionResult
45874590
): {
45884591
dsMatches: DataStrategyMatch[];
@@ -4717,13 +4720,23 @@ function getMatchesToLoad(
47174720
return;
47184721
}
47194722

4723+
let fetcher = state.fetchers.get(key);
4724+
let isMidInitialLoad =
4725+
fetcher && fetcher.state !== "idle" && fetcher.data === undefined;
47204726
let fetcherMatches = matchRoutes(routesToUse, f.path, basename);
47214727

47224728
// If the fetcher path no longer matches, push it in with null matches so
47234729
// we can trigger a 404 in callLoadersAndMaybeResolveData. Note this is
47244730
// currently only a use-case for Remix HMR where the route tree can change
47254731
// at runtime and remove a route previously loaded via a fetcher
47264732
if (!fetcherMatches) {
4733+
// If this fetcher is still in it's initial loading state, then this is
4734+
// most likely not a 404 and the fetcher is still in the middle of lazy
4735+
// route discovery so we can just skip revalidation and let it finish
4736+
// it's initial load
4737+
if (hasPatchRoutesOnNavigation && isMidInitialLoad) {
4738+
return;
4739+
}
47274740
revalidatingFetchers.push({
47284741
key,
47294742
routeId: f.routeId,
@@ -4744,7 +4757,6 @@ function getMatchesToLoad(
47444757
// Revalidating fetchers are decoupled from the route matches since they
47454758
// load from a static href. They revalidate based on explicit revalidation
47464759
// (submission, useRevalidator, or X-Remix-Revalidate)
4747-
let fetcher = state.fetchers.get(key);
47484760
let fetcherMatch = getTargetMatch(fetcherMatches, f.path);
47494761

47504762
let fetchController = new AbortController();
@@ -4768,11 +4780,7 @@ function getMatchesToLoad(
47684780
lazyRoutePropertiesToSkip,
47694781
scopedContext
47704782
);
4771-
} else if (
4772-
fetcher &&
4773-
fetcher.state !== "idle" &&
4774-
fetcher.data === undefined
4775-
) {
4783+
} else if (isMidInitialLoad) {
47764784
if (isRevalidationRequired) {
47774785
// If the fetcher hasn't ever completed loading yet, then this isn't a
47784786
// revalidation, it would just be a brand new load if an explicit

0 commit comments

Comments
 (0)