CSS files cannot be treeshaken with side effects

BuptStEve commented 3 years ago

Describe the bug

vue-cli(webpack) VS vite

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

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to https://github.com/vuejs/vue-next instead.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.

bluwy commented 2 years ago

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 commented 2 years ago

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.

bluwy commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 1 year ago

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 commented 1 year ago

@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.

viridia commented 1 year ago

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.)

laygir commented 1 year ago

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 commented 1 year ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

Yeah it seems like the respective issue on Webpack side is https://github.com/webpack-contrib/css-loader/issues/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 commented 6 months ago

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 https://github.com/webpack-contrib/css-loader/issues/769#issuecomment-427634591)

bluwy commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 https://github.com/vitejs/vite/issues/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, https://github.com/vitejs/vite/issues/8237 should be followed instead.

sapphi-red commented 6 months ago

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 commented 6 months ago

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 https://github.com/vitejs/vite/issues/8237 perhaps.

vitejs / vite