-
-
Notifications
You must be signed in to change notification settings - Fork 7.5k
fix(hmr): register css deps as type: asset
#20391
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
Changes from 7 commits
f0432f4
82a73a2
c30247f
30fa462
9eea422
0f1a8dd
f61267f
02f1fe2
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 |
|---|---|---|
|
|
@@ -98,7 +98,6 @@ import { addToHTMLProxyTransformResult } from './html' | |
| import { | ||
| assetUrlRE, | ||
| cssEntriesMap, | ||
| fileToDevUrl, | ||
| fileToUrl, | ||
| publicAssetUrlCache, | ||
| publicAssetUrlRE, | ||
|
|
@@ -1096,20 +1095,7 @@ export function cssAnalysisPlugin(config: ResolvedConfig): Plugin { | |
| // main import to hot update | ||
| const depModules = new Set<string | EnvironmentModuleNode>() | ||
| for (const file of pluginImports) { | ||
| if (isCSSRequest(file)) { | ||
| depModules.add(moduleGraph.createFileOnlyEntry(file)) | ||
| } else { | ||
| const url = await fileToDevUrl( | ||
| this.environment, | ||
| file, | ||
| /* skipBase */ true, | ||
| ) | ||
| if (url.startsWith('data:')) { | ||
| depModules.add(moduleGraph.createFileOnlyEntry(file)) | ||
| } else { | ||
| depModules.add(await moduleGraph.ensureEntryFromUrl(url)) | ||
| } | ||
| } | ||
| depModules.add(moduleGraph.createFileOnlyEntry(file)) | ||
|
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. This is great! I just explored the case in vitejs/vite-plugin-react#616, so let me share what I found. Client hot update looks like this when changing server component: before after In my case, I already had filter based on id The double slash in |
||
| } | ||
| moduleGraph.updateModuleInfo( | ||
| thisModule, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,12 +10,7 @@ import type { | |
| InvokeSendData, | ||
| } from '../../shared/invokeMethods' | ||
| import { CLIENT_DIR } from '../constants' | ||
| import { | ||
| createDebugger, | ||
| isCSSRequest, | ||
| monotonicDateNow, | ||
| normalizePath, | ||
| } from '../utils' | ||
| import { createDebugger, monotonicDateNow, normalizePath } from '../utils' | ||
| import type { InferCustomEventPayload, ViteDevServer } from '..' | ||
| import { getHookHandler } from '../plugins' | ||
| import { isExplicitImportRequired } from '../plugins/importAnalysis' | ||
|
|
@@ -73,7 +68,7 @@ export interface HmrContext { | |
| } | ||
|
|
||
| interface PropagationBoundary { | ||
| boundary: EnvironmentModuleNode | ||
| boundary: EnvironmentModuleNode & { type: 'js' | 'css' } | ||
| acceptedVia: EnvironmentModuleNode | ||
| isWithinCircularImport: boolean | ||
| } | ||
|
|
@@ -693,7 +688,16 @@ export function updateModules( | |
| ) | ||
| } | ||
|
|
||
| if (needFullReload) { | ||
| // html file cannot be hot updated | ||
sapphi-red marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| const isClientHtmlChange = | ||
| file.endsWith('.html') && | ||
| environment.name === 'client' && | ||
| // if the html file is imported as a module, we assume that this file is | ||
| // not used as the template for top-level request response | ||
| // (i.e. not used by the middleware). | ||
| modules.every((mod) => mod.type !== 'js') | ||
|
|
||
| if (needFullReload || isClientHtmlChange) { | ||
| const reason = | ||
| typeof needFullReload === 'string' | ||
| ? colors.dim(` (${needFullReload})`) | ||
|
|
@@ -705,6 +709,12 @@ export function updateModules( | |
| hot.send({ | ||
| type: 'full-reload', | ||
| triggeredBy: path.resolve(environment.config.root, file), | ||
| path: | ||
| !isClientHtmlChange || | ||
| environment.config.server.middlewareMode || | ||
| updates.length > 0 // if there's an update, other URLs may be affected | ||
| ? '*' | ||
| : '/' + file, | ||
| }) | ||
| return | ||
| } | ||
|
|
@@ -761,25 +771,13 @@ function propagateUpdate( | |
| } | ||
|
|
||
| if (node.isSelfAccepting) { | ||
| // isSelfAccepting is only true for js and css | ||
| const boundary = node as EnvironmentModuleNode & { type: 'js' | 'css' } | ||
| boundaries.push({ | ||
| boundary: node, | ||
| acceptedVia: node, | ||
| boundary, | ||
| acceptedVia: boundary, | ||
| isWithinCircularImport: isNodeWithinCircularImports(node, currentChain), | ||
| }) | ||
|
|
||
| // additionally check for CSS importers, since a PostCSS plugin like | ||
| // Tailwind JIT may register any file as a dependency to a CSS file. | ||
| for (const importer of node.importers) { | ||
| if (isCSSRequest(importer.url) && !currentChain.includes(importer)) { | ||
| propagateUpdate( | ||
| importer, | ||
| traversedModules, | ||
| boundaries, | ||
| currentChain.concat(importer), | ||
| ) | ||
| } | ||
| } | ||
|
Comment on lines
-770
to
-781
Member
Author
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. This part is no longer needed. |
||
|
|
||
| return false | ||
| } | ||
|
|
||
|
|
@@ -789,36 +787,29 @@ function propagateUpdate( | |
| // Also, the imported module (this one) must be updated before the importers, | ||
| // so that they do get the fresh imported module when/if they are reloaded. | ||
| if (node.acceptedHmrExports) { | ||
| // acceptedHmrExports is only true for js and css | ||
| const boundary = node as EnvironmentModuleNode & { type: 'js' | 'css' } | ||
| boundaries.push({ | ||
| boundary: node, | ||
| acceptedVia: node, | ||
| boundary, | ||
| acceptedVia: boundary, | ||
| isWithinCircularImport: isNodeWithinCircularImports(node, currentChain), | ||
| }) | ||
| } else { | ||
| if (!node.importers.size) { | ||
| return true | ||
| } | ||
|
|
||
| // #3716, #3913 | ||
| // For a non-CSS file, if all of its importers are CSS files (registered via | ||
| // PostCSS plugins) it should be considered a dead end and force full reload. | ||
| if ( | ||
| !isCSSRequest(node.url) && | ||
| // we assume .svg is never an entrypoint and does not need a full reload | ||
| // to avoid frequent full reloads when an SVG file is referenced in CSS files (#18979) | ||
| !node.file?.endsWith('.svg') && | ||
| [...node.importers].every((i) => isCSSRequest(i.url)) | ||
| ) { | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| for (const importer of node.importers) { | ||
| const subChain = currentChain.concat(importer) | ||
|
|
||
| if (importer.acceptedHmrDeps.has(node)) { | ||
| // acceptedHmrDeps has value only for js and css | ||
| const boundary = importer as EnvironmentModuleNode & { | ||
| type: 'js' | 'css' | ||
| } | ||
| boundaries.push({ | ||
| boundary: importer, | ||
| boundary, | ||
| acceptedVia: node, | ||
| isWithinCircularImport: isNodeWithinCircularImports(importer, subChain), | ||
| }) | ||
|
|
@@ -886,11 +877,6 @@ function isNodeWithinCircularImports( | |
| // Node may import itself which is safe | ||
| if (importer === node) continue | ||
|
|
||
| // a PostCSS plugin like Tailwind JIT may register | ||
| // any file as a dependency to a CSS file. | ||
| // But in that case, the actual dependency chain is separate. | ||
| if (isCSSRequest(importer.url)) continue | ||
|
|
||
| // Check circular imports | ||
| const importerIndex = nodeChain.indexOf(importer) | ||
| if (importerIndex > -1) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| <div>partial</div> |
Uh oh!
There was an error while loading. Please reload this page.
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.
Since this line is removed, #19563 and #19786 will be fixed.