diff --git a/.changeset/serious-berries-pretend.md b/.changeset/serious-berries-pretend.md new file mode 100644 index 0000000000..5d871d63b9 --- /dev/null +++ b/.changeset/serious-berries-pretend.md @@ -0,0 +1,5 @@ +--- +"@react-router/dev": patch +--- + +avoid duplicate stylesheet links in framework mode when parent/layout routes and suspended child routes reference the same dynamic CSS asset, while preserving leaf-route dynamic CSS retention behavior diff --git a/integration/vite-css-suspended-duplicate-test.ts b/integration/vite-css-suspended-duplicate-test.ts new file mode 100644 index 0000000000..41244b0db5 --- /dev/null +++ b/integration/vite-css-suspended-duplicate-test.ts @@ -0,0 +1,177 @@ +import { test, expect } from "@playwright/test"; +import getPort from "get-port"; + +import { + build, + createEditor, + createProject, + reactRouterServe, + viteConfig, +} from "./helpers/vite.js"; + +const js = String.raw; + +test.describe("Vite CSS suspended duplicate styles", () => { + let port: number; + let cwd: string; + let stop: () => void; + + test.beforeAll(async () => { + port = await getPort(); + cwd = await createProject( + { + "vite.config.ts": await viteConfig.basic({ + port, + templateName: "vite-6-template", + vanillaExtract: true, + }), + "app/routes.ts": js` + import { type RouteConfig, index, route } from "@react-router/dev/routes"; + + export default [ + route(":slug", "routes/$slug/layout.tsx", [ + index("routes/$slug/route.tsx"), + ]), + ] satisfies RouteConfig; + `, + "app/components/WithStyles.css.ts": js` + import { style } from "@vanilla-extract/css"; + + export const withStyles = style({ + color: "rgb(255, 0, 0)", + }); + `, + "app/components/WithStyles.tsx": js` + import * as styles from "./WithStyles.css"; + + export function WithStyles() { + return
with styles
; + } + `, + "app/components/Suspended.tsx": js` + import { WithStyles } from "./WithStyles"; + + export function Suspended() { + return ; + } + `, + "app/routes/$slug/layout.tsx": js` + import { lazy, Suspense } from "react"; + import { Outlet } from "react-router"; + + const SuspendedLazy = lazy(() => + import("../../components/Suspended").then(({ Suspended }) => ({ + default: Suspended, + })), + ); + + import { WithStyles } from "../../components/WithStyles"; + + export default function LayoutRoute() { + return ( + <> +

layout

+ + + + + + ); + } + `, + "app/routes/$slug/route.tsx": js` + import { lazy, Suspense } from "react"; + + const SuspendedLazy = lazy(() => + import("../../components/Suspended").then(({ Suspended }) => ({ + default: Suspended, + })), + ); + + export default function SlugIndexRoute() { + return ( + <> +

route

+ + + + + ); + } + `, + }, + "vite-6-template", + ); + + let edit = createEditor(cwd); + await edit("package.json", (contents) => + contents.replace('"sideEffects": false', '"sideEffects": true'), + ); + + let { status } = build({ cwd }); + expect(status).toBe(0); + stop = await reactRouterServe({ cwd, port }); + }); + + test.afterAll(() => stop()); + + test("does not duplicate stylesheet links for suspended components in nested routes", async ({ + page, + }) => { + let stylesheetRequests: string[] = []; + page.on("requestfinished", (request) => { + if (request.resourceType() === "stylesheet") { + stylesheetRequests.push(request.url()); + } + }); + + await page.goto(`http://localhost:${port}/some`, { + waitUntil: "networkidle", + }); + + await expect(page.locator("[data-layout]")).toHaveText("layout"); + await expect(page.locator("[data-route]")).toHaveText("route"); + await expect(page.locator("[data-with-styles]").first()).toHaveCSS( + "color", + "rgb(255, 0, 0)", + ); + + let stylesheetHrefs = await page.evaluate(() => + Array.from( + document.querySelectorAll("link[rel='stylesheet']"), + ) + .map((link) => link.getAttribute("href")) + .filter((href): href is string => href != null), + ); + + let normalizeHref = (href: string) => href.replace(/#$/, ""); + let normalizedLinkHrefs = stylesheetHrefs + .map(normalizeHref) + .filter((href) => href.includes("WithStyles.css.ts-")); + let normalizedRequestHrefs = stylesheetRequests.map((requestUrl) => { + let url = new URL(requestUrl); + return normalizeHref(url.pathname + url.search + url.hash); + }) + .filter((href) => href.includes("WithStyles.css.ts-")); + + let duplicateLinkHrefs = normalizedLinkHrefs.filter( + (href, index, hrefs) => hrefs.indexOf(href) !== index, + ); + let duplicateRequestHrefs = normalizedRequestHrefs.filter( + (href, index, hrefs) => hrefs.indexOf(href) !== index, + ); + + expect(normalizedLinkHrefs.length).toBeGreaterThan(0); + expect( + duplicateLinkHrefs, + `Duplicate stylesheet links found.\nraw=${JSON.stringify(stylesheetHrefs)}\nnormalized=${JSON.stringify(normalizedLinkHrefs)}`, + ).toEqual([]); + + if (normalizedRequestHrefs.length > 0) { + expect( + duplicateRequestHrefs, + `Duplicate stylesheet requests found.\nraw=${JSON.stringify(stylesheetRequests)}\nnormalized=${JSON.stringify(normalizedRequestHrefs)}`, + ).toEqual([]); + } + }); +}); diff --git a/packages/react-router-dev/vite/plugin.ts b/packages/react-router-dev/vite/plugin.ts index 6eaf1297a3..ef4ab3a4f2 100644 --- a/packages/react-router-dev/vite/plugin.ts +++ b/packages/react-router-dev/vite/plugin.ts @@ -358,6 +358,13 @@ const getReactRouterManifestBuildAssets = ( invariant(entryChunk, `Chunk not found: ${entryFilePath}`); let isRootRoute = Boolean(route && route.parentId === undefined); + // Keep `#` only for leaf routes to preserve dynamic-import CSS retention + // without duplicating parent/layout route styles. + let shouldUseForciblyUniqueCssHrefs = + route !== null && + !Object.values(ctx.reactRouterConfig.routes).some( + (candidateRoute) => candidateRoute.parentId === route.id, + ); let routeModuleChunks = routeChunkNames .map((routeChunkName) => @@ -413,7 +420,10 @@ const getReactRouterManifestBuildAssets = ( // route-level CSS is removed from the document. We use a hash here // because it's a unique `href` value but isn't a unique network // request and only adds a single character. - return allDynamicCssFiles.has(href) ? `${publicHref}#` : publicHref; + return allDynamicCssFiles.has(href) && + shouldUseForciblyUniqueCssHrefs + ? `${publicHref}#` + : publicHref; }), ] .flat(1)