Skip to content

Commit 32aa8ae

Browse files
brophdawg11brophdawg11
committed
Merge branch 'dev' into brophdawg11/client-middleware-return
2 parents f79b040 + 166fd94 commit 32aa8ae

File tree

5 files changed

+68
-33
lines changed

5 files changed

+68
-33
lines changed

‎.changeset/cold-maps-wave.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+
Bubble client pre-next middleware error to the shallowest ancestor that needs to load, not strictly the shallowest ancestor with a loader

‎.github/workflows/release.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ jobs:
2121
outputs:
2222
published_packages: ${{ steps.changesets.outputs.publishedPackages }}
2323
published: ${{ steps.changesets.outputs.published }}
24+
permissions:
25+
id-token: write # enable generation of an ID token for publishing
2426
steps:
2527
- name: ⬇️ Checkout repo
2628
uses: actions/checkout@v4
@@ -62,6 +64,7 @@ jobs:
6264
createGithubReleases: false
6365
env:
6466
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
67+
NPM_CONFIG_PROVENANCE: true
6568
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
6669

6770
find_package_version:

‎integration/middleware-test.ts‎

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ test.describe("Middleware", () => {
490490
return <h1>Should not see me</h1>
491491
}
492492
export function ErrorBoundary({ error }) {
493-
return <h1>{error.message}</h1>
493+
return <h1 data-error>{error.message}</h1>
494494
}
495495
`,
496496
},
@@ -508,10 +508,8 @@ test.describe("Middleware", () => {
508508
await page.waitForSelector('a:has-text("Link")');
509509

510510
(await page.getByRole("link"))?.click();
511-
// Bubbles to the root error boundary in SPA mode because every route has
512-
// a loader to load the client assets
513-
await page.waitForSelector('h1:has-text("Application Error")');
514-
await page.waitForSelector('pre:has-text("Error: broken!")');
511+
await page.waitForSelector("[data-error]");
512+
expect(await page.innerText("[data-error]")).toBe("broken!");
515513

516514
appFixture.close();
517515
});
@@ -557,7 +555,7 @@ test.describe("Middleware", () => {
557555
export function ErrorBoundary({ loaderData, error }) {
558556
return (
559557
<>
560-
<h1>{error.message}</h1>
558+
<h1 data-error>{error.message}</h1>
561559
<pre>{loaderData ?? 'empty'}</pre>
562560
</>
563561
);
@@ -578,7 +576,8 @@ test.describe("Middleware", () => {
578576
await page.waitForSelector('a:has-text("Link")');
579577

580578
(await page.getByRole("link"))?.click();
581-
await page.waitForSelector('h1:has-text("broken!")');
579+
await page.waitForSelector("[data-error]");
580+
expect(await page.innerText("[data-error]")).toBe("broken!");
582581
expect(await page.innerText("pre")).toBe("empty");
583582

584583
appFixture.close();
@@ -1120,7 +1119,6 @@ test.describe("Middleware", () => {
11201119
}
11211120
`,
11221121
"app/routes/broken.tsx": js`
1123-
import { useRouteError } from 'react-router'
11241122
export const unstable_clientMiddleware = [
11251123
async ({ request, context }, next) => {
11261124
throw new Error('broken!')
@@ -1129,8 +1127,8 @@ test.describe("Middleware", () => {
11291127
export default function Component() {
11301128
return <h1>Should not see me</h1>
11311129
}
1132-
export function ErrorBoundary() {
1133-
return <h1>{useRouteError().message}</h1>
1130+
export function ErrorBoundary({ error }) {
1131+
return <h1 data-error>{error.message}</h1>
11341132
}
11351133
`,
11361134
},
@@ -1148,10 +1146,8 @@ test.describe("Middleware", () => {
11481146
await page.waitForSelector('a:has-text("Link")');
11491147

11501148
(await page.getByRole("link"))?.click();
1151-
// Bubbles to the root error boundary in SPA mode because every route has
1152-
// a loader to load the client assets
1153-
await page.waitForSelector('h1:has-text("Application Error")');
1154-
await page.waitForSelector('pre:has-text("Error: broken!")');
1149+
await page.waitForSelector("[data-error]");
1150+
expect(await page.innerText("[data-error]")).toBe("broken!");
11551151

11561152
appFixture.close();
11571153
});
@@ -1196,7 +1192,7 @@ test.describe("Middleware", () => {
11961192
export function ErrorBoundary({ loaderData, error }) {
11971193
return (
11981194
<>
1199-
<h1>{error.message}</h1>
1195+
<h1 data-error>{error.message}</h1>
12001196
<pre>{loaderData ?? 'empty'}</pre>
12011197
</>
12021198
);
@@ -1217,7 +1213,8 @@ test.describe("Middleware", () => {
12171213
await page.waitForSelector('a:has-text("Link")');
12181214

12191215
(await page.getByRole("link"))?.click();
1220-
await page.waitForSelector('h1:has-text("broken!")');
1216+
await page.waitForSelector("[data-error]");
1217+
expect(await page.innerText("[data-error]")).toBe("broken!");
12211218
expect(await page.innerText("pre")).toBe("empty");
12221219

12231220
appFixture.close();
@@ -1987,7 +1984,7 @@ test.describe("Middleware", () => {
19871984
return <h1>Should not see me</h1>
19881985
}
19891986
export function ErrorBoundary({ error }) {
1990-
return <h1>{error.message}</h1>
1987+
return <h1 data-error>{error.message}</h1>
19911988
}
19921989
`,
19931990
},
@@ -2002,7 +1999,7 @@ test.describe("Middleware", () => {
20021999

20032000
let app = new PlaywrightFixture(appFixture, page);
20042001
await app.goto("/broken");
2005-
expect(await page.innerText("h1")).toBe("broken!");
2002+
expect(await page.innerText("[data-error]")).toBe("broken!");
20062003
expect(errors).toEqual([
20072004
["handleError", "GET", "/broken", new Error("broken!")],
20082005
]);
@@ -2049,7 +2046,7 @@ test.describe("Middleware", () => {
20492046
return <h1>Should not see me</h1>
20502047
}
20512048
export function ErrorBoundary({ error }) {
2052-
return <h1>{error.message}</h1>
2049+
return <h1 data-error>{error.message}</h1>
20532050
}
20542051
`,
20552052
},
@@ -2066,8 +2063,8 @@ test.describe("Middleware", () => {
20662063
await app.goto("/");
20672064

20682065
(await page.$('a[href="/broken"]'))?.click();
2069-
await page.waitForSelector("h1");
2070-
expect(await page.innerText("h1")).toBe("broken!");
2066+
await page.waitForSelector("[data-error]");
2067+
expect(await page.innerText("[data-error]")).toBe("broken!");
20712068
expect(errors).toEqual([
20722069
["handleError", "GET", "/broken.data", new Error("broken!")],
20732070
]);
@@ -2117,7 +2114,7 @@ test.describe("Middleware", () => {
21172114
export function ErrorBoundary({ error, loaderData }) {
21182115
return (
21192116
<>
2120-
<h1>{error.message}</h1>
2117+
<h1 data-error>{error.message}</h1>
21212118
<pre>{loaderData ?? 'empty'}</pre>
21222119
</>
21232120
);
@@ -2135,7 +2132,7 @@ test.describe("Middleware", () => {
21352132

21362133
let app = new PlaywrightFixture(appFixture, page);
21372134
await app.goto("/broken");
2138-
expect(await page.innerText("h1")).toBe("broken!");
2135+
expect(await page.innerText("[data-error]")).toBe("broken!");
21392136
expect(await page.innerText("pre")).toBe("empty");
21402137
expect(errors).toEqual([
21412138
["handleError", "GET", "/broken", new Error("broken!")],
@@ -2186,7 +2183,7 @@ test.describe("Middleware", () => {
21862183
export function ErrorBoundary({ error, loaderData }) {
21872184
return (
21882185
<>
2189-
<h1>{error.message}</h1>
2186+
<h1 data-error>{error.message}</h1>
21902187
<pre>{loaderData ?? 'empty'}</pre>
21912188
</>
21922189
);
@@ -2207,7 +2204,8 @@ test.describe("Middleware", () => {
22072204

22082205
(await page.$('a[href="/broken"]'))?.click();
22092206
await page.waitForSelector("h1");
2210-
expect(await page.innerText("h1")).toBe("broken!");
2207+
await page.waitForSelector("[data-error]");
2208+
expect(await page.innerText("[data-error]")).toBe("broken!");
22112209
expect(await page.innerText("pre")).toBe("empty");
22122210
expect(errors).toEqual([
22132211
["handleError", "GET", "/broken.data", new Error("broken!")],

‎packages/react-router/__tests__/router/context-middleware-test.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1413,6 +1413,17 @@ describe("context/middleware", () => {
14131413
],
14141414
loader: () => "D",
14151415
},
1416+
{
1417+
id: "e",
1418+
path: "e",
1419+
hasErrorBoundary: true,
1420+
unstable_middleware: [
1421+
() => {
1422+
throw new Error("E ERROR");
1423+
},
1424+
],
1425+
loader: () => "E",
1426+
},
14161427
],
14171428
},
14181429
],
@@ -1422,12 +1433,24 @@ describe("context/middleware", () => {
14221433
],
14231434
});
14241435

1436+
// Bubbles to B because it's the initial load and it's loader hasn't run
14251437
await router.navigate("/a/b/c/d");
1426-
14271438
expect(router.state.loaderData).toEqual({});
14281439
expect(router.state.errors).toEqual({
14291440
b: new Error("D ERROR"),
14301441
});
1442+
1443+
// Load data into B
1444+
await router.navigate("/a/b");
1445+
expect(router.state.loaderData).toEqual({ b: "B" });
1446+
expect(router.state.errors).toEqual(null);
1447+
1448+
// B doesn't have to revalidate so we can surface this error at E
1449+
await router.navigate("/a/b/c/e");
1450+
expect(router.state.loaderData).toEqual({ b: "B" });
1451+
expect(router.state.errors).toEqual({
1452+
e: new Error("E ERROR"),
1453+
});
14311454
});
14321455
});
14331456
});

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

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5431,14 +5431,20 @@ function runClientMiddlewarePipeline(
54315431
}),
54325432
);
54335433
} else {
5434-
// We never even got to the handlers, so we've got no data.
5435-
// Find the boundary at or above the source of the middleware
5436-
// error or the highest loader. We can't render any UI below
5437-
// the highest loader since we have no loader data available
5434+
// We never even got to the handlers, so we might not have data for new routes.
5435+
// Find the boundary at or above the source of the middleware error or the
5436+
// highest route that needs to load - we can't render any UI below that since
5437+
// we won't have valid loader data.
5438+
let { matches } = args;
5439+
let maxBoundaryIdx = Math.min(
5440+
// Throwing route
5441+
matches.findIndex((m) => m.route.id === routeId) || 0,
5442+
// or the shallowest route that needs to load data
5443+
matches.findIndex((m) => m.unstable_shouldCallHandler()) || 0,
5444+
);
54385445
let boundaryRouteId = findNearestBoundary(
5439-
args.matches,
5440-
args.matches.find((m) => m.route.id === routeId || m.route.loader)
5441-
?.route.id || routeId,
5446+
matches,
5447+
matches[maxBoundaryIdx].route.id,
54425448
).route.id;
54435449
return Promise.resolve({
54445450
[boundaryRouteId]: { type: "error", result: error },

0 commit comments

Comments
 (0)