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

CSS files cannot be treeshaken with side effects #4389

Open
6 tasks done
BuptStEve opened this issue Jul 26, 2021 · 19 comments · May be fixed by #16058
Open
6 tasks done

CSS files cannot be treeshaken with side effects #4389

BuptStEve opened this issue Jul 26, 2021 · 19 comments · May be fixed by #16058
Labels
p2-to-be-discussed Enhancement under consideration (priority)

Comments

@BuptStEve
Copy link

BuptStEve commented Jul 26, 2021

Describe the bug

image

vue-cli(webpack) VS vite

image

Reproduction

https://github.com/BuptStEve/vite-css-treeshake

  • npm start to run vue-cli

  • npm run vite to run vite

  • npm run build to run vue-cli

  • npm run vite:build to run vite

System Info

System:
    OS: macOS 11.4
    CPU: (8) x64 Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz
    Memory: 2.44 GB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.17.1 - ~/.nvm/versions/node/v14.17.1/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.14.13 - ~/.nvm/versions/node/v14.17.1/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  Browsers:
    Chrome: 92.0.4515.107
    Safari: 14.1.1
  npmPackages:
    vite: ^2.4.3 => 2.4.3

Used Package Manager

yarn

Logs

No response

Validations

@bluwy
Copy link
Member

bluwy commented Mar 13, 2022

I'm confused, if the package.json marked CSS files as side-effectful, shouldn't Webpack include the CSS that would result in what Vite produces? Vite currently treats all CSS as side-effectful, and side-effect modules aren't treeshaken out because they aren't pure. So from my point of view, Vite is doing the right thing here. Can you shed some light on the issue here?

@BuptStEve
Copy link
Author

BuptStEve commented Mar 15, 2022

I'm confused, if the package.json marked CSS files as side-effectful, shouldn't Webpack include the CSS that would result in what Vite produces? Vite currently treats all CSS as side-effectful, and side-effect modules aren't treeshaken out because they aren't pure. So from my point of view, Vite is doing the right thing here. Can you shed some light on the issue here?

But I just import the component A, so the codes of B(including the CSS) should be treeshaken out.

image

@bluwy
Copy link
Member

bluwy commented Mar 15, 2022

Ah okay. Found the webpack documentation for it, it's weird that the project's package.json's sideEffects determine which module is side-effect-free (or pure), the inverse of its implication. But that may be me not understanding the docs well yet.

Vite currently doesn't support sideEffects in the project's package.json AFAIK, so this makes it a feature request.

But I just import the component A, so the codes of B(including the CSS) should be treeshaken out.

Vite treats all CSS files as side-effectful by default, that means even if we didn't used B, we still imported B, and B contains an import to a CSS file, that CSS file "effects" the state of the application (which makes sense because you add global styles), so that CSS is included in the final bundle, or else your application state could be inconsistent.

If you remove the sideEffects in package.json, you can see that's webpack's default behaviour too.

@BuptStEve
Copy link
Author

Yeah, this feature is useful for UI library to auto import component's own style by default.
And when you build app, it also helps treeshake unused ones(with style).

@sapphi-red
Copy link
Member

