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

fix(css): referencing aliased svg asset with lightningcss enabled errored #18819

Merged

Conversation

sapphi-red
Copy link
Member

Description

fixes #18806

@sapphi-red sapphi-red added feat: css p2-edge-case Bug, but has workaround or limited in scope (priority) regression The issue only appears after a new release labels Nov 28, 2024
@@ -3177,7 +3177,6 @@ async function compileLightningCSS(
css = css.replace(dep.placeholder, () => dep.url)
break
}
deps.add(dep.url)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line added non-resolved URLs to deps. But deps should only include file paths.

That URL was passed to fileToDevUrl here.

await fileToDevUrl(
this.environment,
file,
/* skipBase */ true,
),

Then, this fs.readFile was called for svg files.
const content = await fsp.readFile(file)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the postcss transformer, we didn't add url() referenced assets to deps.

postcssPlugins.push(
UrlRewritePostcssPlugin({
replacer: urlReplacer,
logger: environment.logger,
}),

So for now, I think we can simply remove this deps.add. In future, maybe we should add url() referenced files to deps as well.

@patak-dev patak-dev merged commit ae68958 into vitejs:main Nov 28, 2024
18 checks passed
@sapphi-red sapphi-red deleted the fix/lightning-css-svg-asset-with-alias branch November 29, 2024 01:51
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Dec 3, 2024
| datasource | package | from  | to    |
| ---------- | ------- | ----- | ----- |
| npm        | vite    | 6.0.1 | 6.0.2 |


## [v6.0.2](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#small602-2024-12-02-small)

-   chore: run typecheck in unit tests ([#18858](vitejs/vite#18858)) ([49f20bb](vitejs/vite@49f20bb)), closes [#18858](vitejs/vite#18858)
-   chore: update broken links in changelog ([#18802](vitejs/vite#18802)) ([cb754f8](vitejs/vite@cb754f8)), closes [#18802](vitejs/vite#18802)
-   chore: update broken links in changelog ([#18804](vitejs/vite#18804)) ([47ec49f](vitejs/vite@47ec49f)), closes [#18804](vitejs/vite#18804)
-   fix: don't store temporary vite config file in `node_modules` if deno ([#18823](vitejs/vite#18823)) ([a20267b](vitejs/vite@a20267b)), closes [#18823](vitejs/vite#18823)
-   fix(css): referencing aliased svg asset with lightningcss enabled errored ([#18819](vitejs/vite#18819)) ([ae68958](vitejs/vite@ae68958)), closes [#18819](vitejs/vite#18819)
-   fix(manifest): use `style.css` as a key for the style file for `cssCodesplit: false` ([#18820](vitejs/vite#18820)) ([ec51115](vitejs/vite@ec51115)), closes [#18820](vitejs/vite#18820)
-   fix(optimizer): resolve all promises when cancelled ([#18826](vitejs/vite#18826)) ([d6e6194](vitejs/vite@d6e6194)), closes [#18826](vitejs/vite#18826)
-   fix(resolve): don't set builtinModules to `external` by default ([#18821](vitejs/vite#18821)) ([2250ffa](vitejs/vite@2250ffa)), closes [#18821](vitejs/vite#18821)
-   fix(ssr): set `ssr.target: 'webworker'` defaults as fallback ([#18827](vitejs/vite#18827)) ([b39e696](vitejs/vite@b39e696)), closes [#18827](vitejs/vite#18827)
-   feat(css): format lightningcss error ([#18818](vitejs/vite#18818)) ([dac7992](vitejs/vite@dac7992)), closes [#18818](vitejs/vite#18818)
-   refactor: make properties of ResolvedServerOptions and ResolvedPreviewOptions required ([#18796](vitejs/vite#18796)) ([51a5569](vitejs/vite@51a5569)), closes [#18796](vitejs/vite#18796)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: css p2-edge-case Bug, but has workaround or limited in scope (priority) regression The issue only appears after a new release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vite V6 can't resolve alias if css transformer is lightningcss
2 participants