Skip to content

Commit 4e427a3

Browse files
authored
Short circuit dataStrategy post processing on aborted requests (#13521)
1 parent 7583dc7 commit 4e427a3

File tree

4 files changed

+74
-2
lines changed

4 files changed

+74
-2
lines changed

.changeset/twenty-snakes-love.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"react-router": patch
3+
---
4+
5+
Short circuit post-processing on aborted `dataStrategy` requests
6+
7+
- This resolves non-user-facing console errors of the form `Cannot read properties of undefined (reading 'result')`

integration/single-fetch-test.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3263,6 +3263,63 @@ test.describe("single-fetch", () => {
32633263
expect(urls[0].endsWith("/fetch.data?_routes=routes%2Ffetch")).toBe(true);
32643264
expect(urls[1].endsWith("/parent/b.data")).toBe(true);
32653265
});
3266+
3267+
test("Aborted fetcher loads don't cause console errors", async ({
3268+
page,
3269+
}) => {
3270+
let fixture = await createFixture({
3271+
files: {
3272+
...files,
3273+
"app/routes/_index.tsx": js`
3274+
import { Form, redirect, useFetcher } from "react-router";
3275+
3276+
export function action() {
3277+
return redirect("/other");
3278+
}
3279+
3280+
export default function Page() {
3281+
const fetcher = useFetcher();
3282+
const isPending = fetcher.state !== "idle";
3283+
3284+
return (
3285+
<>
3286+
<button id="fetch" onClick={() => fetcher.load("/fetch")}>
3287+
{isPending ? "Loading..." : "First load data"}
3288+
</button>
3289+
<Form method="POST">
3290+
<button type="submit">Then submit before load ends</button>
3291+
</Form>
3292+
</>
3293+
);
3294+
}
3295+
`,
3296+
"app/routes/other.tsx": js`
3297+
export default function Component() {
3298+
return <p id="other">Other</p>;
3299+
}
3300+
`,
3301+
"app/routes/fetch.tsx": js`
3302+
export async function loader() {
3303+
await new Promise((r) => setTimeout(r, 10000));
3304+
return 'nope';
3305+
}
3306+
`,
3307+
},
3308+
});
3309+
let appFixture = await createAppFixture(fixture);
3310+
let app = new PlaywrightFixture(appFixture, page);
3311+
3312+
// Capture console logs and uncaught errors
3313+
let msgs: string[] = [];
3314+
page.on("console", (msg) => msgs.push(msg.text()));
3315+
page.on("pageerror", (error) => msgs.push(error.message));
3316+
3317+
await app.goto("/", true);
3318+
app.clickElement("#fetch");
3319+
await app.clickSubmitButton("/?index");
3320+
await page.waitForSelector("#other");
3321+
expect(msgs).toEqual([]);
3322+
});
32663323
});
32673324

32683325
test.describe("prefetching", () => {

packages/react-router/__tests__/router/revalidate-test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,8 +1112,12 @@ describe("router.revalidate", () => {
11121112
revalidation: "idle",
11131113
loaderData: { root: "ROOT**" },
11141114
});
1115+
// The interrupted revalidation hooks into the next completed navigation
1116+
// so it reflects the end state value
11151117
expect(revalidationValue).toBe("ROOT**");
1116-
expect(navigationValue).toBe("ROOT**");
1118+
// The interim navigation gets interrupted and ends while the router still
1119+
// reflects the original value
1120+
expect(navigationValue).toBe("ROOT");
11171121
expect(navigationValue2).toBe("ROOT**");
11181122

11191123
// no-op
@@ -1126,7 +1130,7 @@ describe("router.revalidate", () => {
11261130
loaderData: { root: "ROOT**" },
11271131
});
11281132
expect(revalidationValue).toBe("ROOT**");
1129-
expect(navigationValue).toBe("ROOT**");
1133+
expect(navigationValue).toBe("ROOT");
11301134
expect(navigationValue2).toBe("ROOT**");
11311135
});
11321136
});

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2864,6 +2864,10 @@ export function createRouter(init: RouterInit): Router {
28642864
return dataResults;
28652865
}
28662866

2867+
if (request.signal.aborted) {
2868+
return dataResults;
2869+
}
2870+
28672871
for (let [routeId, result] of Object.entries(results)) {
28682872
if (isRedirectDataStrategyResult(result)) {
28692873
let response = result.result as Response;

0 commit comments

Comments
 (0)