Skip to content

Avoid fetcher 404 when lazy route discovery is racing against a navigation #13564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/purple-boats-bake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"react-router": patch
---

Avoid initial fetcher execution 404 error when Lazy Route Discovery is interrupted by a navigation
73 changes: 73 additions & 0 deletions integration/fetcher-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -525,3 +525,76 @@ test.describe("fetcher aborts and adjacent forms", () => {
await page.waitForSelector("#idle", { timeout: 2000 });
});
});

test.describe("fetcher lazy route discovery", () => {
let fixture: Fixture;
let appFixture: AppFixture;

test.afterAll(() => {
appFixture.close();
});

test("skips revalidation of initial load fetchers performing lazy route discovery", async ({
page,
}) => {
fixture = await createFixture({
files: {
"app/routes/parent.tsx": js`
import * as React from "react";
import { useFetcher, useNavigate, Outlet } from "react-router";

export default function Index() {
const fetcher = useFetcher();
const navigate = useNavigate();

React.useEffect(() => {
fetcher.load('/api');
}, []);

React.useEffect(() => {
navigate('/parent/child');
}, []);

return (
<>
<h1>Parent</h1>
{fetcher.data ?
<pre data-fetcher>{fetcher.data}</pre> :
null}
<Outlet/>
</>
);
}
`,
"app/routes/parent.child.tsx": js`
export default function Index() {
return <h2>Child</h2>;
}
`,
"app/routes/api.tsx": js`
export async function loader() {
return "FETCHED!"
}
`,
},
});

// Slow down the fetcher discovery a tiny bit so it doesn't resolve prior
// to the navigation
page.route(/\/__manifest/, async (route) => {
console.log(route.request().url());
if (route.request().url().includes(encodeURIComponent("/api"))) {
await new Promise((r) => setTimeout(r, 100));
}
route.continue();
});

appFixture = await createAppFixture(fixture);
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/parent");
await page.waitForSelector("h2", { timeout: 3000 });
await expect(page.locator("h2")).toHaveText("Child");
await page.waitForSelector("[data-fetcher]", { timeout: 3000 });
await expect(page.locator("[data-fetcher]")).toHaveText("FETCHED!");
});
});
20 changes: 14 additions & 6 deletions packages/react-router/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1995,6 +1995,7 @@ export function createRouter(init: RouterInit): Router {
fetchRedirectIds,
routesToUse,
basename,
init.patchRoutesOnNavigation != null,
pendingActionResult
);

Expand Down Expand Up @@ -2441,6 +2442,7 @@ export function createRouter(init: RouterInit): Router {
fetchRedirectIds,
routesToUse,
basename,
init.patchRoutesOnNavigation != null,
[match.route.id, actionResult]
);

Expand Down Expand Up @@ -4583,6 +4585,7 @@ function getMatchesToLoad(
fetchRedirectIds: Set<string>,
routesToUse: AgnosticDataRouteObject[],
basename: string | undefined,
hasPatchRoutesOnNavigation: boolean,
pendingActionResult?: PendingActionResult
): {
dsMatches: DataStrategyMatch[];
Expand Down Expand Up @@ -4717,13 +4720,23 @@ function getMatchesToLoad(
return;
}

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

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

let fetchController = new AbortController();
Expand All @@ -4768,11 +4780,7 @@ function getMatchesToLoad(
lazyRoutePropertiesToSkip,
scopedContext
);
} else if (
fetcher &&
fetcher.state !== "idle" &&
fetcher.data === undefined
) {
} else if (isMidInitialLoad) {
if (isRevalidationRequired) {
// If the fetcher hasn't ever completed loading yet, then this isn't a
// revalidation, it would just be a brand new load if an explicit
Expand Down
Loading