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

broken scss url rewrites with scss variables #5337

Closed
7 tasks done
gluck opened this issue Oct 18, 2021 · 7 comments
Closed
7 tasks done

broken scss url rewrites with scss variables #5337

gluck opened this issue Oct 18, 2021 · 7 comments

Comments

@gluck
Copy link
Contributor

gluck commented Oct 18, 2021

Describe the bug

When an imported scss file has variable url($foo) links, they get badly rewritten:

scss-import-test/index.scss:

$default-image-1: "/default.png";
$default-image-2: "./default.png";
$default-image-3: "default.png";

body {
  background-image: url($default-image-1);
  background-image: url($default-image-2);
  background-image: url($default-image-3);
  background-image: url("/default.png");
  background-image: url("./default.png");
  background-image: url("default.png");
}

Gets rewritten as (notice the invalid css):

body {
  background-image: url(node_modules/scss-import-test/"/default.png"); // NOK
  background-image: url(node_modules/scss-import-test/"./default.png"); // NOK
  background-image: url(node_modules/scss-import-test/"default.png"); // NOK
  background-image: url("/default.png");
  background-image: url("/node_modules/scss-import-test/default.png"); // OK
  background-image: url("/node_modules/scss-import-test/default.png"); // OK
}

Note that when imported through another (intermediate) scss file, the output is different:
scss-import-test/nested.scss:

@import './index.scss'

Results into (valid css, but the paths aren't rewritten):

body {
  background-image: url("/default.png");
  background-image: url("./default.png");
  background-image: url("default.png");
  background-image: url("/default.png");
  background-image: url("./default.png");
  background-image: url("default.png");
}

I believe this erratic behavior is caused by rebaseUrls/rewriteCssUrls being executed on (raw SCSS) content from the first import from src (and only the first, further imports will get processed by vite-url-rewrite postcss plugin, on CSS ast, with variable interpolated), can probably submit a PR if agreed.

Reproduction

https://stackblitz.com/edit/vite-dgawmt?file=index.scss

System Info

Stackbliz doesn't run envinfo.
vite@2.6.9

Used Package Manager

npm

Logs

No response

Validations

gluck added a commit to divriots/browser-vite that referenced this issue Oct 19, 2021
@rburgst
Copy link

rburgst commented Jan 1, 2022

any news on that? I have the same problem with bootswatch: https://stackblitz.com/edit/vitejs-vite-kjplvo?file=package.json

@gluck
Copy link
Contributor Author

gluck commented Jan 1, 2022

I didn't get any feedback so I didn't push the PR here. Feel free to pick up the linked change and test if it works for you.

@Sefriol
Copy link

Sefriol commented Apr 6, 2022

Can this be separated into different plugin in the meantime? It seems quite drastic to create a new vite version just to fix this issue.

(I am also having problem with Bootswatch, but noticed the same issue with our own scss files as well)

@gluck
Copy link
Contributor Author

gluck commented Apr 6, 2022

SCSS processing is core vite so I don't believe much can be done in a plugin for that. But the fix is ony a couple of lines divriots@20f34e3 so it shouldn't take too much effort to push it here.

@Sefriol
Copy link

Sefriol commented Apr 6, 2022

Seems kinda weird oversight for preferred building block for so many frameworks. That's why I would love to get an input from vite team to confirm that this is an issue and it's taken into consideration.

Hard to recommend this for any serious development if you need to "hack it" to work some libraries like bootswatch.

@patak-dev
Copy link
Member

@gluck if you or others want to send a PR to fix this with some test cases, we should be able to discuss it. Your examples are clear bugs.

@sapphi-red
Copy link
Member

Closing as it is organized into #7651.

@github-actions github-actions bot locked and limited conversation to collaborators May 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants