Skip to content

Commit 246f836

Browse files
markdalgleishmarkdalgleish
authored
Dedupe imported route modules in RSC Framework Mode (#14186)
1 parent 4dfb883 commit 246f836

File tree

3 files changed

+364
-18
lines changed

3 files changed

+364
-18
lines changed

‎integration/deduped-route-modules-test.ts‎

Lines changed: 294 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,294 @@
1+
import { test, expect } from "@playwright/test";
2+
3+
import { createFixture, createAppFixture } from "./helpers/create-fixture.js";
4+
import type { Fixture, AppFixture } from "./helpers/create-fixture.js";
5+
import { PlaywrightFixture } from "./helpers/playwright-fixture.js";
6+
import {
7+
type TemplateName,
8+
reactRouterConfig,
9+
viteConfig,
10+
} from "./helpers/vite.js";
11+
12+
const templateNames = [
13+
"vite-5-template",
14+
"rsc-vite-framework",
15+
] as const satisfies TemplateName[];
16+
17+
// This test ensures that code is not accidentally duplicated when a route is
18+
// imported within user code since they're not importing one of our internal
19+
// virtual route modules.
20+
test.describe("Deduped route modules", () => {
21+
for (const templateName of templateNames) {
22+
test.describe(`template: ${templateName}`, () => {
23+
let fixture: Fixture;
24+
let appFixture: AppFixture;
25+
26+
test.beforeAll(async () => {
27+
fixture = await createFixture({
28+
templateName,
29+
files: {
30+
"vite.config.js": await viteConfig.basic({
31+
templateName,
32+
}),
33+
"react-router.config.ts": reactRouterConfig({
34+
viteEnvironmentApi: templateName.includes("rsc"),
35+
}),
36+
"app/routes/client-first.a.tsx": `
37+
import { Link } from "react-router";
38+
39+
export const customExport = (() => {
40+
globalThis.custom_export_count = (globalThis.custom_export_count || 0) + 1;
41+
return () => true;
42+
})();
43+
44+
export const loader = (() => {
45+
globalThis.loader_count = (globalThis.loader_count || 0) + 1;
46+
return () => ({
47+
customExportCount: globalThis.custom_export_count,
48+
loaderCount: globalThis.loader_count,
49+
componentCount: globalThis.component_count,
50+
});
51+
})();
52+
53+
export const clientLoader = (() => {
54+
globalThis.client_loader_count = (globalThis.client_loader_count || 0) + 1;
55+
return async ({ serverLoader }) => {
56+
const loaderData = await serverLoader();
57+
return {
58+
loaderCount: loaderData.loaderCount,
59+
clientLoaderCount: globalThis.client_loader_count,
60+
serverCustomExportCount: loaderData.customExportCount,
61+
clientCustomExportCount: globalThis.custom_export_count,
62+
serverComponentCount: loaderData.componentCount,
63+
clientComponentCount: globalThis.component_count,
64+
};
65+
};
66+
})();
67+
clientLoader.hydrate = true;
68+
69+
const RouteA = (() => {
70+
globalThis.component_count = (globalThis.component_count || 0) + 1;
71+
return ({ loaderData }: Route.ComponentProps) => {
72+
return (
73+
<>
74+
<h1>Module Count</h1>
75+
<p>Loader count: <span data-loader-count>{loaderData.loaderCount}</span></p>
76+
<p>Client loader count: <span data-client-loader-count>{loaderData.clientLoaderCount}</span></p>
77+
<p>Server custom export count: <span data-server-custom-export-count>{loaderData.serverCustomExportCount}</span></p>
78+
<p>Client custom export count: <span data-client-custom-export-count>{loaderData.clientCustomExportCount}</span></p>
79+
<p>Server component count: <span data-server-component-count>{loaderData.serverComponentCount}</span></p>
80+
<p>Client component count: <span data-client-component-count>{loaderData.clientComponentCount}</span></p>
81+
<p><Link to="/client-first/b">Go to Route B</Link></p>
82+
</>
83+
);
84+
};
85+
})();
86+
87+
export default RouteA;
88+
`,
89+
"app/routes/client-first.b.tsx": `
90+
import { Link } from "react-router";
91+
92+
import { customExport } from "./client-first.a";
93+
94+
export default function RouteB() {
95+
return customExport && (
96+
<>
97+
<h1>Route B</h1>
98+
<p>This route imports the route module from Route A, so could potentially cause code duplication.</p>
99+
<p><Link to="/client-first/a">Go to Route A</Link></p>
100+
</>
101+
);
102+
}
103+
`,
104+
105+
...(templateName.includes("rsc")
106+
? {
107+
"app/routes/rsc-server-first.a/route.tsx": `
108+
import { Link } from "react-router";
109+
import { ModuleCounts, clientLoader } from "./client";
110+
111+
export const customExport = (() => {
112+
globalThis.rsc_custom_export_count = (globalThis.rsc_custom_export_count || 0) + 1;
113+
return () => true;
114+
})();
115+
116+
export const loader = (() => {
117+
globalThis.rsc_loader_count = (globalThis.rsc_loader_count || 0) + 1;
118+
return () => ({
119+
customExportCount: globalThis.rsc_custom_export_count,
120+
loaderCount: globalThis.rsc_loader_count,
121+
componentCount: globalThis.rsc_component_count,
122+
});
123+
})();
124+
125+
export { clientLoader };
126+
127+
export const ServerComponent = (() => {
128+
globalThis.rsc_component_count = (globalThis.rsc_component_count || 0) + 1;
129+
return () => {
130+
return (
131+
<>
132+
<h1>RSC Server-First Module Count</h1>
133+
<ModuleCounts />
134+
<p><Link to="/rsc-server-first/b">Go to RSC Route B</Link></p>
135+
</>
136+
);
137+
};
138+
})();
139+
`,
140+
"app/routes/rsc-server-first.a/client.tsx": `
141+
"use client";
142+
143+
import { useLoaderData } from "react-router";
144+
145+
export const clientLoader = (() => {
146+
globalThis.rsc_client_loader_count = (globalThis.rsc_client_loader_count || 0) + 1;
147+
return async ({ serverLoader }) => {
148+
const loaderData = await serverLoader();
149+
return {
150+
loaderCount: loaderData.loaderCount,
151+
clientLoaderCount: globalThis.rsc_client_loader_count,
152+
serverCustomExportCount: loaderData.customExportCount,
153+
clientCustomExportCount: globalThis.rsc_custom_export_count,
154+
serverComponentCount: loaderData.componentCount,
155+
};
156+
};
157+
})();
158+
clientLoader.hydrate = true;
159+
160+
export function ModuleCounts() {
161+
const loaderData = useLoaderData();
162+
return (
163+
<>
164+
<p>Loader count: <span data-loader-count>{loaderData.loaderCount}</span></p>
165+
<p>Client loader count: <span data-client-loader-count>{loaderData.clientLoaderCount}</span></p>
166+
<p>Server custom export count: <span data-server-custom-export-count>{loaderData.serverCustomExportCount}</span></p>
167+
<p>Client custom export count: <span data-client-custom-export-count>{loaderData.clientCustomExportCount}</span></p>
168+
<p>Server component count: <span data-server-component-count>{loaderData.serverComponentCount}</span></p>
169+
</>
170+
);
171+
}
172+
`,
173+
"app/routes/rsc-server-first.b.tsx": `
174+
import { Link } from "react-router";
175+
176+
import { customExport } from "./rsc-server-first.a/route";
177+
178+
// Ensure custom export is used in the client build in this route
179+
export const handle = customExport;
180+
181+
export function ServerComponent() {
182+
return customExport && (
183+
<>
184+
<h1>RSC Route B</h1>
185+
<p>This route imports the route module from RSC Route A, so could potentially cause code duplication.</p>
186+
<p><Link to="/rsc-server-first/a">Go to RSC Route A</Link></p>
187+
</>
188+
);
189+
}
190+
`,
191+
}
192+
: {}),
193+
},
194+
});
195+
196+
appFixture = await createAppFixture(fixture);
197+
});
198+
199+
test.afterAll(() => {
200+
appFixture.close();
201+
});
202+
203+
let logs: string[] = [];
204+
205+
test.beforeEach(({ page }) => {
206+
page.on("console", (msg) => {
207+
logs.push(msg.text());
208+
});
209+
});
210+
211+
test.afterEach(() => {
212+
expect(logs).toHaveLength(0);
213+
});
214+
215+
test("Client-first routes", async ({ page }) => {
216+
let app = new PlaywrightFixture(appFixture, page);
217+
218+
let pageErrors: unknown[] = [];
219+
page.on("pageerror", (error) => pageErrors.push(error));
220+
221+
await app.goto(`/client-first/b`, true);
222+
expect(pageErrors).toEqual([]);
223+
224+
await app.clickLink("/client-first/a");
225+
await page.waitForSelector("[data-loader-count]");
226+
expect(await page.locator("[data-loader-count]").textContent()).toBe(
227+
"1",
228+
);
229+
expect(
230+
await page.locator("[data-client-loader-count]").textContent(),
231+
).toBe("1");
232+
expect(
233+
await page.locator("[data-server-custom-export-count]").textContent(),
234+
).toBe(
235+
templateName.includes("rsc")
236+
? // In RSC, custom exports are present in both the react-server and react-client
237+
// environments (so they're available to be imported by both),
238+
// which means the Node server actually gets 2 copies
239+
"2"
240+
: "1",
241+
);
242+
expect(
243+
await page.locator("[data-client-custom-export-count]").textContent(),
244+
).toBe("1");
245+
expect(
246+
await page.locator("[data-server-component-count]").textContent(),
247+
).toBe("1");
248+
expect(
249+
await page.locator("[data-client-component-count]").textContent(),
250+
).toBe("1");
251+
expect(pageErrors).toEqual([]);
252+
});
253+
254+
test("Server-first routes", async ({ page }) => {
255+
test.skip(
256+
!templateName.includes("rsc"),
257+
"Server-first routes are an RSC-only feature",
258+
);
259+
260+
let app = new PlaywrightFixture(appFixture, page);
261+
262+
let pageErrors: unknown[] = [];
263+
page.on("pageerror", (error) => pageErrors.push(error));
264+
265+
await app.goto(`/rsc-server-first/b`, true);
266+
expect(pageErrors).toEqual([]);
267+
268+
await app.clickLink("/rsc-server-first/a");
269+
await page.waitForSelector("[data-loader-count]");
270+
expect(await page.locator("[data-loader-count]").textContent()).toBe(
271+
"1",
272+
);
273+
expect(
274+
await page.locator("[data-client-loader-count]").textContent(),
275+
).toBe("1");
276+
expect(
277+
await page.locator("[data-server-custom-export-count]").textContent(),
278+
).toBe(
279+
// In RSC, custom exports are present in both the react-server and react-client
280+
// environments (so they're available to be imported by both),
281+
// which means the Node server actually gets 2 copies
282+
"2",
283+
);
284+
expect(
285+
await page.locator("[data-client-custom-export-count]").textContent(),
286+
).toBe("1");
287+
expect(
288+
await page.locator("[data-server-component-count]").textContent(),
289+
).toBe("1");
290+
expect(pageErrors).toEqual([]);
291+
});
292+
});
293+
}
294+
});

‎packages/react-router-dev/vite/rsc/plugin.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { createVirtualRouteConfig } from "./virtual-route-config";
1717
import {
1818
transformVirtualRouteModules,
1919
parseRouteExports,
20+
isVirtualClientRouteModuleId,
2021
CLIENT_NON_COMPONENT_EXPORTS,
2122
} from "./virtual-route-modules";
2223
import validatePluginOrder from "../plugins/validate-plugin-order";
@@ -171,7 +172,14 @@ export function reactRouterRSCVitePlugin(): Vite.PluginOption[] {
171172
{
172173
name: "react-router/rsc/virtual-route-modules",
173174
transform(code, id) {
174-
return transformVirtualRouteModules({ code, id, viteCommand });
175+
if (!routeIdByFile) return;
176+
return transformVirtualRouteModules({
177+
code,
178+
id,
179+
viteCommand,
180+
routeIdByFile,
181+
viteEnvironment: this.environment,
182+
});
175183
},
176184
},
177185
{
@@ -240,8 +248,8 @@ export function reactRouterRSCVitePlugin(): Vite.PluginOption[] {
240248
const useFastRefresh = !ssr && (isJSX || code.includes(devRuntime));
241249
if (!useFastRefresh) return;
242250

243-
const routeId = routeIdByFile?.get(filepath);
244-
if (routeId !== undefined) {
251+
if (isVirtualClientRouteModuleId(id)) {
252+
const routeId = routeIdByFile?.get(filepath);
245253
return { code: addRefreshWrapper({ routeId, code, id }) };
246254
}
247255

0 commit comments

Comments
 (0)