@bluwy I think your first understanding about sideEffects field is correct.
According to the webpack docs here, it shows "sideEffects": [ "**/*.css" ], means **/*.css has side-effectful. (Not side-effect-free)

When this was changed like this, webpack did not output any css.

  "sideEffects": [
-    "*.css",
    "src/components/a/index.js",
    "src/components/b/index.js"
  ],

So in conclusion I think when there is sideEffects field, Vite should respect the field for css files (and others).
For example, if sideEffects field exists and does not include something like *.css, vite should stop treating css files as side-effect-ful.

@mayank99
Copy link
Contributor

mayank99 commented Sep 29, 2022

Vite treats all CSS files as side-effectful by default, that means even if we didn't used B, we still imported B, and B contains an import to a CSS file, that CSS file "effects" the state of the application (which makes sense because you add global styles), so that CSS is included in the final bundle, or else your application state could be inconsistent.

@bluwy Is there a way to work around this behavior?

Components often use scoped selectors to ensure there are no global side-effects, so it would be nice to tell vite to treat these css imports as side-effect free. I understand that this is a feature request but there must be something we (component library authors) can do or document today to reduce bundle size?

@bluwy
Copy link
Member

bluwy commented Oct 3, 2022

@mayank99 I don't think there's a workaround today, but I agree there should be a way around this, like the sideEffects field discussed above. I'll put this on the team board to confirm if this is the design we want, or if there's an alternate API design.

@bluwy bluwy added the p2-to-be-discussed Enhancement under consideration (priority) label Oct 3, 2022
@bluwy bluwy added this to Team Board Oct 3, 2022
@bluwy bluwy moved this to Discussing in Team Board Oct 3, 2022
@viridia
Copy link

viridia commented Oct 27, 2022

I've needed something like this as well - I'm currently working on a UI component library that is optimized for server-side rendering, and the challenge I'm facing is that the app doesn't need the CSS for every component in the library, but only the components that it is actually using.

I can think of several approaches - one would be a build configuration parameter telling it to treat files with the "module.css" extension differently than other imports.

The other would be a doc-comment such as @localdep or @pure that would be associated with the CSS module import statement, letting Vite know that the CSS import is a local dependency of the exported symbols in that module - meaning that if all of those exported symbols are tree-shaken out, then the CSS can be removed as well. (I say "exported symbols" because tree-shaking, as I understand it, does not eliminate modules, but rather exports. So the removal of the CSS module would be contingent on the module contents, not the module itself.)

@bluwy bluwy mentioned this issue Nov 1, 2022
7 tasks
@laygir
Copy link

laygir commented Feb 7, 2023

I have a Vuetify 3 + Vite project and was hoping to get rid of the unnecessary 20K lines of css from my final bundle.

@PeterEsenwa
Copy link

My team has a component library using different technologies for building components, like Lit and Quasar/Vue. This would be useful to help strip out unused Quasar styles.

@bluwy
Copy link
Member

bluwy commented Feb 27, 2024

I'm currently looking into this as it'll benefit UI frameworks with scoped CSS. Especially for component libraries or barrel files, as today, all the scoped CSS will get bundled even if the related component is not bundled.

However marking CSS are side-effect-free is technically not the solution. For example, if you have import "./global.css" in your entrypoint JS, "side-effect-free" means that the import can be removed completely (and its related CSS asset) because you're not really using anything from the module anyway, which is incorrect.

I'm currently trying to figure an API to say "this CSS import is related to this default/named export. if the export is treeshaken, also treeshake the CSS". I'm not sure if it's possible to do it automatically. Also, this issue affects CSS modules today.

@mayank99
Copy link
Contributor

This is a tricky problem. The ideal way would be something like: "If nothing else from this module is imported, then tree-shake its CSS imports too". But I understand that's not how JS works, especially when using barrel files.

I'm not sure that a new API would be useful for component library authors, because it wouldn't work with other bundlers (esbuild, webpack, etc). It would be nice if there was some consensus across different bundlers, to ease the burden from library authors.

(To get around this problem today, I've resorted to removing all side-effect imports, and provide explicit CSS exports for applications to consume, which is obviously not ideal.)

@bluwy
Copy link
Member

bluwy commented Feb 28, 2024

Yeah it seems like the respective issue on Webpack side is webpack-contrib/css-loader#506

The ideal way would be something like: "If nothing else from this module is imported, then tree-shake its CSS imports too".

This is a little risky I think. If you have a library that export my-lib/styles.js that only imports CSS, and export nothing (or export something but you don't necessarily have to import), then we can't treeshake the CSS away. It's technically still being used.

Also, I think we should look at CSS treeshaking as an optimization standpoint. In dev where no treeshaking is happening, you'd load all the styles from a barrel file, but in prod, suddenly some may be missing which causes inconsistencies.

The main usecase I'm looking into so far are for scoped CSS and CSS modules only, as in dev, the styles shouldn't bleed so extraneously-loaded styles are fine. In prod, we can then optimize and remove them.


So from that, I think we need some sort of opt-in API and likely Vite plugins could automate it. And if it comes to that, it'll be tricky to make it bundler-agnostic.

@sapphi-red
Copy link
Member

Yeah it seems like the respective issue on Webpack side is webpack-contrib/css-loader#506

I think that issue is about a more granular tree shaking at selector level. The OP is writing about the same problem with us but the discussion is mainly focused on a more granular one.

The ideal way would be something like: "If nothing else from this module is imported, then tree-shake its CSS imports too".

I guess Webpack handles like this. I found this example (https://github.com/pp3nd3x/webpack-treeshaking-css-modules) and it looks like doing like that. (I found it at webpack-contrib/css-loader#769 (comment))

@bluwy
Copy link
Member

bluwy commented Feb 28, 2024

Yeah the linked issue seems to diverge to a different treeshaking topic, but that's the best I've found 😅

The repo you linked seems to be about "CSS modules", which I also want to cover. But my main concern of the approach proposed by Mayank was for general global CSS imports, which I don't think is safe to treeshake that way.

@sapphi-red
Copy link
Member

The repo you linked seems to be about "CSS modules"

Ah, I overlooked it and assumed it's the normal global CSS imports 🤦. Webpack does recommend adding *.css to sideEffects field.

@bluwy
Copy link
Member

bluwy commented Feb 29, 2024

I took another look at the repro and understand the issue better now. It isn't related to CSS at all. Webpack has a unique optimization where if you mark a barrel file as side-effect free, then any re-exports will redirect to the re-exported module directly. Specifically optimization.sideEffects and optimization.providedExports are at play here, which is also enabled in dev. For example:

// src/barrel/index.js
export { a } from './a.js'
export { b } from './b.js'
// src/index.js
import { a } from './barrel'
// webpack rewrites this to
// import { a } from './barrel/a.js'
// because the barrel file is marked side effect free

console.log(a)

Vite intentionally doesn't have these optimizations in dev because they have perf penalties. If there's interest for it, I think #8237 is the right issue to follow. As even if we respect sideEffects in user project today, it will only work in build which can cause unintended inconsistencies.

For this issue, I'll focus on CSS treeshaking only and will close this once it's resolved. To get the repro resolved, #8237 should be followed instead.

@bluwy bluwy linked a pull request Feb 29, 2024 that will close this issue
9 tasks
@sapphi-red
Copy link
Member

It seems rollup treats each export statements as side-effect free if the barrel file is marked as side effect free.
https://stackblitz.com/edit/rollup-repro-jgfcu7?file=package.json,rollup.config.js

Also it seems the repro now works: https://stackblitz.com/edit/github-sqz9ee?file=package.json
With Vite 2.x, the CSS file is .a{color:#00f}.a,.b{color:#ff0}.
With Vite 3.x, the CSS file doesn't exist at all.
With Vite 4.x, the CSS file is .a{color:#00f}.
Interesting that the behavior changes 🤔

@bluwy
Copy link
Member

bluwy commented Feb 29, 2024

Oh interesting. I thought the behaviour would be the same so I didn't bother to check that. So I guess on the build side it is already fixed (sideEffects works in user files from the repro), however it doesn't work in dev yet. So now it has the inconsistency. That's an interesting reason to work towards #8237 perhaps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-to-be-discussed Enhancement under consideration (priority)
Projects
Status: Has plan
Development

Successfully merging a pull request may close this issue.

7 participants