Skip to content

Commit 784e8d5

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

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
@@ -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
});
@@ -3006,9 +3004,7 @@ describe("lazily loaded route modules", () => {
30063004
!(context instanceof Response),
30073005
"Expected a StaticContext instance"
30083006
);
3009-
expect(context.loaderData).toEqual({
3010-
root: null,
3011-
});
3007+
expect(context.loaderData).toEqual({});
30123008
expect(context.errors).toEqual({
30133009
root: new Error("LAZY LOADER ERROR"),
30143010
});

‎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
@@ -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 () => {
@@ -1812,6 +1794,7 @@ describe("shared server runtime", () => {
18121794

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

1797+
debugger;
18151798
let result = await handler(request);
18161799
expect(result.status).toBe(500);
18171800
expect(testAction.mock.calls.length).toBe(1);
@@ -1830,7 +1813,6 @@ describe("shared server runtime", () => {
18301813
expect(context.errors!["routes/test"].stack).toBeUndefined();
18311814
expect(context.loaderData).toEqual({
18321815
root: "root",
1833-
"routes/test": null,
18341816
});
18351817
});
18361818

@@ -1881,7 +1863,6 @@ describe("shared server runtime", () => {
18811863
expect(context.errors!["routes/_index"].stack).toBeUndefined();
18821864
expect(context.loaderData).toEqual({
18831865
root: "root",
1884-
"routes/_index": null,
18851866
});
18861867
});
18871868

@@ -1940,8 +1921,6 @@ describe("shared server runtime", () => {
19401921
expect(context.errors!["routes/__layout"].stack).toBeUndefined();
19411922
expect(context.loaderData).toEqual({
19421923
root: "root",
1943-
"routes/__layout": null,
1944-
"routes/__layout/test": null,
19451924
});
19461925
});
19471926

@@ -2000,8 +1979,6 @@ describe("shared server runtime", () => {
20001979
expect(context.errors!["routes/__layout"].stack).toBeUndefined();
20011980
expect(context.loaderData).toEqual({
20021981
root: "root",
2003-
"routes/__layout": null,
2004-
"routes/__layout/index": null,
20051982
});
20061983
});
20071984

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

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