-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Avoid full reload when CSS-like HMR is handled by another plugin #19904
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e6833b7
4391793
2c1c351
efd5ecd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -584,6 +584,174 @@ describe.each(['postcss', 'lightningcss'])('%s', (transformer) => { | |
| }, | ||
| ) | ||
|
|
||
| ;(transformer === 'postcss' ? test : test.skip)( | ||
| 'css-like scanned file changes do not force a full reload when another plugin handles CSS HMR', | ||
| { | ||
| fs: { | ||
| 'package.json': json`{}`, | ||
| 'pnpm-workspace.yaml': yaml` | ||
| # | ||
| packages: | ||
| - project-a | ||
| `, | ||
| 'project-a/package.json': json` | ||
| { | ||
| "type": "module", | ||
| "dependencies": { | ||
| "@tailwindcss/vite": "workspace:^", | ||
| "tailwindcss": "workspace:^" | ||
| }, | ||
| "devDependencies": { | ||
| ${transformer === 'lightningcss' ? `"lightningcss": "^1",` : ''} | ||
| "vite": "^8" | ||
| } | ||
| } | ||
| `, | ||
| 'project-a/vite.config.ts': ts` | ||
| import fs from 'node:fs' | ||
| import fsp from 'node:fs/promises' | ||
| import path from 'node:path' | ||
| import tailwindcss from '@tailwindcss/vite' | ||
| import { fileURLToPath } from 'node:url' | ||
| import { defineConfig, normalizePath } from 'vite' | ||
|
|
||
| function appendLog(file, payload) { | ||
| fs.appendFileSync(file, JSON.stringify(payload) + '\\n', 'utf8') | ||
| } | ||
|
|
||
| function hmrWiretap(logFile) { | ||
| return { | ||
| name: 'hmr-wiretap', | ||
| configureServer(server) { | ||
| fs.writeFileSync(logFile, '', 'utf8') | ||
|
|
||
| const originalWsSend = server.ws.send.bind(server.ws) | ||
| server.ws.send = ((payload, ...args) => { | ||
| appendLog(logFile, { source: 'server.ws.send', payload }) | ||
| return originalWsSend(payload, ...args) | ||
| }) as typeof server.ws.send | ||
|
|
||
| for (const [environmentName, environment] of Object.entries(server.environments)) { | ||
| const originalHotSend = environment.hot.send.bind(environment.hot) | ||
| environment.hot.send = ((payload) => { | ||
|
Comment on lines
+629
to
+636
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The |
||
| appendLog(logFile, { | ||
| source: 'environment.hot.send', | ||
| environmentName, | ||
| payload, | ||
| }) | ||
| return originalHotSend(payload) | ||
| }) as typeof environment.hot.send | ||
| } | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| function componentStylePlugin() { | ||
| let probeFile = '' | ||
| let wrapperFile = '' | ||
|
|
||
| return { | ||
| name: 'component-style-plugin', | ||
| enforce: 'pre', | ||
| configResolved(config) { | ||
| probeFile = normalizePath(path.resolve(config.root, 'src/probe.component.css')) | ||
| wrapperFile = normalizePath(path.resolve(config.root, 'src/component-wrapper.css')) | ||
| }, | ||
| async transform(_, id) { | ||
| if (normalizePath(id.split('?')[0]) !== wrapperFile) return | ||
|
|
||
| this.addWatchFile(probeFile) | ||
| const content = await fsp.readFile(probeFile, 'utf8') | ||
| return [ | ||
| "@import 'tailwindcss';", | ||
| "@source './probe.component.css';", | ||
| content, | ||
| ].join('\\n') | ||
| }, | ||
| hotUpdate({ file, timestamp }) { | ||
| if (normalizePath(file) !== probeFile) return | ||
|
|
||
| this.environment.hot.send({ | ||
| type: 'update', | ||
| updates: [ | ||
| { | ||
| type: 'css-update', | ||
| path: '/src/component-wrapper.css', | ||
| acceptedPath: '/src/component-wrapper.css', | ||
| timestamp, | ||
| }, | ||
| ], | ||
| }) | ||
|
|
||
| return [] | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| export default defineConfig({ | ||
| css: ${transformer === 'postcss' ? '{}' : "{ transformer: 'lightningcss' }"}, | ||
| build: { cssMinify: false }, | ||
| logLevel: 'info', | ||
| plugins: [ | ||
| tailwindcss(), | ||
| componentStylePlugin(), | ||
| hmrWiretap(path.resolve(path.dirname(fileURLToPath(import.meta.url)), 'hmr.log')), | ||
| ], | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| }) | ||
| `, | ||
| 'project-a/index.html': html` | ||
| <html> | ||
| <head> | ||
| <link rel="stylesheet" href="./src/component-wrapper.css" /> | ||
| </head> | ||
| <body> | ||
| <div class="probe font-bold">Hello</div> | ||
| </body> | ||
| </html> | ||
| `, | ||
| 'project-a/src/component-wrapper.css': css` | ||
| /* transformed by componentStylePlugin */ | ||
| `, | ||
| 'project-a/src/probe.component.css': css` | ||
| .probe { | ||
| @apply bg-blue-500; | ||
| } | ||
| `, | ||
| }, | ||
| }, | ||
| async ({ root, spawn, fs, expect }) => { | ||
| let process = await spawn('pnpm vite dev --debug hmr', { | ||
| cwd: path.join(root, 'project-a'), | ||
| }) | ||
| await process.onStdout((m) => m.includes('ready in')) | ||
|
|
||
| let url = '' | ||
| await process.onStdout((m) => { | ||
| let match = /Local:\s*(http.*)\//.exec(m) | ||
| if (match) url = match[1] | ||
| return Boolean(url) | ||
| }) | ||
|
|
||
| await fetchStyles(url, '/index.html') | ||
|
|
||
| await fs.write('project-a/hmr.log', '') | ||
| await fs.write( | ||
| 'project-a/src/probe.component.css', | ||
| css` | ||
| .probe { | ||
| @apply bg-red-500; | ||
| } | ||
| `, | ||
| ) | ||
|
|
||
| await retryAssertion(async () => { | ||
| let log = await fs.read('project-a/hmr.log') | ||
| expect(log).toContain('"type":"update"') | ||
| expect(log).not.toContain('"type":"full-reload"') | ||
| }) | ||
| }, | ||
| ) | ||
|
|
||
| test( | ||
| `source(none) disables looking at the module graph`, | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -315,6 +315,16 @@ export default function tailwindcss(opts: PluginOptions = {}): Plugin[] { | |
| ) | ||
| } | ||
|
|
||
| // CSS-like files may still be handled by another plugin's stylesheet | ||
| // HMR pipeline even when the module graph only exposes asset-like | ||
| // placeholder modules during this pass. We still need to invalidate | ||
| // the watched modules so Tailwind rebuilds, but we should not force | ||
| // a full page reload that can race against a later targeted | ||
| // css-update payload. | ||
| if (isPotentialCssRootFile(file)) { | ||
| return [] | ||
| } | ||
|
|
||
|
Comment on lines
+318
to
+327
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test is passing (good!), but getting rid of this code doesn't fail the integration test, so either the test is not testing what it's supposed to test, or this isn't the actual fix. |
||
| if (env === this.environment.name) { | ||
| this.environment.hot.send({ type: 'full-reload' }) | ||
| } else if (server.hot.send) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lightningcsswithout documented justificationThe production fix in
index.tsis transformer-agnostic: theisPotentialCssRootFilecheck runs identically regardless of whether PostCSS or LightningCSS processes the CSS. Skipping the test for thelightningcssvariant leaves an untested path — if the fix ever regresses for LightningCSS users, this test would not catch it. If the Analog repro is genuinely PostCSS-only (e.g., becausecomponentStylePlugininteracts with the PostCSS pipeline specifically), a brief comment explaining whylightningcssis excluded would help future reviewers.