fix: restore full page reload for watched external files on Vite 7.1+#19670
fix: restore full page reload for watched external files on Vite 7.1+#19670RobinMalfait merged 13 commits intotailwindlabs:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdded a 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/`@tailwindcss-vite/src/index.ts:
- Around line 98-115: The current handleHotUpdate logic uses
files.includes(path.basename(file)) which can false-positive across different
watched directories; update the check to compare full resolved paths instead —
for the server.watcher.getWatched() loop, replace the basename check with
something like files.some(f => path.resolve(dir, f) === path.resolve(file)) so
the code only matches when the exact watched file path equals the changed file;
keep the rest of handleHotUpdate (including server.moduleGraph.getModulesByFile
and the full-reload payload send) unchanged.
- Around line 104-106: The reload check uses
Array.from(server.watcher.getWatched()) which returns [] because getWatched()
returns a plain object; replace Array.from(...) with
Object.entries(server.watcher.getWatched()) and keep the .some(...) logic over
([dir, files]) to locate the watched directory; also tighten the directory check
in the callback by preventing prefix false-matches—change file.startsWith(dir)
to ensure a directory boundary (e.g., file.startsWith(dir + path.sep) or
comparable check) while still using path.basename(file) to check the file name.
🧹 Nitpick comments (1)
packages/@tailwindcss-vite/src/index.ts (1)
98-115: Consider returning an empty array to suppress duplicate default HMR handling.When
handleHotUpdatedoesn't return a value, Vite continues its default HMR pipeline. For non-module files this is typically harmless, but explicitly returning[](empty modules array) after sending the full-reload makes intent clear and avoids potential double-processing.Suggested change
if (server.hot) { server.hot.send(payload) } else { server.ws.send(payload) } + return [] }
|
Thanks for the cleanup @RobinMalfait, Appreciate the help getting this ready. |
|
@Arxsher thank you for the PR! Just doing a few more tests here and then I think we can get it merged! |
This will add some integration tests for various versions of Vite to ensure that "external" (asset) files cause a full reload. The output of each Vite version is slightly different so we try to detect a few messages depending on the version.
When you use the `@source` directive in your CSS file, then internally we will call `addWatchFile` for each of the matching files. This way Vite will watch these files and trigger the `@tailwindcss/vite` plugin when things change. However, with external files we need to make sure that we trigger a full page reload to see any effect. For example when a .php file changed. The default behavior in Vite would be to trigger a hot update of the index.css file. Which is correct for updating the CSS, but since the .php file is just an asset it won't have any effect on the current page. The original logic assumed that "asset" modules (any module with type of asset and/or id=undefined) are not in the module graph, but in my testing it looks like they are part of the module graph (because we call `addWatchFile` on these files). This means that we need another way of knowing whether or not the module should result in a full page reload or not. To do this, the `handleHotUpdate` receives an array of modules which we need to analyze. For example a .js file will have a type of `js` in one module, but it will also have a type of `asset` in the other module. This is because of the `addWatchFile` that we added. Now, the `handleHotUpdate` is going to be deprecated soon, so we also updated to the `hotUpdate` API instead. If only some of the modules are asset modules, then we can let Vite do its job, aka do a hot module replacement. If all of the modules are assets, then we have to determine whether the file is part of any of the CSS file sources. We already have a concept of a "root" which maps to a CSS file handled by Tailwind CSS and we now also expose the scanned files so we can match these against the file that was updated. These roots are indexed by environment, which is one reason why we switched to the `hotUpdate` API because `this.environment` is available there. To be 100% sure, we check the current file and walk up the tree (importers) to see if any of its files is part of a Tailwind CSS root. Once we reach a root and if that root has a scanned file of the current file that changed, then we know to do a full page reload. I don't think it matters _which_ root it belonged to because we will perform a full reload anyway. I guess in theory if we changed a file where its contents is currently not on the page then maybe we shouldn't reload, but we can improve that if this is causing issues later.
|
Alright, did more testing and added integration tests. I also reworked the entire But longs story short:
Not sure what your test setup looked like, but in my testing we never did a full reload because the very first check to know whether a file was in the moduleGraph bailed out early. |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/@tailwindcss-vite/src/index.ts (2)
519-552:isScannedFile: linear scan + synchronousrealpathSyncon the hot-update path.Two performance concerns in this helper:
root.scannedFiles.includes(file)(Line 539) is O(n) over potentially thousands of watched files, and is called for every root encountered during the BFS walk. Consider exposing aSet<string>from the scanner instead of an array, which would make lookups O(1).
realpathSync(file)(Line 539) is a synchronous filesystem call on the hot-update path. If symlinks are uncommon in practice, this could be guarded (e.g., only resolve when the first check fails and the path looks like it could be a symlink) — though the current short-circuit||already limits it to the fallback case.For large monorepos with many source files, the combination could add noticeable latency to HMR.
#!/bin/bash # Check what scanner.files returns — is it an array or something else? rg -n 'get files' --type ts -A 5 | head -30 ast-grep --pattern 'class Scanner { $$$ get files() { $$$ } $$$ }'🤖 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 519 - 552, isScannedFile is doing O(n) array.includes checks against root.scannedFiles and calling realpathSync on the hot-update path; change the scanner or root construction to expose a Set<string> (e.g., scannedFilesSet) and replace root.scannedFiles.includes(...) with scannedFilesSet.has(...), and avoid synchronous realpathSync by either using a precomputed canonical path stored in the Set or only calling realpathSync as a rare fallback after the first fast check fails; update references in isScannedFile to use the new scannedFilesSet and the lazy/fallback realpath resolution to eliminate repeated synchronous filesystem calls.
156-212: ThehotUpdatehook logic looks correct and well-documented — good job handling the Vite version differences.A few observations:
- Minor: duplicate env when
this.environment.name === 'client'— the loop iterates['client', 'client']. It's harmless (returns on first match) but the intent could be clearer.- Line 164 typo: "These modules typically haven an id" → "have an id".
Suggested tweak to deduplicate environments
- for (let env of [this.environment.name, 'client']) { + for (let env of new Set([this.environment.name, 'client'])) {🤖 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 156 - 212, The hotUpdate hook iterates over [this.environment.name, 'client'] which can duplicate when this.environment.name === 'client'; change the env loop in hotUpdate to deduplicate (e.g., build environments = Array.from(new Set([this.environment.name, 'client'])) or conditionally push 'client' only when different) so the loop won’t run the same env twice, and also fix the comment typo in the hotUpdate block by replacing "haven an id" with "have an id".integrations/vite/index.test.ts (1)
524-525: Nitpick:jsxtag used for.tsfiles — consider usingtsorjsfor consistency.Other tests in this file use
tsorjstemplate tags for TypeScript source files. Usingjsxhere formain.tsandapp.tsis functionally harmless but inconsistent with the rest of the file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations/vite/index.test.ts` around lines 524 - 525, Replace the inconsistent jsx template tag used for the TypeScript fixtures with the appropriate ts (or js) template tag: update the entries for 'project-a/src/main.ts' and 'project-a/src/app.ts' to use ts instead of jsx so they match the rest of the tests and maintain consistency across the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@integrations/vite/index.test.ts`:
- Around line 524-525: Replace the inconsistent jsx template tag used for the
TypeScript fixtures with the appropriate ts (or js) template tag: update the
entries for 'project-a/src/main.ts' and 'project-a/src/app.ts' to use ts instead
of jsx so they match the rest of the tests and maintain consistency across the
file.
In `@packages/`@tailwindcss-vite/src/index.ts:
- Around line 519-552: isScannedFile is doing O(n) array.includes checks against
root.scannedFiles and calling realpathSync on the hot-update path; change the
scanner or root construction to expose a Set<string> (e.g., scannedFilesSet) and
replace root.scannedFiles.includes(...) with scannedFilesSet.has(...), and avoid
synchronous realpathSync by either using a precomputed canonical path stored in
the Set or only calling realpathSync as a rare fallback after the first fast
check fails; update references in isScannedFile to use the new scannedFilesSet
and the lazy/fallback realpath resolution to eliminate repeated synchronous
filesystem calls.
- Around line 156-212: The hotUpdate hook iterates over [this.environment.name,
'client'] which can duplicate when this.environment.name === 'client'; change
the env loop in hotUpdate to deduplicate (e.g., build environments =
Array.from(new Set([this.environment.name, 'client'])) or conditionally push
'client' only when different) so the loop won’t run the same env twice, and also
fix the comment typo in the hotUpdate block by replacing "haven an id" with
"have an id".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/@tailwindcss-vite/src/index.ts (2)
548-551:Array.prototype.includesis O(n) onscannedFiles— called twice per root match.
root.scannedFilesreturnsthis.scanner?.files, and if that's a large array (hundreds/thousands of watched files), the linear scan via.includes()will be slow. This runs on every hot-update for external files.Consider using a
SetforscannedFilesif performance becomes a concern, though for typical project sizes this is likely fine.🤖 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 548 - 551, The code calls root.scannedFiles.includes(checks.file) and .includes(checks.realpath) which does two O(n) scans on the scannedFiles array per match; change scannedFiles to a Set or create a Set view once and use set.has(...) to perform O(1) lookups (e.g., compute const scannedFilesSet = new Set(root.scannedFiles) once where the watcher/root is initialized or at start of the hot-update handler), then replace both includes checks with scannedFilesSet.has(checks.file) and scannedFilesSet.has(checks.realpath) to avoid repeated linear scans; keep references to root.scannedFiles, checks.file, and checks.realpath when making the change.
180-210: Module invalidation is always onthis.environmentregardless ofenvloop variable.When the loop is on the
'client'iteration butthis.environment.nameis something else (e.g.,'ssr'), lines 192-199 still invalidate modules onthis.environment.moduleGraph(the SSR graph), not the client graph. The module invalidation is harmless in practice since afull-reloadis sent anyway, but it's technically invalidating the wrong environment's modules.If this is intentional (i.e., we only care about triggering the reload, not precise invalidation), a brief comment would help future readers.
🤖 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 180 - 210, The invalidation is always calling this.environment.moduleGraph.invalidateModule even when env === 'client'; update the code in the loop (the block that creates invalidatedModules and iterates over modules) to pick the correct module graph based on env — use this.environment.moduleGraph when env === this.environment.name and use server.moduleGraph (or the appropriate server-side graph) when env === 'client' — then call invalidateModule on that selected graph for each mod; alternatively, if the intent is to ignore precise invalidation, add a brief clarifying comment above the invalidation block referencing this.environment.moduleGraph.invalidateModule and why always invalidating the current environment is acceptable.integrations/vite/index.test.ts (1)
489-501: Minor inconsistency:jsontag for package.json with conditional interpolation.All other tests in this file that have the
${transformer === 'lightningcss' ? ...}conditional inpackage.jsonuse thetxttemplate tag (e.g., lines 29, 106, 307, 597, 683, 785). This test usesjsoninstead. If thejsontag performs stricter parsing or normalization, this could behave differently. Consider usingtxtfor consistency.Suggested change
- 'project-a/package.json': json` - { + 'project-a/package.json': txt` + {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations/vite/index.test.ts` around lines 489 - 501, The snippet writing 'project-a/package.json' uses the json`...` template tag with a conditional interpolation (${transformer === 'lightningcss' ? ...}) which is inconsistent with other tests; change the template tag from json to txt so the block reads txt`{ ... }` to match other cases and avoid potential stricter parsing differences (refer to the package.json creation around the 'project-a/package.json' entry and the transformer variable used in the conditional).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/`@tailwindcss-vite/src/index.ts:
- Around line 526-533: The realpath getter on the checks object calls
realpathSync(file) which can throw ENOENT if the watched file was deleted
(triggered during hotUpdate); update the getter in the checks object to wrap the
realpathSync(file) call in a try/catch, and on error return the original file
path (or the passed-in file variable) and define the cached 'realpath' property
with that fallback value so subsequent accesses won't rethrow; update references
to the checks.realpath getter (used in hotUpdate) to rely on this safe fallback.
---
Nitpick comments:
In `@integrations/vite/index.test.ts`:
- Around line 489-501: The snippet writing 'project-a/package.json' uses the
json`...` template tag with a conditional interpolation (${transformer ===
'lightningcss' ? ...}) which is inconsistent with other tests; change the
template tag from json to txt so the block reads txt`{ ... }` to match other
cases and avoid potential stricter parsing differences (refer to the
package.json creation around the 'project-a/package.json' entry and the
transformer variable used in the conditional).
In `@packages/`@tailwindcss-vite/src/index.ts:
- Around line 548-551: The code calls root.scannedFiles.includes(checks.file)
and .includes(checks.realpath) which does two O(n) scans on the scannedFiles
array per match; change scannedFiles to a Set or create a Set view once and use
set.has(...) to perform O(1) lookups (e.g., compute const scannedFilesSet = new
Set(root.scannedFiles) once where the watcher/root is initialized or at start of
the hot-update handler), then replace both includes checks with
scannedFilesSet.has(checks.file) and scannedFilesSet.has(checks.realpath) to
avoid repeated linear scans; keep references to root.scannedFiles, checks.file,
and checks.realpath when making the change.
- Around line 180-210: The invalidation is always calling
this.environment.moduleGraph.invalidateModule even when env === 'client'; update
the code in the loop (the block that creates invalidatedModules and iterates
over modules) to pick the correct module graph based on env — use
this.environment.moduleGraph when env === this.environment.name and use
server.moduleGraph (or the appropriate server-side graph) when env === 'client'
— then call invalidateModule on that selected graph for each mod; alternatively,
if the intent is to ignore precise invalidation, add a brief clarifying comment
above the invalidation block referencing
this.environment.moduleGraph.invalidateModule and why always invalidating the
current environment is acceptable.
|
Thanks for the detailed breakdown and the refactor, @RobinMalfait! The standalone reproduction I used was a bit more isolated, which explains why the module graph behavior looked different there. Really glad to see the robust solution with the new |
|
Thanks for the breakdown and the refactor, @RobinMalfait! My standalone setup was likely too simplified, so I missed that those files were still hitting the module graph. The switch to the new |
2 participants
PR: Fix @source file changes not triggering full page reload on Vite 7.1+
Description
This PR addresses issue #19637 where template files (PHP, HTML, Blade, etc.) watched via the
@sourcedirective fail to trigger a full page reload when using Vite 7.1 or newer.Root Cause
Vite 7.1 introduced the Environment API, which supersedes the legacy WebSocket API for HMR. Specifically:
server.ws.sendis deprecated/ignored for certain external file updates in favor ofserver.hot.send.@tailwindcss/viteplugin currently collectsViteDevServerinstances but lacks ahandleHotUpdatehook to explicitly trigger reloads for non-module files added viaaddWatchFile.Changes
handleHotUpdatehook in the@tailwindcss/viteplugin..php,.html) but are watched by Tailwind.full-reloadusing the newserver.hot.sendAPI if available (Vite 7.1+), with a fallback toserver.ws.sendfor backward compatibility.Verification
.phpfile.server.hot.sendrestores the expected reload behavior.[ci-all]