fix(vite): skip full reload for server only modules scanned by client css#19745
Conversation
WalkthroughAdds a regression test in integrations/vite/react-router.test.ts that verifies editing a server-only loader dependency triggers Hot Data Reload (HDR) instead of a full page reload. The test scaffolds a React Router app, a server-only module consumed by a route loader, edits that module from "HDR: 0" to "HDR: 1", and asserts the UI updates and a client HMR message is emitted. Updates packages/@tailwindcss-vite/src/index.ts hotUpdate logic to only mark files as external when modules exist and to abort hot updates if non-asset modules are present in other environments. Adds a CHANGELOG entry noting skipped full reloads for server-only modules. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/@tailwindcss-vite/src/index.ts (1)
222-229: Variablemodulesshadows the function parameter.On line 225,
const modulesshadows themodulesparameter from line 190. This reduces readability and could lead to bugs during future maintenance. Consider renaming to clarify it refers to modules in a different environment.♻️ Suggested rename to avoid shadowing
for (const environment of Object.values(server.environments)) { if (environment.name === this.environment.name) continue - const modules = environment.moduleGraph.getModulesByFile(file) - if (modules && [...modules].some((m) => m.type !== 'asset')) { + const envModules = environment.moduleGraph.getModulesByFile(file) + if (envModules && [...envModules].some((m) => m.type !== 'asset')) { return } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/`@tailwindcss-vite/src/index.ts around lines 222 - 229, The local const named "modules" inside the loop over server.environments shadows the function parameter "modules"; rename the loop-local variable (e.g., to "envModules" or "otherEnvModules") and update its usage in the condition that calls environment.moduleGraph.getModulesByFile(file) and the subsequent [...modules].some(...) check so that the parameter "modules" is not shadowed and readability is restored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/`@tailwindcss-vite/src/index.ts:
- Around line 222-229: The local const named "modules" inside the loop over
server.environments shadows the function parameter "modules"; rename the
loop-local variable (e.g., to "envModules" or "otherEnvModules") and update its
usage in the condition that calls environment.moduleGraph.getModulesByFile(file)
and the subsequent [...modules].some(...) check so that the parameter "modules"
is not shadowed and readability is restored.
|
Hi team, I'll work on fixing the full reload for server-only modules. Let me investigate! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/@tailwindcss-vite/src/index.ts (1)
218-233: Good fix for server-only module handling; consider renaming shadowed variable.The cross-environment guard correctly implements the fix for skipping full reloads when a file has non-asset modules in other environments (e.g., SSR). This allows SSR frameworks to handle their own HMR workflows.
Minor readability concern:
moduleson line 225 shadows themodulesparameter from the function signature (line 190). Consider renaming tootherEnvModulesor similar for clarity.Suggested rename for clarity
for (let environment of Object.values(server.environments)) { if (environment.name === this.environment.name) continue - let modules = environment.moduleGraph.getModulesByFile(file) - if (modules) { - for (let module of modules) { + let otherEnvModules = environment.moduleGraph.getModulesByFile(file) + if (otherEnvModules) { + for (let module of otherEnvModules) { if (module.type !== 'asset') { return } } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/`@tailwindcss-vite/src/index.ts around lines 218 - 233, The loop over server.environments uses a local variable named modules which shadows the function parameter modules; rename the inner variable (e.g., otherEnvModules or envModules) and update all uses inside the loop (the call to environment.moduleGraph.getModulesByFile and the subsequent for..of checking module.type) to reference the new name, leaving the function parameter modules intact to avoid confusion and potential bugs in server.environments, environment, moduleGraph.getModulesByFile and module.type checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/`@tailwindcss-vite/src/index.ts:
- Around line 218-233: The loop over server.environments uses a local variable
named modules which shadows the function parameter modules; rename the inner
variable (e.g., otherEnvModules or envModules) and update all uses inside the
loop (the call to environment.moduleGraph.getModulesByFile and the subsequent
for..of checking module.type) to reference the new name, leaving the function
parameter modules intact to avoid confusion and potential bugs in
server.environments, environment, moduleGraph.getModulesByFile and module.type
checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3c71cd02-034d-4915-956e-eafb55f4cfc9
📒 Files selected for processing (1)
packages/@tailwindcss-vite/src/index.ts
RobinMalfait
left a comment
RobinMalfait
left a comment
There was a problem hiding this comment.
Beautiful, thank you very much!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Line 19: Update the changelog entry text (the line mentioning "Skip full
reload for server only modules when using `@tailwindcss/vite`") to more
precisely reflect the implemented hot-update guard: change it to something like
"Skip client full reloads for server-only modules watched during CSS scanning
when using `@tailwindcss/vite`" so it matches the behavior covered by the
regression test in integrations/vite/react-router.test.ts.
hi-ogawa
deleted the
03-02-fix_vite_skip_full_reload_for_server_only_modules_watched_by_client
branch
…g RSC HMR
@tailwindcss/vite's hotUpdate hook sends a bare {type:'full-reload'} to the
client environment when server-only files change, because Tailwind scans them
for CSS class names. This breaks RSC HMR by causing a full page reload instead
of letting rsc:update + router.refresh() handle it gracefully.
Root cause identified using console.trace on hot.send across all Vite
environments — the stack trace pointed directly at @tailwindcss/vite's
hotUpdate hook in the generate:serve plugin.
This is a known bug fixed in tailwindlabs/tailwindcss#19745 (merged Mar 12)
but not yet released (latest is 4.2.1 from Feb 23). The workaround deletes
Tailwind's hotUpdate hook via configResolved, same approach used by other
RSC frameworks hitting this issue.
Also adds:
- 'main entry hmr' e2e test with window sentinel to verify no full reload
- AGENTS.md section on debugging unwanted full page reloads in Vite
- Corrects AGENTS.md: server HMR does NOT preserve client state
- Waku reference note in AGENTS.md for cross-checking Vite RSC patterns
3 participants
Summary
The change in #19670 didn't take account for server only modules managed by SSR framework. Forcing full reload for this path breaks server HMR. This PR added a check to determine whether the same modified file has associated modules in a different environment module graph to avoid this.
Test plan
Added an integration test for React router HDR (server loader hmr). This test fails on main.
Also the local build is tested on
@vitejs/plugin-rscCI and confirmed the fix vitejs/vite-plugin-react#1132