Skip to content
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

fix(hmr): register inlined assets as a dependency of CSS file #18979

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 47 additions & 27 deletions packages/vite/src/node/plugins/css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,20 +373,20 @@ export function cssPlugin(config: ResolvedConfig): Plugin {
const resolveUrl = (url: string, importer?: string) =>
idResolver(environment, url, importer)

const urlReplacer: CssUrlReplacer = async (url, importer) => {
const urlResolver: CssUrlResolver = async (url, importer) => {
const decodedUrl = decodeURI(url)
if (checkPublicFile(decodedUrl, config)) {
if (encodePublicUrlsInCSS(config)) {
return publicFileToBuiltUrl(decodedUrl, config)
return [publicFileToBuiltUrl(decodedUrl, config), undefined]
} else {
return joinUrlSegments(config.base, decodedUrl)
return [joinUrlSegments(config.base, decodedUrl), undefined]
}
}
const [id, fragment] = decodedUrl.split('#')
let resolved = await resolveUrl(id, importer)
if (resolved) {
if (fragment) resolved += '#' + fragment
return fileToUrl(this, resolved)
return [await fileToUrl(this, resolved), resolved]
}
if (config.command === 'build') {
const isExternal = config.build.rollupOptions.external
Expand All @@ -405,7 +405,7 @@ export function cssPlugin(config: ResolvedConfig): Plugin {
)
}
}
return url
return [url, undefined]
}

const {
Expand All @@ -418,7 +418,7 @@ export function cssPlugin(config: ResolvedConfig): Plugin {
id,
raw,
preprocessorWorkerController!,
urlReplacer,
urlResolver,
)
if (modules) {
moduleCache.set(id, modules)
Expand Down Expand Up @@ -1058,17 +1058,20 @@ export function cssAnalysisPlugin(config: ResolvedConfig): Plugin {
// main import to hot update
const depModules = new Set<string | EnvironmentModuleNode>()
for (const file of pluginImports) {
depModules.add(
isCSSRequest(file)
? moduleGraph.createFileOnlyEntry(file)
: await moduleGraph.ensureEntryFromUrl(
await fileToDevUrl(
this.environment,
file,
/* skipBase */ true,
),
),
)
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))
}
}
}
moduleGraph.updateModuleInfo(
thisModule,
Expand Down Expand Up @@ -1267,7 +1270,7 @@ async function compileCSS(
id: string,
code: string,
workerController: PreprocessorWorkerController,
urlReplacer?: CssUrlReplacer,
urlResolver?: CssUrlResolver,
): Promise<{
code: string
map?: SourceMapInput
Expand All @@ -1277,7 +1280,7 @@ async function compileCSS(
}> {
const { config } = environment
if (config.css.transformer === 'lightningcss') {
return compileLightningCSS(id, code, environment, urlReplacer)
return compileLightningCSS(id, code, environment, urlResolver)
}

const { modules: modulesOptions, devSourcemap } = config.css
Expand Down Expand Up @@ -1386,10 +1389,11 @@ async function compileCSS(
)
}

if (urlReplacer) {
if (urlResolver) {
postcssPlugins.push(
UrlRewritePostcssPlugin({
replacer: urlReplacer,
resolver: urlResolver,
deps,
logger: environment.logger,
}),
)
Expand Down Expand Up @@ -1723,6 +1727,12 @@ async function resolvePostcssConfig(
return result
}

type CssUrlResolver = (
url: string,
importer?: string,
) =>
| [url: string, id: string | undefined]
| Promise<[url: string, id: string | undefined]>
type CssUrlReplacer = (
url: string,
importer?: string,
Expand All @@ -1739,7 +1749,8 @@ export const importCssRE =
const cssImageSetRE = /(?<=image-set\()((?:[\w-]{1,256}\([^)]*\)|[^)])*)(?=\))/

const UrlRewritePostcssPlugin: PostCSS.PluginCreator<{
replacer: CssUrlReplacer
resolver: CssUrlResolver
deps: Set<string>
logger: Logger
}> = (opts) => {
if (!opts) {
Expand All @@ -1763,8 +1774,13 @@ const UrlRewritePostcssPlugin: PostCSS.PluginCreator<{
const isCssUrl = cssUrlRE.test(declaration.value)
const isCssImageSet = cssImageSetRE.test(declaration.value)
if (isCssUrl || isCssImageSet) {
const replacerForDeclaration = (rawUrl: string) => {
return opts.replacer(rawUrl, importer)
const replacerForDeclaration = async (rawUrl: string) => {
const [newUrl, resolvedId] = await opts.resolver(rawUrl, importer)
// only register inlined assets to avoid frequent full refresh (#18979)
if (newUrl.startsWith('data:') && resolvedId) {
opts.deps.add(resolvedId)
}
return newUrl
}
if (isCssUrl && isCssImageSet) {
promises.push(
Expand Down Expand Up @@ -3135,7 +3151,7 @@ async function compileLightningCSS(
id: string,
src: string,
environment: PartialEnvironment,
urlReplacer?: CssUrlReplacer,
urlResolver?: CssUrlResolver,
): ReturnType<typeof compileCSS> {
const { config } = environment
const deps = new Set<string>()
Expand Down Expand Up @@ -3221,11 +3237,15 @@ async function compileLightningCSS(
css = css.replace(dep.placeholder, () => dep.url)
break
}
if (urlReplacer) {
const replaceUrl = await urlReplacer(
if (urlResolver) {
const [replaceUrl, resolvedId] = await urlResolver(
dep.url,
toAbsolute(dep.loc.filePath),
)
// only register inlined assets to avoid frequent full refresh (#18979)
if (replaceUrl.startsWith('data:') && resolvedId) {
deps.add(resolvedId)
}
css = css.replace(dep.placeholder, () => replaceUrl)
} else {
css = css.replace(dep.placeholder, () => dep.url)
Expand Down
3 changes: 3 additions & 0 deletions packages/vite/src/node/server/hmr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,9 @@ function propagateUpdate(
// 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
Expand Down
30 changes: 27 additions & 3 deletions playground/assets/__tests__/assets.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,12 +287,36 @@ describe('css url() references', () => {
})

test('url() with svg', async () => {
expect(await getBg('.css-url-svg')).toMatch(/data:image\/svg\+xml,.+/)
const bg = await getBg('.css-url-svg')
expect(bg).toMatch(/data:image\/svg\+xml,.+/)
expect(bg).toContain('blue')
expect(bg).not.toContain('red')

if (isServe) {
editFile('nested/fragment-bg-hmr.svg', (code) =>
code.replace('fill="blue"', 'fill="red"'),
)
await untilUpdated(() => getBg('.css-url-svg'), 'red')
}
})

test('image-set() with svg', async () => {
expect(await getBg('.css-image-set-svg')).toMatch(/data:image\/svg\+xml,.+/)
})

test('url() with svg in .css?url', async () => {
const bg = await getBg('.css-url-svg-in-url')
expect(bg).toMatch(/data:image\/svg\+xml,.+/)
expect(bg).toContain('blue')
expect(bg).not.toContain('red')

if (isServe) {
editFile('nested/fragment-bg-hmr2.svg', (code) =>
code.replace('fill="blue"', 'fill="red"'),
)
await untilUpdated(() => getBg('.css-url-svg'), 'red')
}
})
})

describe('image', () => {
Expand Down Expand Up @@ -546,8 +570,8 @@ test.runIf(isBuild)('manifest', async () => {

for (const file of listAssets('foo')) {
if (file.endsWith('.css')) {
// ignore icons-*.css as it's imported with ?url
if (file.includes('icons-')) continue
// ignore icons-*.css and css-url-url-*.css as it's imported with ?url
if (file.includes('icons-') || file.includes('css-url-url-')) continue
expect(entry.css).toContain(`assets/${file}`)
} else if (!file.endsWith('.js')) {
expect(entry.assets).toContain(`assets/${file}`)
Expand Down
4 changes: 4 additions & 0 deletions playground/assets/css/css-url-url.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.css-url-svg-in-url {
background: url(../nested/fragment-bg-hmr2.svg);
background-size: 10px;
}
2 changes: 1 addition & 1 deletion playground/assets/css/css-url.css

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions playground/assets/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ <h2>CSS url references</h2>
<div class="css-url-svg">
<span style="background: #fff">CSS SVG background</span>
</div>
<div class="css-url-svg-in-url">
<span style="background: #fff">CSS (?url) SVG background</span>
</div>

<div class="css-image-set-svg">
<span style="background: #fff">CSS SVG background with image-set</span>
Expand Down Expand Up @@ -528,6 +531,12 @@ <h3>assets in template</h3>
import cssUrl from './css/icons.css?url'
text('.url-css', cssUrl)

import cssUrlUrl from './css/css-url-url.css?url'
const linkTag = document.createElement('link')
linkTag.href = cssUrlUrl
linkTag.rel = 'stylesheet'
document.body.appendChild(linkTag)

// const url = new URL('non_existent_file.png', import.meta.url)
const metaUrl = new URL('./nested/asset.png', import.meta.url)
text('.import-meta-url', metaUrl)
Expand Down
21 changes: 21 additions & 0 deletions playground/assets/nested/fragment-bg-hmr.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
21 changes: 21 additions & 0 deletions playground/assets/nested/fragment-bg-hmr2.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading