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

Dynamic CSS import fails due to double slashes #4731

Closed
7 tasks done
vwkd opened this issue Aug 25, 2021 · 7 comments
Closed
7 tasks done

Dynamic CSS import fails due to double slashes #4731

vwkd opened this issue Aug 25, 2021 · 7 comments

Comments

@vwkd
Copy link
Contributor

vwkd commented Aug 25, 2021

Describe the bug

When building for production, a dynamic CSS import with relative path fails to load because it starts with double slashes.

This issue occurs only when building for production (npm run build), and not when running the development server (npm run dev).

For example, in a script in the root folder, a relative import like this

await import("./style.css");

Results in markup with a path starting with an incorrect double slash like this.

<link rel="stylesheet" href="//assets/style.9765b413.css">

I think I tracked it down to line L48 in importAnalysisBuild.ts introduced by #4096

dep = `${base}${dep}`

If base ends with a slash and dep starts with a slash, string concatenation gives two slashes. With Node, one would use path.join(base, dep) for that reason. Not sure if something like that exists for the browser.

Reproduction

See https://github.com/vwkd/vite-duplicate-dynamic-import

  1. Run npm run build && npm run preview
  2. Observe no text color change (import failed)

You can see the second erroneous import failing in the network tab and a console error being logged

network

Note, this issue occurs only when building for production, not when running the dev server.

  1. Run npm run dev
  2. Observe text color green (import successful)

System Info

System:
    OS: macOS 11.5
  Binaries:
    Node: 16.6.0 - ~/.nvm/versions/node/v16.6.0/bin/node
    npm: 7.19.1 - ~/.nvm/versions/node/v16.6.0/bin/npm
  Browsers:
    Chrome: 92.0.4515.131
    Firefox: 90.0.2
    Safari: 14.1.2
  npmPackages:
    @sveltejs/adapter-static: ^1.0.0-next.11 => 1.0.0-next.16 
    @sveltejs/kit: next => 1.0.0-next.144 
    svelte: ^3.34.0 => 3.42.1

Used Package Manager

yarn

Logs

No response

Validations

@rynpsc
Copy link

rynpsc commented Aug 25, 2021

I'm also seeing this issue with dynamic css. In my case I have base set to /dist/ which is getting resolved to dist//dist/assets/main.6159197d.css. By default base is / so I wondering if this issue is a bit more complex than a double slash and instead the base is being duplicated?

await import('./styles/main.scss');

Looking at what Vite is outputting the base path is being added to the call to __vitePreload

await __vitePreload(() => Promise.resolve({}), true ? ["/dist/assets/main.6159197d.css"] : void 0);

and the within the __vitePreload base is getting prefixed again.

const base = "/dist/";
const __vitePreload = function preload(baseModule, deps) {
  if (!deps || deps.length === 0) {
    return baseModule();
  }
  return Promise.all(deps.map((dep) => {
    dep = `${base}${dep}`;

@vwkd
Copy link
Contributor Author

vwkd commented Aug 25, 2021

@rynpsc Good catch! Removing the trailing slash for / is actually making it an empty string so my fix was only accidentally working in my case.

I think the second prefix ${base}${dep} was introduced in #4096 . I wonder what the reason was. Maybe @hex-ci can help explain.

EDIT: A quick test showed simply removing ${base}${dep} like it was before that PR solves the issue, both for the default base / and for base /dist/.

@hex-ci
Copy link
Contributor

hex-ci commented Aug 26, 2021

I just tested it with the project you provided. I introduced my patch in Vite 2.4.4. Then I tried 2.4.3. There is no difference between the code built in 2.4.3 and the code built in 2.4.4. I checked 2.5.0 and 2.5.1. In these two versions, changes were made to the dynamic import code. I suspect that these changes caused your problem.

And I found that there was a BUG for dynamically importing css in versions prior to 2.5.1, which was fixed in 2.5.1. It may be a problem caused by this aspect, and relevant developers need to check it again.

@vwkd

@hex-ci
Copy link
Contributor

hex-ci commented Aug 26, 2021

I found the problem in the latest code:

deps.add(config.base + file)

The config.base in the code deps.add(config.base + file) is redundant and should be removed.

@hex-ci
Copy link
Contributor

hex-ci commented Aug 26, 2021

This PR caused this BUG: #3333

And I tried to fix it: #4740

@hex-ci
Copy link
Contributor

hex-ci commented Sep 13, 2021

Vite 2.5.7 version has been released, you can try to see if this version solves your problem.

@vwkd
Copy link
Contributor Author

vwkd commented Sep 13, 2021

It does! Thank you for the fix @hex-ci. 🙏

@vwkd vwkd closed this as completed Sep 13, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Sep 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants