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

Author: bgoscinski

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

@@ -65,6 +65,8 @@ function preload(
     return baseModule()
   }
 
+  const links = document.getElementsByTagName('link')
+
   return Promise.all(
     deps.map((dep) => {
       // @ts-ignore
@@ -75,10 +77,24 @@ function preload(
       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}`)) {
+      const isBaseRelative = !!importerUrl
+
+      // check if the file is already preloaded by SSR markup
+      if (isBaseRelative) {
+        // When isBaseRelative is true then we have `importerUrl` and `dep` is
+        // already converted to an absolute URL by the `assetsURL` function
+        for (let i = links.length - 1; i >= 0; i--) {
+          const link = links[i]
+          // The `links[i].href` is an absolute URL thanks to browser doing the work
+          // for us. See https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#reflecting-content-attributes-in-idl-attributes:idl-domstring-5
+          if (link.href === dep && (!isCss || link.rel === 'stylesheet')) {
+            return
+          }
+        }
+      } else if (document.querySelector(`link[href="${dep}"]${cssSelector}`)) {
         return
       }
+
       // @ts-ignore
       const link = document.createElement('link')
       // @ts-ignore

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

@@ -0,0 +1,121 @@
+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'] },
+  build: { assetsInlineLimit: 0 }
+})
+
+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 getLinks() {
+  const links = await page.$$('link')
+  return await Promise.all(
+    links.map((handle) => {
+      return handle.evaluate((link) => ({
+        pathname: new URL(link.href).pathname,
+        rel: link.rel,
+        as: link.as
+      }))
+    })
+  )
+}
+
+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 getLinks()).toEqual([
+          {
+            pathname: expect.stringMatching(/^\/assets\/index\..+\.css$/),
+            rel: 'stylesheet',
+            as: ''
+          },
+          {
+            pathname: expect.stringMatching(/^\/assets\/dynamic\..+\.css$/),
+            rel: 'preload',
+            as: 'style'
+          },
+          {
+            pathname: expect.stringMatching(/^\/assets\/dynamic\..+\.js$/),
+            rel: 'modulepreload',
+            as: 'script'
+          },
+          {
+            pathname: expect.stringMatching(/^\/assets\/dynamic\..+\.css$/),
+            rel: 'stylesheet',
+            as: ''
+          },
+          {
+            pathname: expect.stringMatching(/^\/assets\/static\..+\.js$/),
+            rel: 'modulepreload',
+            as: 'script'
+          },
+          {
+            pathname: expect.stringMatching(/^\/assets\/index\..+\.js$/),
+            rel: 'modulepreload',
+            as: 'script'
+          }
+        ])
+      })
+    }
+  )
+
+  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 getLinks()).toEqual([
+          {
+            pathname: '/dynamic.css',
+            rel: 'preload',
+            as: 'style'
+          }
+        ])
+      })
+    }
+  )
+})

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,10 @@
+import './static.js'
+
+const link = document.head.appendChild(document.createElement('link'))
+link.rel = 'preload'
+link.as = 'style'
+link.href = new URL('./dynamic.css', import.meta.url).href
+
+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,