Skip to content

Commit f13a3d4

Browse files
brophdawg11brophdawg11
committed
Stop inserting null placeholders for non-executed loaders in staticHandler.query
1 parent bb81a9a commit f13a3d4

File tree

6 files changed

+25
-60
lines changed

6 files changed

+25
-60
lines changed

‎.changeset/smart-ads-doubt.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
"react-router": patch
3+
---
4+
5+
Do not automatically add `null` to `staticHandler.query()` `loaderData` if routes do not have loaders
6+
7+
- This was a requirement for Remix v2 because we used `JSON.stringify()` to serialize the `loaderData` of the client and it would strip `undefined` values which would cause issues for client-side router hydration
8+
- Therefore, we had a restriction that you could not return `undefined` from a `loader`/`action`
9+
- We used to check `loaderData[routeId] !== undefined` to see if a given route already had any data or not
10+
- Once we implemented Single fetch and began serializing data via `turbo-stream`, we no longer has this restriction because it can serialize `undefined` correctly
11+
- In React Router v7, we began allowing loaders to return `undefined`
12+
- Therefore, our check of `loaderData[routeId] !== undefined` was no longer valid and we adjusted to a check of `routeId in loaderData`
13+
- This check can fail if we are sticking a null value in `loaderData`, so we have to remove that logic
14+
- ⚠️ 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

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

Lines changed: 1 addition & 4 deletions
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
})

