fix: don't duplicate styles with dynamic import (fix #9967)

packages/vite/src/node/plugins/importAnalysisBuild.ts

@@ -74,11 +74,24 @@ function preload(
       // @ts-ignore
       seen[dep] = true
       const isCss = dep.endsWith('.css')
-      const cssSelector = isCss ? '[rel="stylesheet"]' : ''
-      // @ts-ignore check if the file is already preloaded by SSR markup
-      if (document.querySelector(`link[href="${dep}"]${cssSelector}`)) {
-        return
+      const separatorIdx = dep.lastIndexOf('/')
+      const shortDep = separatorIdx < 0 ? dep : dep.slice(separatorIdx + 1)
+      const cssSelector = isCss ? '[rel=stylesheet]' : ''
+      const linkSelector = `link[href$="${shortDep}"]${cssSelector}`
+      const links = document.querySelectorAll<HTMLLinkElement>(linkSelector)
+      // When relativePreloadUrls is false then dep looks like /assets/foo.js
+      // and importerUrl === undefined so to get absolute URL with origin
+      // (https://blah.com/assets/foo.js) we need to provide location.href as a
+      // base to URL constructor. When relativePreloadUrls is true then we have
+      // importerUrl and dep is already converted to a full absolute URL by the
+      // assetsURL function
+      const depWithOrigin = new URL(dep, location.href).href
+      for (let i = links.length - 1; i >= 0; i--) {
+        // link.href is normalized by the browser to an absolute URL with origin
+        // even if link.getAttribute('href') is relative
+        if (links[i].href === depWithOrigin) return
       }
+
       // @ts-ignore
       const link = document.createElement('link')
       // @ts-ignore

playground/css-dynamic-import/__tests__/css-dynamic-import.spec.ts

@@ -0,0 +1,86 @@
+import type { InlineConfig } from 'vite'
+import { build, createServer, preview } from 'vite'
+import { expect, test } from 'vitest'
+import { getColor, isBuild, isServe, page, ports, rootDir } from '~utils'
+
+const baseOptions = [
+  { base: '', label: 'relative' },
+  { base: '/', label: 'absolute' }
+]
+
+const getConfig = (base: string): InlineConfig => ({
+  base,
+  root: rootDir,
+  logLevel: 'silent',
+  preview: { port: ports['css/dynamic-import'] }
+})
+
+async function withBuild(base: string, fn: () => Promise<void>) {
+  const config = getConfig(base)
+  await build(config)
+  const server = await preview(config)
+
+  try {
+    await page.goto(server.resolvedUrls.local[0])
+    await fn()
+  } finally {
+    server.httpServer.close()
+  }
+}
+
+async function withServe(base: string, fn: () => Promise<void>) {
+  const config = getConfig(base)
+  const server = await createServer(config)
+  await server.listen()
+  await new Promise((r) => setTimeout(r, 500))
+
+  try {
+    await page.goto(server.resolvedUrls.local[0])
+    await fn()
+  } finally {
+    await server.close()
+  }
+}
+
+async function getChunks() {
+  const links = await page.$$('link')
+  const hrefs = await Promise.all(links.map((l) => l.evaluate((el) => el.href)))
+  return hrefs.map((href) => {
+    // drop hash part from the file name
+    const [_, name, ext] = href.match(/assets\/([a-z]+)\..*?\.(.*)$/)
+    return `${name}.${ext}`
+  })
+}
+
+baseOptions.forEach(({ base, label }) => {
+  test.runIf(isBuild)(
+    `doesn't duplicate dynamically imported css files when built with ${label} base`,
+    async () => {
+      await withBuild(base, async () => {
+        await page.waitForSelector('.loaded', { state: 'attached' })
+
+        expect(await getColor('.css-dynamic-import')).toBe('green')
+        expect(await getChunks()).toEqual([
+          'index.css',
+          'dynamic.js',
+          'dynamic.css',
+          'static.js',
+          'index.js'
+        ])
+      })
+    }
+  )
+
+  test.runIf(isServe)(
+    `doesn't duplicate dynamically imported css files when served with ${label} base`,
+    async () => {
+      await withServe(base, async () => {
+        await page.waitForSelector('.loaded', { state: 'attached' })
+
+        expect(await getColor('.css-dynamic-import')).toBe('green')
+        // in serve there is no preloading
+        expect(await getChunks()).toEqual([])
+      })
+    }
+  )
+})

playground/css-dynamic-import/__tests__/serve.ts

@@ -0,0 +1,10 @@
+// this is automatically detected by playground/vitestSetup.ts and will replace
+// the default e2e test serve behavior
+
+// The server is started in the test, so we need to have a custom serve
+// function or a default server will be created
+export async function serve() {
+  return {
+    close: () => Promise.resolve()
+  }
+}

playground/css-dynamic-import/dynamic.css

@@ -0,0 +1,3 @@
+.css-dynamic-import {
+  color: green;
+}

playground/css-dynamic-import/dynamic.js

@@ -0,0 +1,6 @@
+import './dynamic.css'
+
+export const lazyLoad = async () => {
+  await import('./static.js')
+  document.body.classList.add('loaded')
+}

playground/css-dynamic-import/index.html

@@ -0,0 +1,3 @@
+<p class="css-dynamic-import">This should be green</p>
+
+<script type="module" src="./index.js"></script>

playground/css-dynamic-import/index.js

@@ -0,0 +1,5 @@
+import './static.js'
+
+import('./dynamic.js').then(async ({ lazyLoad }) => {
+  await lazyLoad()
+})

playground/css-dynamic-import/static.css

@@ -0,0 +1,3 @@
+.css-dynamic-import {
+  color: red;
+}

playground/css-dynamic-import/static.js

@@ -0,0 +1,3 @@
+import './static.css'
+
+export const foo = 'foo'

playground/test-utils.ts

@@ -29,7 +29,8 @@ export const ports = {
   'ssr-vue': 9604,
   'ssr-webworker': 9605,
   'css/postcss-caching': 5005,
-  'css/postcss-plugins-different-dir': 5006
+  'css/postcss-plugins-different-dir': 5006,
+  'css/dynamic-import': 5007
 }
 export const hmrPorts = {
   'optimize-missing-deps': 24680,