fix: CSS modules not working in _app/_layout/_error and across routes in non-island components by Hajime-san · Pull Request #3781 · denoland/fresh

Hajime-san · 2026-04-25T08:05:28Z

Originally reported:

CSS Modules compilation issues #3633 (comment)

Repro:
https://github.com/honmanyau/fresh-2-layout-css-module-issue/blob/24bf9c597a057c60acdb2c2934a65f22b6276ecd/routes/page-b/_layout.tsx

The PR seems to resolve the island components problem, but not non-island.

fix: CSS modules not working in _app/_layout/_error and across routes in dev #3764

This PR introduces CSS Modules handling for non-island components rendered through utility files such as _app, _layout, _error, and _404. The core change is that utility-file mod.css is now preserved through Fresh’s command and render pipeline, so CSS referenced from app/layout/route/error rendering can be accumulated and emitted in the final response instead of being dropped.

On the Vite side, development now also collects CSS exposed through fresh-route-css::* virtual modules, which is the path used by server-rendered non-island components in utility files. For production builds, CSS placeholder replacement is no longer limited to route chunks, because this CSS can be hoisted into shared chunks such as server-entry.

I added new tests and confirmed to work fine about our private project via links in deno config locally.

bartlomieju

Nice fix — the approach of threading css through the existing command → segment → context pipeline is clean and consistent with the rest of the architecture. A few things caught my eye below (inline), but overall this looks solid.

The setAdditionalStyles refactor from a simple setter to a dedup-accumulator is exactly right, getSsrModule() properly handles both real files and virtual module IDs, and the BFS graph walk in collectRouteCss is thorough.

Let me know if you'd like a hand getting this over the finish line — happy to help with any of the items below or with testing.

bartlomieju · 2026-04-25T12:32:34Z

+              `["__FRESH_CSS_PLACEHOLDER__"]`,
+              info.css
+                ? JSON.stringify(info.css.map((css) => `/${css}`))
+                : "null",


This uses content.replace() which only replaces the first occurrence of the placeholder. If two utility-file CSS placeholders (e.g. from both _app and _layout) get hoisted into the same shared chunk like server-entry, only the first one would be replaced.

Should this be replaceAll (or a loop) to be safe?

Alternatively, if each route virtual module gets its own unique placeholder, this is fine — but it might be worth a comment explaining why a single replace is sufficient.

Nice catch. I notice that it causes requesting to http://localhost:8000/__FRESH_CSS_PLACEHOLDER__ in build mode. It should be replaceAll. Fixed and added a test.

bartlomieju · 2026-04-25T12:32:34Z


+          route.css = server === undefined
+            ? route.css
+            : await collectRouteCss(server, route.filePath);


This re-walks the full SSR import graph and overwrites route.css on every load call (including HMR invalidations). That seems intentional for picking up changes during dev, but for deep dependency trees the fetchModule calls inside collectRouteCss could add up.

Might be worth a short comment here noting this is intentional for HMR correctness, so a future reader doesn't try to "optimize" it with caching.

Sorry for confusing, it seems to be my AI slop code. This code is unnecessary for HMR.
I'm not sure if the HMR is working with the changes in this PR. Since it definitely works after a page reload, I'm thinking of putting it out of scope for now.

bartlomieju · 2026-04-25T12:32:34Z

@@ -298,16 +321,18 @@ export class Context<State> {



Nit: appDef?.css evaluates to undefined when appDef is null, which the new setAdditionalStyles handles via the css == null guard. Works correctly, but it's a subtle null-propagation chain. An explicit guard might be clearer:

if (appDef !== null) { setAdditionalStyles(this, appDef.css); }

Especially since the appDef !== null checks already exist a few lines below.

bartlomieju · 2026-04-25T12:32:35Z

+
+      if (ctx.#additionalStyles === null) {
+        ctx.#additionalStyles = css.slice();
+        return;


Minor: includes() is O(n) per insertion. For the small CSS lists in practice this is fine, but a Set would be more idiomatic if you ever want to tighten this up. Low priority.

Yep, I added a FIXME comment below. I think it could be use Set instead of css: Array<string>.It seems that at least all the tests pass in the following branch.

Hajime-san/fresh@fix-ssr-css-modules...Hajime-san:fresh:perf-css

I'm planning it as a follow up PR.

bartlomieju · 2026-04-25T12:32:35Z

+        });
+      },
+    );
+  },