‎packages/react-router/__tests__/router/lazy-test.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,9 +1127,7 @@ describe("lazily loaded route modules", () => {
11271127
!(context instanceof Response),
11281128
"Expected a StaticContext instance"
11291129
);
1130-
expect(context.loaderData).toEqual({
1131-
root: null,
1132-
});
1130+
expect(context.loaderData).toEqual({});
11331131
expect(context.errors).toEqual({
11341132
lazy: new Error("LAZY LOADER ERROR"),
11351133
});
@@ -1163,9 +1161,7 @@ describe("lazily loaded route modules", () => {
11631161
!(context instanceof Response),
11641162
"Expected a StaticContext instance"
11651163
);
1166-
expect(context.loaderData).toEqual({
1167-
root: null,
1168-
});
1164+
expect(context.loaderData).toEqual({});
11691165
expect(context.errors).toEqual({
11701166
root: new Error("LAZY LOADER ERROR"),
11711167
});

‎packages/react-router/__tests__/router/ssr-test.ts

Lines changed: 2 additions & 7 deletions
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" },

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

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,10 +1144,7 @@ describe("shared server runtime", () => {
11441144
let context = calls[0][3].staticHandlerContext as StaticHandlerContext;
11451145
expect(context.errors).toBeTruthy();
11461146
expect(context.errors!.root.status).toBe(400);
1147-
expect(context.loaderData).toEqual({
1148-
root: null,
1149-
"routes/test": null,
1150-
});
1147+
expect(context.loaderData).toEqual({});
11511148
});
11521149

11531150
test("thrown action responses bubble up for index routes", async () => {
@@ -1191,10 +1188,7 @@ describe("shared server runtime", () => {
11911188
let context = calls[0][3].staticHandlerContext as StaticHandlerContext;
11921189
expect(context.errors).toBeTruthy();
11931190
expect(context.errors!.root.status).toBe(400);
1194-
expect(context.loaderData).toEqual({
1195-
root: null,
1196-
"routes/_index": null,
1197-
});
1191+
expect(context.loaderData).toEqual({});
11981192
});
11991193

12001194
test("thrown action responses catch deep", async () => {
@@ -1240,7 +1234,6 @@ describe("shared server runtime", () => {
12401234
expect(context.errors!["routes/test"].status).toBe(400);
12411235
expect(context.loaderData).toEqual({
12421236
root: "root",
1243-
"routes/test": null,
12441237
});
12451238
});
12461239

@@ -1287,7 +1280,6 @@ describe("shared server runtime", () => {
12871280
expect(context.errors!["routes/_index"].status).toBe(400);
12881281
expect(context.loaderData).toEqual({
12891282
root: "root",
1290-
"routes/_index": null,
12911283
});
12921284
});
12931285

@@ -1342,8 +1334,6 @@ describe("shared server runtime", () => {
13421334
expect(context.errors!["routes/__layout"].data).toBe("action");
13431335
expect(context.loaderData).toEqual({
13441336
root: "root",
1345-
"routes/__layout": null,
1346-
"routes/__layout/test": null,
13471337
});
13481338
});
13491339

@@ -1398,8 +1388,6 @@ describe("shared server runtime", () => {
13981388
expect(context.errors!["routes/__layout"].data).toBe("action");
13991389
expect(context.loaderData).toEqual({
14001390
root: "root",
1401-
"routes/__layout": null,
1402-
"routes/__layout/index": null,
14031391
});
14041392
});
14051393

@@ -1533,10 +1521,7 @@ describe("shared server runtime", () => {
15331521
expect(context.errors!.root).toBeInstanceOf(Error);
15341522
expect(context.errors!.root.message).toBe("Unexpected Server Error");
15351523
expect(context.errors!.root.stack).toBeUndefined();
1536-
expect(context.loaderData).toEqual({
1537-
root: null,
1538-
"routes/test": null,
1539-
});
1524+
expect(context.loaderData).toEqual({});
15401525
});
15411526

15421527
test("action errors bubble up for index routes", async () => {
@@ -1582,10 +1567,7 @@ describe("shared server runtime", () => {
15821567
expect(context.errors!.root).toBeInstanceOf(Error);
15831568
expect(context.errors!.root.message).toBe("Unexpected Server Error");
15841569
expect(context.errors!.root.stack).toBeUndefined();
1585-
expect(context.loaderData).toEqual({
1586-
root: null,
1587-
"routes/_index": null,
1588-
});
1570+
expect(context.loaderData).toEqual({});
15891571
});
15901572

15911573
test("action errors catch deep", async () => {
@@ -1617,6 +1599,7 @@ describe("shared server runtime", () => {
16171599

16181600
let request = new Request(`${baseUrl}/test`, { method: "post" });
16191601

1602+
debugger;
16201603
let result = await handler(request);
16211604
expect(result.status).toBe(500);
16221605
expect(testAction.mock.calls.length).toBe(1);
@@ -1635,7 +1618,6 @@ describe("shared server runtime", () => {
16351618
expect(context.errors!["routes/test"].stack).toBeUndefined();
16361619
expect(context.loaderData).toEqual({
16371620
root: "root",
1638-
"routes/test": null,
16391621
});
16401622
});
16411623

@@ -1686,7 +1668,6 @@ describe("shared server runtime", () => {
16861668
expect(context.errors!["routes/_index"].stack).toBeUndefined();
16871669
expect(context.loaderData).toEqual({
16881670
root: "root",
1689-
"routes/_index": null,
16901671
});
16911672
});
16921673

@@ -1745,8 +1726,6 @@ describe("shared server runtime", () => {
17451726
expect(context.errors!["routes/__layout"].stack).toBeUndefined();
17461727
expect(context.loaderData).toEqual({
17471728
root: "root",
1748-
"routes/__layout": null,
1749-
"routes/__layout/test": null,
17501729
});
17511730
});
17521731

@@ -1805,8 +1784,6 @@ describe("shared server runtime", () => {
18051784
expect(context.errors!["routes/__layout"].stack).toBeUndefined();
18061785
expect(context.loaderData).toEqual({
18071786
root: "root",
1808-
"routes/__layout": null,
1809-
"routes/__layout/index": null,
18101787
});
18111788
});
18121789

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

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4076,11 +4076,7 @@ export function createStaticHandler(
40764076
if (matchesToLoad.length === 0) {
40774077
return {
40784078
matches,
4079-
// Add a null for all matched routes for proper revalidation on the client
4080-
loaderData: matches.reduce(
4081-
(acc, m) => Object.assign(acc, { [m.route.id]: null }),
4082-
{}
4083-
),
4079+
loaderData: {},
40844080
errors:
40854081
pendingActionResult && isErrorResult(pendingActionResult[1])
40864082
? {
@@ -4115,16 +4111,6 @@ export function createStaticHandler(
41154111
skipLoaderErrorBubbling
41164112
);
41174113

4118-
// Add a null for any non-loader matches for proper revalidation on the client
4119-
let executedLoaders = new Set<string>(
4120-
matchesToLoad.map((match) => match.route.id)
4121-
);
4122-
matches.forEach((match) => {
4123-
if (!executedLoaders.has(match.route.id)) {
4124-
handlerContext.loaderData[match.route.id] = null;
4125-
}
4126-
});
4127-
41284114
return {
41294115
...handlerContext,
41304116
matches,

0 commit comments

Comments
 (0)