From d199621fd4ec254708ed6cb04da7b278976ac713 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bartosz=20Go=C5=9Bci=C5=84ski?= <bargosc@gmail.com>
Date: Tue, 13 Sep 2022 20:12:41 +0200
Subject: [PATCH] fix: differentiate between rel=preload and rel=stylesheet for
 CSS files

---
 .../src/node/plugins/importAnalysisBuild.ts   | 15 +++--
 .../__tests__/css-dynamic-import.spec.ts      | 65 ++++++++++++++-----
 playground/css-dynamic-import/index.js        |  5 ++
 3 files changed, 63 insertions(+), 22 deletions(-)

diff --git a/packages/vite/src/node/plugins/importAnalysisBuild.ts b/packages/vite/src/node/plugins/importAnalysisBuild.ts
index d5ca19f0141d54..ed8d8853e7680a 100644
--- a/packages/vite/src/node/plugins/importAnalysisBuild.ts
+++ b/packages/vite/src/node/plugins/importAnalysisBuild.ts
@@ -65,13 +65,7 @@ function preload(
     return baseModule()
   }
 
-  const absoluteUrls = new Set<string>()
   const links = document.getElementsByTagName('link')
-  for (let i = links.length - 1; i >= 0; 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
-    absoluteUrls.add(links[i].href)
-  }
 
   return Promise.all(
     deps.map((dep) => {
@@ -89,7 +83,14 @@ function preload(
       if (isBaseRelative) {
         // When isBaseRelative is true then we have `importerUrl` and `dep` is
         // already converted to an absolute URL by the `assetsURL` function
-        if (absoluteUrls.has(dep)) return
+        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
       }
diff --git a/playground/css-dynamic-import/__tests__/css-dynamic-import.spec.ts b/playground/css-dynamic-import/__tests__/css-dynamic-import.spec.ts
index 9a308efb8e8758..56757fc293dbba 100644
--- a/playground/css-dynamic-import/__tests__/css-dynamic-import.spec.ts
+++ b/playground/css-dynamic-import/__tests__/css-dynamic-import.spec.ts
@@ -12,7 +12,8 @@ const getConfig = (base: string): InlineConfig => ({
   base,
   root: rootDir,
   logLevel: 'silent',
-  preview: { port: ports['css/dynamic-import'] }
+  preview: { port: ports['css/dynamic-import'] },
+  build: { assetsInlineLimit: 0 }
 })
 
 async function withBuild(base: string, fn: () => Promise<void>) {
@@ -42,14 +43,17 @@ async function withServe(base: string, fn: () => Promise<void>) {
   }
 }
 
-async function getChunks() {
+async function getLinks() {
   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}`
-  })
+  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 }) => {
@@ -60,12 +64,37 @@ baseOptions.forEach(({ base, label }) => {
         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'
+        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'
+          }
         ])
       })
     }
@@ -79,7 +108,13 @@ baseOptions.forEach(({ base, label }) => {
 
         expect(await getColor('.css-dynamic-import')).toBe('green')
         // in serve there is no preloading
-        expect(await getChunks()).toEqual([])
+        expect(await getLinks()).toEqual([
+          {
+            pathname: '/dynamic.css',
+            rel: 'preload',
+            as: 'style'
+          }
+        ])
       })
     }
   )
diff --git a/playground/css-dynamic-import/index.js b/playground/css-dynamic-import/index.js
index 9115d2b4aa007e..5a0c724da737db 100644
--- a/playground/css-dynamic-import/index.js
+++ b/playground/css-dynamic-import/index.js
@@ -1,5 +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()
 })