The tests cover _layout.tsx which is great. The PR title also mentions _app, _error, and _404 — are those paths already exercised by existing tests, or would it be worth adding a case or two here to lock in coverage for those as well?

Hajime-san · 2026-04-26T01:20:30Z

@bartlomieju
Thanks!
Upon investigation, I found that our project was exhibiting behavior where the screen was not rendering in dev mode.
Also, it appears that CI is crashing only on Windows. I will re-request review once the items already reviewed and the above problems are resolved.

UPDATE:

the screen was not rendering in dev mode.

It seems an another problem of links. It works fine with vendor and copy-pasting locally.

This reverts commit f1934f0.

Hajime-san · 2026-04-26T02:28:10Z

+              `["__FRESH_CSS_PLACEHOLDER__"]`,
+              info.css
+                ? JSON.stringify(info.css.map((css) => `/${css}`))
+                : "null",


Nice catch. I notice that it causes requesting to http://localhost:8000/__FRESH_CSS_PLACEHOLDER__ in build mode. It should be replaceAll. Fixed and added a test.

Hajime-san · 2026-04-26T11:03:10Z

    files.push({
      id,
-      filePath: entry.path,
+      filePath: toPosix(entry.path),


Suddenly, several tests failed on Windows. It's likely that a potential bug was occured by this PR.

vite dev - css modules => ./packages/fresh/src/test_utils.ts:198:8 vite dev - css modules in _app/_layout/_error non-island component are injected => ./packages/fresh/src/test_utils.ts:198:8 vite dev - route css import => ./packages/fresh/src/test_utils.ts:198:8

debug console on Windows

------- post-test output ------- 👺 > fresh:route-css > name _index <!DOCTYPE html> <html lang="en"> <head> <meta charset="utf-8"> <meta name="viewport" content="width=device-width, initial-scale=1.0"> </head> <body> <div class="green"> <h1 class="_root_1mdiz_1"> green text </h1> </div> <div class="red"> <h1 class="_root_xlv1v_1"> red text </h1> </div> <h1> ok </h1> </body> </html> [ "Port 5173 is in use, trying another one...", "", " VITE v7.3.1 ready in 1276 ms", "", " ➜ Local: http://127.0.0.1:5174/" ] [ "Port 5173 is in use, trying another one...", "", " VITE v7.3.1 ready in 1276 ms", "", " ➜ Local: http://127.0.0.1:5174/", "\x1b[2m \x1b[32m➜\x1b[39m \x1b[1mNetwork\x1b[22m\x1b[2m: use \x1b[22m\x1b[1m--host\x1b[22m\x1b[2m to expose\x1b[22m", "👺 > fresh:route-css > route {", ' id: "/_app",', ' filePath: "D:\\\\a\\\\fresh\\\\fresh\\\\packages\\\\plugin-vite\\\\tests\\\\fixtures\\\\non_island_css_modules\\\\routes\\\\_app.tsx",', ' type: "app",', ' pattern: "*",', ' routePattern: "*",', " lazy: false,", " css: [", ' "/@fs/D:/a/fresh/fresh/packages/plugin-vite/demo/components/CssModulesNonIsland.module.css"', " ],", " overrideConfig: undefined", "}", "👺 > fresh:route-css > name __app", "👺 > fresh:route-css > route {", ' id: "/_layout",', ' filePath: "D:\\\\a\\\\fresh\\\\fresh\\\\packages\\\\plugin-vite\\\\tests\\\\fixtures\\\\non_island_css_modules\\\\routes\\\\_layout.tsx",', ' type: "layout",', ' pattern: "/",', ' routePattern: "/",', " lazy: false,", " css: [", ' "/@fs/D:/a/fresh/fresh/packages/plugin-vite/demo/components/CssModulesNonIsland3.module.css"', " ],", " overrideConfig: undefined", "}", "👺 > fresh:route-css > name __layout", "👺 > fresh:route-css > route {", ' id: "/_error",', ' filePath: "D:\\\\a\\\\fresh\\\\fresh\\\\packages\\\\plugin-vite\\\\tests\\\\fixtures\\\\non_island_css_modules\\\\routes\\\\_error.tsx",', ' type: "error",', ' pattern: "/",', ' routePattern: "/",', " lazy: false,", " css: [", ' "/@fs/D:/a/fresh/fresh/packages/plugin-vite/demo/components/CssModulesNonIsland2.module.css"', " ],", " overrideConfig: undefined", "}", "👺 > fresh:route-css > name __error", "👺 > fresh:route-css > route {", ' id: "/index",', ' filePath: "D:\\\\a\\\\fresh\\\\fresh\\\\packages\\\\plugin-vite\\\\tests\\\\fixtures\\\\non_island_css_modules\\\\routes\\\\index.tsx",', ' type: "route",', ' pattern: "/",', ' routePattern: "/",', " lazy: true,", " css: [],", ' overrideConfig: { methods: "ALL" }', "}", "👺 > fresh:route-css > name _index", "👺 > renderRoute > setAdditionalStyles []", "👺 > render Compose VNode > setAdditionalStyles []", "👺 > render compose App > setAdditionalStyles []" ] ./packages/plugin-vite/tests/dev_server_test.ts => vite dev - css modules in _app/_layout/_error non-island component are injected ...----- post-test output end ----- ./packages/plugin-vite/tests/dev_server_test.ts => vite dev - css modules in _app/_layout/_error non-island component are injected ... FAILED (5s) ./packages/plugin-vite/tests/build_test.ts => vite build - tailwind _app ... ok (6s) ./packages/plugin-vite/tests/build_test.ts => vite build - partial island ... ok (1s) ------- post-test output -------

Hajime-san · 2026-04-26T11:13:37Z

+
+      if (ctx.#additionalStyles === null) {
+        ctx.#additionalStyles = css.slice();
+        return;


Yep, I added a FIXME comment below. I think it could be use Set instead of css: Array<string>.It seems that at least all the tests pass in the following branch.

Hajime-san/fresh@fix-ssr-css-modules...Hajime-san:fresh:perf-css

I'm planning it as a follow up PR.

Hajime-san · 2026-04-27T07:56:40Z

-            routeFileToName.set(route.filePath, name);
+            routeFileToName.set(toPosix(route.filePath), name);


seems to be same as https://github.com/denoland/fresh/pull/3781/changes#r3143377420

Hajime-san · 2026-04-27T07:56:53Z

-                const normalized = path.normalize(item.file);
+                const normalized = toPosix(path.normalize(item.file));


Hajime-san added 8 commits April 25, 2026 14:24

add test

0dc5b17

works

c03d476

fix

cbc4256

simpler

69afe2e

revert

db93938

comment

1f42f05

comment

df3fcdb

try fix windows

f1934f0

Hajime-san force-pushed the fix-ssr-css-modules branch from fa5fa74 to f1934f0 Compare April 25, 2026 10:15

Hajime-san marked this pull request as draft April 25, 2026 11:29

bartlomieju reviewed Apr 25, 2026

View reviewed changes

Hajime-san added 6 commits April 26, 2026 11:17

add test fixture

5be11d3

replaceAll

bf4193a

Revert "try fix windows"

b6fe5ed

This reverts commit f1934f0.

lint

c54f479

rm

c5fd7e5

rm slop impl

a5d90ca

Hajime-san force-pushed the fix-ssr-css-modules branch 4 times, most recently from b069071 to 0395403 Compare April 26, 2026 07:58

normalize windows path

ba75ad3

Hajime-san force-pushed the fix-ssr-css-modules branch from 0395403 to ba75ad3 Compare April 26, 2026 08:14

Hajime-san added 2 commits April 26, 2026 19:54

review

881cd2a

review

9c1e62f

Hajime-san force-pushed the fix-ssr-css-modules branch from 1d16070 to 9c1e62f Compare April 26, 2026 11:43

review

814e3b5

Hajime-san commented Apr 26, 2026

View reviewed changes

Hajime-san marked this pull request as ready for review April 26, 2026 12:15

This comment was marked as outdated.

Sign in to view

Hajime-san requested a review from bartlomieju April 26, 2026 12:16

Hajime-san force-pushed the fix-ssr-css-modules branch 2 times, most recently from ef270d9 to 814e3b5 Compare April 27, 2026 03:55

Hajime-san added 4 commits April 27, 2026 15:04

rm wrong HMR

948dbb9

try windows

3770e6d

try windows

871e871

try windows

493f81a

Hajime-san commented Apr 27, 2026

View reviewed changes

		routeFileToName.set(route.filePath, name);
		routeFileToName.set(toPosix(route.filePath), name);

		const normalized = path.normalize(item.file);
		const normalized = toPosix(path.normalize(item.file));

Conversation

Hajime-san commented Apr 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bartlomieju left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hajime-san commented Apr 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Hajime-san commented Apr 25, 2026 •

edited

Loading

Hajime-san commented Apr 26, 2026 •

edited

Loading