Do not force full page reloads when using @tailwindcss/vite - #20414
Conversation
We're not doing full reloads ourselves anymore
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Reviews (2): Last reviewed commit: "update CHANGELOG" | Re-trigger Greptile |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe Vite plugin removes server tracking and external-file full-reload handling. Existing integration tests now verify CSS HMR updates, with exceptions for Vite 6 and 7.0.8. A new Preact test verifies initial styles, component HMR updates, candidate rescanning after 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
integrations/vite/preact.test.ts (2)
32-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConfirm that the
hmr.logpath matches the test working directory.The wiretap plugin writes to
path.resolve('hmr.log'), which resolves against the dev server process cwd. The test readshmr.logrelative to the test root. Preferserver.config.rootso the log path stays stable if the spawn cwd changes.♻️ Proposed change to anchor the log path
configureServer(server) { - let logFile = path.resolve('hmr.log') + let logFile = path.resolve(server.config.root, 'hmr.log') fs.writeFileSync(logFile, '')
130-163: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the update payload for the rescanned candidate.
This block asserts that the CSS contains the new candidate and that no
full-reloadpayload was sent. Add an assertion that an"type":"update"payload was sent after thepackage.jsonwrite, so a silent full page refresh by other means cannot pass the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1c00d1b2-27ca-4874-8da6-68446158a979
📒 Files selected for processing (4)
integrations/vite/index.test.tsintegrations/vite/preact.test.tspackages/@tailwindcss-vite/src/index.test.tspackages/@tailwindcss-vite/src/index.ts
💤 Files with no reviewable changes (1)
- packages/@tailwindcss-vite/src/index.test.ts
This PR removes all of the custom HMR handling we had in the
@tailwindcss/viteplugin.When Vite 7.1 was introduced, Vite stopped performing a full page reload for unknown files and instead started performing normal
hmrupdates. This resulted in this issue: #19637At the time, it felt like something we could easily re-add: if a file is not covered by Vite, we can perform a
full-reload. This meant that a.phpfile would trigger a full page reload as expected.The reason the
.phpfile triggered Vite in the first place is because those files were scanned by us (@tailwindcss/vite) so it made sense.However, this then resulted in a plethora of issues, and it feels a bit like a game of whac-a-mole.
@tailwindcss/vitecrashes on every edit under Vite'sexperimental.bundledDev#20378Fixes: #19744
Fixes: #19903
Fixes: #20320
Fixes: #20378
Fixes: #20411
We kept updating the logic by safelisting certain extensions, checking different servers and/or environments, handling the fact that
serverin the callback could be absent inexperimental.bundledDevmode, etc. etc.Now, when investigating the last issue (#20411), I can trigger full reloads by changing
.json,.yamlor.svgfiles. This makes sense since they aren't handled by default.So thinking about this more, I think it's just not Tailwind's responsibility to tell Vite to reload the browser or not. Yes, we use the
addWatchFileAPI, so files are being watched because of us. However, our only goal is to update the.cssfile (and HMR that).This means that we can just drop all the custom HMR handling we have in
@tailwindcss/vite.This also means that #19637 would regress and won't trigger full page reloads. But this can be easily handled by a plugin responsible for this behavior:
Test plan