vite icon indicating copy to clipboard operation
vite copied to clipboard

Published 20 hours ago • verified publisher icon vitejs

Relative `base` option + dynamic import leads to styles injected two times in production build

Open bgoscinski opened this issue 3 years ago • 5 comments

Describe the bug

When base is set to relative (empty string or './') then in prod build when dynamically importing a module that has some CSS dependencies these dependencies will be appended second time to <head> leading to wrong styles. In vite serve scenario this seem to work fine.

I managed to find possible root cause:

When base is absolute then the generated assetsUrl returns absolute URLs without origin part which match the ones vite embeds into index.html 👍:

// ./assets/index.888728d5.js
const assetsURL = function(dep) {
  return "/" + dep;
};

// index.html
<script type="module" crossorigin src="/assets/index.888728d5.js"></script>
<link rel="stylesheet" href="/assets/index.6744de28.css">

However when base is relative then assetsUrl returns absolute URLs with origin part and vite embeds relative URLs into index.html 🐛:

// ./assets/index.46a59a36.js
const assetsURL = function(dep, importerUrl) {
  // This returns for example 'https://some.origin/foo/bar/assets/index.6744de28.css'
  return new URL(dep, importerUrl).href;
};

// index.html
<script type="module" crossorigin src="./assets/index.46a59a36.js"></script>
<link rel="stylesheet" href="./assets/index.6744de28.css">

This mismatch in relative base case causes __vitePreload to think that given asset was not yet loaded because dep is an absolute URL with origin and index.html has relative URL to the file.

// __vitePreload function
if (document.querySelector(`link[href="${dep}"]${cssSelector}`)) {
  return; // <=========== This early return is not reached
}

To reproduce this bug

  1. Clone https://github.com/bgoscinski/vite-relative-base-plus-dynamic-import-repro
  2. npm i && npx vite build && npx vite preview
  3. Open the preview. Background should be green
  4. Click the button. Background should remain green but it changes to red 🐛. This happens because there is a second linkto the same CSS file at the bottom of <head>. This second one "wins" the style cascade now :( image
  5. Kill the preview
  6. Remove base option from vite.config.js
  7. npx vite build && npx vite preview
  8. Open the preview. Background should be green
  9. Click the button. Background remains green 👍

Reproduction

https://github.com/bgoscinski/vite-relative-base-plus-dynamic-import-repro

System Info

$ npx envinfo --system --npmPackages '{vite,@vitejs/*}' --binaries --browsers

  System:
    OS: Windows 10 10.0.19044
    CPU: (8) x64 Intel(R) Core(TM) i5-9300H CPU @ 2.40GHz
    Memory: 6.84 GB / 15.86 GB
  Binaries:
    Node: 16.13.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 8.19.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 104.0.5112.102
    Edge: Spartan (44.19041.1266.0), Chromium (104.0.1293.70)
    Internet Explorer: 11.0.19041.1566
  npmPackages:
    vite: ^3.0.9 => 3.0.9

Used Package Manager

npm

Logs

No response

Validations

bgoscinski avatar Sep 02 '22 13:09 bgoscinski

@bgoscinski great findings! Let us know if you would like to work on a PR to fix this 👍🏼

patak-dev avatar Sep 02 '22 13:09 patak-dev

Sure, I can get my hands dirty, but I'd need some guidance about how should I fix it. I can see few ways it can be fixed:

  1. Making assetsURL return a URL relative to index.html when base is relative. I'm not sure if it's so simple to get full index.html URL though.
  2. Somehow populate seen object with absolute URLs of styles that vite embeds into index.html as soon as the app loads (before any dynamic import could run)
  3. Replace document.querySelector(`link[href="${dep}"]${cssSelector}`) check with something like document.querySelectorAll('link').some(link => link.href === dep && (!isCss || link.rel === 'stylesheet')). This option takes advantage of link.href property being always an absolute URL with origin (while the attribute can be relative)

bgoscinski avatar Sep 02 '22 14:09 bgoscinski

I think something along the lines of 3 is the best, as even with two, people may add a link to a CSS by hand that we will miss with 2. For the moment, you could use dep before being modified here https://github.com/vitejs/vite/blob/4158b98c8fc30070967de1176f4b1c252d275386/packages/vite/src/node/plugins/importAnalysisBuild.ts#L71 But after https://github.com/vitejs/vite/pull/9938, this function may already get a full URL. Maybe we could find a way to compare full URL with relative ones. It isn't clear at this point what is best (more after #9938)

patak-dev avatar Sep 02 '22 15:09 patak-dev

Yeah, I think it's safest to always (with both relative and absolute base option) normalize dep to full absolute URL (like https://some.origin/foo/bar/assets/index.6744de28.css) and then compare it to each link.href property which is also a full absolute URL.

On it.

bgoscinski avatar Sep 02 '22 15:09 bgoscinski

Hi! I was noticing this on my work on PR #9920, and I did fixed it there also, by this code in the preload function: https://github.com/vitejs/vite/blob/8f68cdfb6650ca22c8128171b37036a595c414b5/packages/vite/src/node/plugins/importAnalysisBuild.ts#L70-L92

What do you think?

Tal500 avatar Sep 06 '22 10:09 Tal500