esbuild icon indicating copy to clipboard operation
esbuild copied to clipboard

CSS is wrongly present in bundle when the CSS import is done in an unused nested JS import

Open KKoukiou opened this issue 1 year ago • 6 comments

test.jsx is my entry point. It imports just Pink JSX file, but it does this through an named import from a high level 'all' file, which includes more exported functions.

In the resulting bundled dist/index.css I am expected to see the CSS that the 'Pink' JSX file imports, aka pink.css but not the CSS from the other exported but unused modules.


[kkoukiou@sequioa test-esbuild-css-treeshaking]$ tail -n +1 test.jsx all/*
==> test.jsx <==
import * as React from 'react'
import * as Server from 'react-dom/server'
import { Pink } from './all'

console.log(Server.renderToString(<Pink />))


==> all/blue.css <==
.mycolor {
    color: blue;
}

==> all/Blue.jsx <==
import * as React from 'react';

import "./blue.css";

export const Blue = () => {
    return "<div class='mycolor'>test</div>";
};

==> all/index.js <==
export * from './Pink';
export * from './Blue';

==> all/pink.css <==
.mycolor {
  color: pink;
}

==> all/Pink.jsx <==
import * as React from 'react';

import "./pink.css";

export const Pink = () => {
    return "<div class='mycolor'>test</div>";
};

However the resulting dist/test.css file contains the CSS which I am expecting to be tree-shaked.

[kkoukiou@sequioa test-esbuild-css-treeshaking]$ cat dist/test.css 
/* pink.css */
.mycolor {
  color: pink;
}

/* blue.css */
.mycolor {
  color: blue;
}

The esbuild is used here like this: ./node_modules/.bin/esbuild test.jsx --bundle --outdir=dist

And lastly the package.json for the complete reproducer:

[kkoukiou@sequioa test-esbuild-css-treeshaking]$ cat package.json 
{
  "devDependencies": {
    "esbuild": "^0.17.8",
    "react": "^18.2.0",
    "react-dom": "^18.2.0"
  }
}

Note: when importing with 'import { Pink } from './all/Pink' it works as expected.

KKoukiou avatar Feb 17 '23 10:02 KKoukiou

Ok, I see that this is a change that was introduced in 17.7 with relevant changelog entry being:

This release ignores the `sideEffects` annotation in `package.json` for CSS files that are imported into JS files using esbuild's `css` loader. This means that these CSS files are no longer be tree-shaken.

I believe this is not the correct solution. CSS imported from within JS files that are being imported should be kept. Otherwise they should be dropped as dead-code together with their parent JS file which was not imported.

Users which want all of the CSS to be with side effects should be able to do that through the package.json sideEffects property like: "sideEffects": ["*.css"]

But this should not be the default.

KKoukiou avatar Feb 17 '23 10:02 KKoukiou

==> main.js <==
function fn(){
  import('./app.js')
}

==> app.js <==
import './app.css'

==> app.css <==
.myApp {
    color: blue;
}

out app.css in main

app.js is asynchronous, app.css should not be in main

Li13 avatar Mar 06 '23 05:03 Li13

I've run into this issue when upgrading esbuild in both Vanilla Extract and Remix. The original behaviour before v0.17.7 is pretty important for locally scoped styles in a component system (whether via manual BEM-like namespacing, or via tools like CSS Modules and Vanilla Extract), otherwise we end up including a lot of unused styles in our bundles.

markdalgleish avatar Mar 28 '23 04:03 markdalgleish

Could CSS imports default to sideEffects: true but if { sideEffects: false } specified via plugin return value then that overrides it?

JS files could be marked as sideEffect: false but contain side-effects anyway, and that's ok. Useful in practice to override bundler in those cases. Same argument holds for CSS, no?

pcattori avatar May 24 '23 23:05 pcattori

From a plugin authoring perspective, @pcattori's suggestion makes sense to me. If I'm writing a plugin that hashes CSS class names (e.g. CSS Modules, Vanilla Extract) I'd like to be able to explicitly set sideEffects: false for the CSS managed by the plugin so that unused CSS can be tree-shaken.

markdalgleish avatar Sep 06 '23 04:09 markdalgleish

Has anyone come up with any solutions that aren't "pin to 0.17.6 till this is fixed" or "import CSS as text and handle it at runtime"? Getting rid of wildcard re-exports doesn't help: any re-export is enough to trigger the issue.

pjeby avatar Nov 04 '23 19:11 pjeby