Skip to content

Commit

Permalink
fix #1370, fix #1458, fix #2905: css tree-shaking
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Feb 9, 2023
1 parent bfe3ead commit 88e17d8
Show file tree
Hide file tree
Showing 4 changed files with 359 additions and 9 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

## Unreleased

* Ignore `sideEffects: false` for imported CSS files ([#1370](https://github.com/evanw/esbuild/issues/1370), [#1458](https://github.com/evanw/esbuild/pull/1458), [#2905](https://github.com/evanw/esbuild/issues/2905))

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.

Importing CSS into JS causes esbuild to automatically create a CSS entry point next to the JS entry point containing the bundled CSS. Previously packages that specified some form of `"sideEffects": false` could potentially cause esbuild to consider one or more of the JS files on the import path to the CSS file to be side-effect free, which would result in esbuild removing that CSS file from the bundle. This was problematic because the removal of that CSS is outwardly observable, since all CSS is global, so it was incorrect for previous versions of esbuild to tree-shake CSS files imported into JS files.

* Add constant folding for certain additional equality cases ([#2394](https://github.com/evanw/esbuild/issues/2394), [#2895](https://github.com/evanw/esbuild/issues/2895))

This release adds constant folding for expressions similar to the following:
Expand Down
211 changes: 211 additions & 0 deletions internal/bundler_tests/bundler_dce_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3444,3 +3444,214 @@ func TestPackageJsonSideEffectsFalseCrossPlatformSlash(t *testing.T) {
},
})
}

func TestTreeShakingJSWithAssociatedCSS(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/project/test.jsx": `
import { Button } from 'pkg/button'
import { Menu } from 'pkg/menu'
render(<Button/>)
`,
"/project/node_modules/pkg/button.js": `
import './button.css'
export let Button
`,
"/project/node_modules/pkg/button.css": `
button { color: red }
`,
"/project/node_modules/pkg/menu.js": `
import './menu.css'
export let Menu
`,
"/project/node_modules/pkg/menu.css": `
menu { color: red }
`,
},
entryPaths: []string{"/project/test.jsx"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
},
})
}

func TestTreeShakingJSWithAssociatedCSSReExportSideEffectsFalse(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/project/test.jsx": `
import { Button } from 'pkg'
render(<Button/>)
`,
"/project/node_modules/pkg/entry.js": `
export { Button } from './components'
`,
"/project/node_modules/pkg/package.json": `{
"main": "./entry.js",
"sideEffects": false
}`,
"/project/node_modules/pkg/components.jsx": `
require('./button.css')
export const Button = () => <button/>
`,
"/project/node_modules/pkg/button.css": `
button { color: red }
`,
},
entryPaths: []string{"/project/test.jsx"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
},
})
}

func TestTreeShakingJSWithAssociatedCSSReExportSideEffectsFalseOnlyJS(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/project/test.jsx": `
import { Button } from 'pkg'
render(<Button/>)
`,
"/project/node_modules/pkg/entry.js": `
export { Button } from './components'
`,
"/project/node_modules/pkg/package.json": `{
"main": "./entry.js",
"sideEffects": ["*.css"]
}`,
"/project/node_modules/pkg/components.jsx": `
require('./button.css')
export const Button = () => <button/>
`,
"/project/node_modules/pkg/button.css": `
button { color: red }
`,
},
entryPaths: []string{"/project/test.jsx"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
},
})
}

func TestTreeShakingJSWithAssociatedCSSExportStarSideEffectsFalse(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/project/test.jsx": `
import { Button } from 'pkg'
render(<Button/>)
`,
"/project/node_modules/pkg/entry.js": `
export * from './components'
`,
"/project/node_modules/pkg/package.json": `{
"main": "./entry.js",
"sideEffects": false
}`,
"/project/node_modules/pkg/components.jsx": `
require('./button.css')
export const Button = () => <button/>
`,
"/project/node_modules/pkg/button.css": `
button { color: red }
`,
},
entryPaths: []string{"/project/test.jsx"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
},
})
}

func TestTreeShakingJSWithAssociatedCSSExportStarSideEffectsFalseOnlyJS(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/project/test.jsx": `
import { Button } from 'pkg'
render(<Button/>)
`,
"/project/node_modules/pkg/entry.js": `
export * from './components'
`,
"/project/node_modules/pkg/package.json": `{
"main": "./entry.js",
"sideEffects": ["*.css"]
}`,
"/project/node_modules/pkg/components.jsx": `
require('./button.css')
export const Button = () => <button/>
`,
"/project/node_modules/pkg/button.css": `
button { color: red }
`,
},
entryPaths: []string{"/project/test.jsx"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
},
})
}

func TestTreeShakingJSWithAssociatedCSSUnusedNestedImportSideEffectsFalse(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/project/test.jsx": `
import { Button } from 'pkg/button'
render(<Button/>)
`,
"/project/node_modules/pkg/package.json": `{
"sideEffects": false
}`,
"/project/node_modules/pkg/button.jsx": `
import styles from './styles'
export const Button = () => <button/>
`,
"/project/node_modules/pkg/styles.js": `
import './styles.css'
export default {}
`,
"/project/node_modules/pkg/styles.css": `
button { color: red }
`,
},
entryPaths: []string{"/project/test.jsx"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
},
})
}

func TestTreeShakingJSWithAssociatedCSSUnusedNestedImportSideEffectsFalseOnlyJS(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/project/test.jsx": `
import { Button } from 'pkg/button'
render(<Button/>)
`,
"/project/node_modules/pkg/package.json": `{
"sideEffects": ["*.css"]
}`,
"/project/node_modules/pkg/button.jsx": `
import styles from './styles'
export const Button = () => <button/>
`,
"/project/node_modules/pkg/styles.js": `
import './styles.css'
export default {}
`,
"/project/node_modules/pkg/styles.css": `
button { color: red }
`,
},
entryPaths: []string{"/project/test.jsx"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
},
})
}
142 changes: 142 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_dce.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1786,6 +1786,148 @@ var init_cjs = __esm({
init_lib();
console.log(keep1(), (init_cjs(), __toCommonJS(cjs_exports)));

================================================================================
TestTreeShakingJSWithAssociatedCSS
---------- /out/test.js ----------
// project/node_modules/pkg/button.js
var Button;

// project/test.jsx
render(/* @__PURE__ */ React.createElement(Button, null));

---------- /out/test.css ----------
/* project/node_modules/pkg/button.css */
button {
color: red;
}

/* project/node_modules/pkg/menu.css */
menu {
color: red;
}

================================================================================
TestTreeShakingJSWithAssociatedCSSExportStarSideEffectsFalse
---------- /out/test.js ----------
// project/node_modules/pkg/button.css
var require_ = __commonJS({
"project/node_modules/pkg/button.css"(exports, module) {
module.exports = {};
}
});

// project/node_modules/pkg/components.jsx
require_();
var Button = () => /* @__PURE__ */ React.createElement("button", null);

// project/test.jsx
render(/* @__PURE__ */ React.createElement(Button, null));

---------- /out/test.css ----------
/* project/node_modules/pkg/button.css */
button {
color: red;
}

================================================================================
TestTreeShakingJSWithAssociatedCSSExportStarSideEffectsFalseOnlyJS
---------- /out/test.js ----------
// project/node_modules/pkg/button.css
var require_ = __commonJS({
"project/node_modules/pkg/button.css"(exports, module) {
module.exports = {};
}
});

// project/node_modules/pkg/components.jsx
require_();
var Button = () => /* @__PURE__ */ React.createElement("button", null);

// project/test.jsx
render(/* @__PURE__ */ React.createElement(Button, null));

---------- /out/test.css ----------
/* project/node_modules/pkg/button.css */
button {
color: red;
}

================================================================================
TestTreeShakingJSWithAssociatedCSSReExportSideEffectsFalse
---------- /out/test.js ----------
// project/node_modules/pkg/button.css
var require_ = __commonJS({
"project/node_modules/pkg/button.css"(exports, module) {
module.exports = {};
}
});

// project/node_modules/pkg/components.jsx
require_();
var Button = () => /* @__PURE__ */ React.createElement("button", null);

// project/test.jsx
render(/* @__PURE__ */ React.createElement(Button, null));

---------- /out/test.css ----------
/* project/node_modules/pkg/button.css */
button {
color: red;
}

================================================================================
TestTreeShakingJSWithAssociatedCSSReExportSideEffectsFalseOnlyJS
---------- /out/test.js ----------
// project/node_modules/pkg/button.css
var require_ = __commonJS({
"project/node_modules/pkg/button.css"(exports, module) {
module.exports = {};
}
});

// project/node_modules/pkg/components.jsx
require_();
var Button = () => /* @__PURE__ */ React.createElement("button", null);

// project/test.jsx
render(/* @__PURE__ */ React.createElement(Button, null));

---------- /out/test.css ----------
/* project/node_modules/pkg/button.css */
button {
color: red;
}

================================================================================
TestTreeShakingJSWithAssociatedCSSUnusedNestedImportSideEffectsFalse
---------- /out/test.js ----------
// project/node_modules/pkg/button.jsx
var Button = () => /* @__PURE__ */ React.createElement("button", null);

// project/test.jsx
render(/* @__PURE__ */ React.createElement(Button, null));

---------- /out/test.css ----------
/* project/node_modules/pkg/styles.css */
button {
color: red;
}

================================================================================
TestTreeShakingJSWithAssociatedCSSUnusedNestedImportSideEffectsFalseOnlyJS
---------- /out/test.js ----------
// project/node_modules/pkg/button.jsx
var Button = () => /* @__PURE__ */ React.createElement("button", null);

// project/test.jsx
render(/* @__PURE__ */ React.createElement(Button, null));

---------- /out/test.css ----------
/* project/node_modules/pkg/styles.css */
button {
color: red;
}

================================================================================
TestTreeShakingLoweredClassStaticField
---------- /out/entry.js ----------
Expand Down
9 changes: 0 additions & 9 deletions internal/linker/linker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2918,15 +2918,6 @@ func (c *linkerContext) findImportedCSSFilesInJSOrder(entryPoint uint32) (order

// Iterate over each part in the file in order
for _, part := range repr.AST.Parts {
// Ignore dead code that has been removed from the bundle. Any code
// that's reachable from the entry point, even through lazy dynamic
// imports, could end up being activated by the bundle and needs its
// CSS to be included. This may change if/when code splitting is
// supported for CSS.
if !part.IsLive {
continue
}

// Traverse any files imported by this part. Note that CommonJS calls
// to "require()" count as imports too, sort of as if the part has an
// ESM "import" statement in it. This may seem weird because ESM imports
Expand Down

0 comments on commit 88e17d8

Please sign in to comment.