Skip to content

Commit aae4b2c

Browse files
brophdawg11brophdawg11
authored
Stop inserting null placeholders for non-executed loaders in staticHandler.query (#13223)
1 parent c40f786 commit aae4b2c

File tree

6 files changed

+22
-69
lines changed

6 files changed

+22
-69
lines changed

Diff for: ‎.changeset/smart-ads-doubt.md

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"react-router": patch
3+
---
4+
5+
Do not automatically add `null` to `staticHandler.query()` `context.loaderData` if routes do not have loaders
6+
7+
- This was a Remix v2 implementation detail inadvertently left in for React Router v7
8+
- Now that we allow returning `undefined` from loaders, our prior check of `loaderData[routeId] !== undefined` was no longer sufficient and was changed to a `routeId in loaderData` check - these `null` values can cause issues for this new check
9+
- ⚠️ This could be a "breaking bug fix" for you if you are doing manual SSR with `createStaticHandler()`/`<StaticRouterProvider>`, and using `context.loaderData` to control `<RouterProvider>` hydration behavior on the client

Diff for: ‎packages/react-router/__tests__/dom/data-static-router-test.tsx

+1-4
Original file line numberDiff line numberDiff line change
@@ -901,10 +901,7 @@ describe("A <StaticRouterProvider>", () => {
901901

902902
let expectedJsonString = JSON.stringify(
903903
JSON.stringify({
904-
loaderData: {
905-
0: null,
906-
"0-0": null,
907-
},
904+
loaderData: {},
908905
actionData: null,
909906
errors: null,
910907
})

Diff for: ‎packages/react-router/__tests__/router/lazy-test.ts

+5-15
Original file line numberDiff line numberDiff line change
@@ -2930,9 +2930,7 @@ describe("lazily loaded route modules", () => {
29302930
!(context instanceof Response),
29312931
"Expected a StaticContext instance"
29322932
);
2933-
expect(context.loaderData).toEqual({
2934-
root: null,
2935-
});
2933+
expect(context.loaderData).toEqual({});
29362934
expect(context.errors).toEqual({
29372935
lazy: new Error("LAZY LOADER ERROR"),
29382936
});
@@ -2969,9 +2967,7 @@ describe("lazily loaded route modules", () => {
29692967
!(context instanceof Response),
29702968
"Expected a StaticContext instance"
29712969
);
2972-
expect(context.loaderData).toEqual({
2973-
root: null,
2974-
});
2970+
expect(context.loaderData).toEqual({});
29752971
expect(context.errors).toEqual({
29762972
lazy: new Error("LAZY LOADER ERROR"),
29772973
});
@@ -3006,9 +3002,7 @@ describe("lazily loaded route modules", () => {
30063002
!(context instanceof Response),
30073003
"Expected a StaticContext instance"
30083004
);
3009-
expect(context.loaderData).toEqual({
3010-
root: null,
3011-
});
3005+
expect(context.loaderData).toEqual({});
30123006
expect(context.errors).toEqual({
30133007
root: new Error("LAZY LOADER ERROR"),
30143008
});
@@ -3045,9 +3039,7 @@ describe("lazily loaded route modules", () => {
30453039
!(context instanceof Response),
30463040
"Expected a StaticContext instance"
30473041
);
3048-
expect(context.loaderData).toEqual({
3049-
root: null,
3050-
});
3042+
expect(context.loaderData).toEqual({});
30513043
expect(context.errors).toEqual({
30523044
root: new Error("LAZY LOADER ERROR"),
30533045
});
@@ -3084,9 +3076,7 @@ describe("lazily loaded route modules", () => {
30843076
!(context instanceof Response),
30853077
"Expected a StaticContext instance"
30863078
);
3087-
expect(context.loaderData).toEqual({
3088-
root: null,
3089-
});
3079+
expect(context.loaderData).toEqual({});
30903080
expect(context.errors).toEqual({
30913081
root: new Error("LAZY LOADER ERROR"),
30923082
});

Diff for: ‎packages/react-router/__tests__/router/ssr-test.ts

+2-7
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ describe("ssr", () => {
186186
});
187187
});
188188

189-
it("should fill in null loaderData values for routes without loaders", async () => {
189+
it("should not fill in null loaderData values for routes without loaders", async () => {
190190
let { query } = createStaticHandler([
191191
{
192192
id: "root",
@@ -215,10 +215,7 @@ describe("ssr", () => {
215215
let context = await query(createRequest("/none"));
216216
expect(context).toMatchObject({
217217
actionData: null,
218-
loaderData: {
219-
root: null,
220-
none: null,
221-
},
218+
loaderData: {},
222219
errors: null,
223220
location: { pathname: "/none" },
224221
});
@@ -228,9 +225,7 @@ describe("ssr", () => {
228225
expect(context).toMatchObject({
229226
actionData: null,
230227
loaderData: {
231-
root: null,
232228
a: "A",
233-
b: null,
234229
},
235230
errors: null,
236231
location: { pathname: "/a/b" },

Diff for: ‎packages/react-router/__tests__/server-runtime/server-test.ts

+4-28
Original file line numberDiff line numberDiff line change
@@ -1339,10 +1339,7 @@ describe("shared server runtime", () => {
13391339
let context = calls[0][3].staticHandlerContext as StaticHandlerContext;
13401340
expect(context.errors).toBeTruthy();
13411341
expect(context.errors!.root.status).toBe(400);
1342-
expect(context.loaderData).toEqual({
1343-
root: null,
1344-
"routes/test": null,
1345-
});
1342+
expect(context.loaderData).toEqual({});
13461343
});
13471344

13481345
test("thrown action responses bubble up for index routes", async () => {
@@ -1386,10 +1383,7 @@ describe("shared server runtime", () => {
13861383
let context = calls[0][3].staticHandlerContext as StaticHandlerContext;
13871384
expect(context.errors).toBeTruthy();
13881385
expect(context.errors!.root.status).toBe(400);
1389-
expect(context.loaderData).toEqual({
1390-
root: null,
1391-
"routes/_index": null,
1392-
});
1386+
expect(context.loaderData).toEqual({});
13931387
});
13941388

13951389
test("thrown action responses catch deep", async () => {
@@ -1435,7 +1429,6 @@ describe("shared server runtime", () => {
14351429
expect(context.errors!["routes/test"].status).toBe(400);
14361430
expect(context.loaderData).toEqual({
14371431
root: "root",
1438-
"routes/test": null,
14391432
});
14401433
});
14411434

@@ -1482,7 +1475,6 @@ describe("shared server runtime", () => {
14821475
expect(context.errors!["routes/_index"].status).toBe(400);
14831476
expect(context.loaderData).toEqual({
14841477
root: "root",
1485-
"routes/_index": null,
14861478
});
14871479
});
14881480

@@ -1537,8 +1529,6 @@ describe("shared server runtime", () => {
15371529
expect(context.errors!["routes/__layout"].data).toBe("action");
15381530
expect(context.loaderData).toEqual({
15391531
root: "root",
1540-
"routes/__layout": null,
1541-
"routes/__layout/test": null,
15421532
});
15431533
});
15441534

@@ -1593,8 +1583,6 @@ describe("shared server runtime", () => {
15931583
expect(context.errors!["routes/__layout"].data).toBe("action");
15941584
expect(context.loaderData).toEqual({
15951585
root: "root",
1596-
"routes/__layout": null,
1597-
"routes/__layout/index": null,
15981586
});
15991587
});
16001588

@@ -1728,10 +1716,7 @@ describe("shared server runtime", () => {
17281716
expect(context.errors!.root).toBeInstanceOf(Error);
17291717
expect(context.errors!.root.message).toBe("Unexpected Server Error");
17301718
expect(context.errors!.root.stack).toBeUndefined();
1731-
expect(context.loaderData).toEqual({
1732-
root: null,
1733-
"routes/test": null,
1734-
});
1719+
expect(context.loaderData).toEqual({});
17351720
});
17361721

17371722
test("action errors bubble up for index routes", async () => {
@@ -1777,10 +1762,7 @@ describe("shared server runtime", () => {
17771762
expect(context.errors!.root).toBeInstanceOf(Error);
17781763
expect(context.errors!.root.message).toBe("Unexpected Server Error");
17791764
expect(context.errors!.root.stack).toBeUndefined();
1780-
expect(context.loaderData).toEqual({
1781-
root: null,
1782-
"routes/_index": null,
1783-
});
1765+
expect(context.loaderData).toEqual({});
17841766
});
17851767

17861768
test("action errors catch deep", async () => {
@@ -1830,7 +1812,6 @@ describe("shared server runtime", () => {
18301812
expect(context.errors!["routes/test"].stack).toBeUndefined();
18311813
expect(context.loaderData).toEqual({
18321814
root: "root",
1833-
"routes/test": null,
18341815
});
18351816
});
18361817

@@ -1881,7 +1862,6 @@ describe("shared server runtime", () => {
18811862
expect(context.errors!["routes/_index"].stack).toBeUndefined();
18821863
expect(context.loaderData).toEqual({
18831864
root: "root",
1884-
"routes/_index": null,
18851865
});
18861866
});
18871867

@@ -1940,8 +1920,6 @@ describe("shared server runtime", () => {
19401920
expect(context.errors!["routes/__layout"].stack).toBeUndefined();
19411921
expect(context.loaderData).toEqual({
19421922
root: "root",
1943-
"routes/__layout": null,
1944-
"routes/__layout/test": null,
19451923
});
19461924
});
19471925

@@ -2000,8 +1978,6 @@ describe("shared server runtime", () => {
20001978
expect(context.errors!["routes/__layout"].stack).toBeUndefined();
20011979
expect(context.loaderData).toEqual({
20021980
root: "root",
2003-
"routes/__layout": null,
2004-
"routes/__layout/index": null,
20051981
});
20061982
});
20071983

Diff for: ‎packages/react-router/lib/router/router.ts

+1-15
Original file line numberDiff line numberDiff line change
@@ -4165,11 +4165,7 @@ export function createStaticHandler(
41654165
if (!dataStrategy && !dsMatches.some((m) => m.shouldLoad)) {
41664166
return {
41674167
matches,
4168-
// Add a null for all matched routes for proper revalidation on the client
4169-
loaderData: matches.reduce(
4170-
(acc, m) => Object.assign(acc, { [m.route.id]: null }),
4171-
{}
4172-
),
4168+
loaderData: {},
41734169
errors:
41744170
pendingActionResult && isErrorResult(pendingActionResult[1])
41754171
? {
@@ -4202,16 +4198,6 @@ export function createStaticHandler(
42024198
skipLoaderErrorBubbling
42034199
);
42044200

4205-
// Add a null for any non-loader matches for proper revalidation on the client
4206-
let executedLoaders = new Set<string>(
4207-
dsMatches.filter((m) => m.shouldLoad).map((match) => match.route.id)
4208-
);
4209-
matches.forEach((match) => {
4210-
if (!executedLoaders.has(match.route.id)) {
4211-
handlerContext.loaderData[match.route.id] = null;
4212-
}
4213-
});
4214-
42154201
return {
42164202
...handlerContext,
42174203
matches,

0 commit comments

Comments
 (0)