From 700ffda8dceaede86104d9d0a6eb8edc6ea82a38 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Thu, 4 Jun 2020 16:06:40 -0700 Subject: [PATCH] implement the "sideEffects" package.json field (#50) --- CHANGELOG.md | 6 + internal/bundler/bundler.go | 65 +-- internal/bundler/bundler_dce_test.go | 593 +++++++++++++++++++++++++++ internal/bundler/bundler_test.go | 2 +- internal/bundler/bundler_ts_test.go | 2 +- internal/bundler/linker.go | 105 +++-- internal/parser/parser.go | 6 + internal/resolver/resolver.go | 78 +++- internal/runtime/runtime.go | 2 +- scripts/end-to-end-tests.js | 45 ++ 10 files changed, 833 insertions(+), 71 deletions(-) create mode 100644 internal/bundler/bundler_dce_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bd13db3e03..79f490a9da5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +* Respect the `sideEffects` field when tree shaking ([#50](https://github.com/evanw/esbuild/issues/50)) + + Tree shaking now respects `"sideEffects": false` in `package.json`, which means esbuild now generates smaller bundles with certain libraries such as [lodash-es](https://www.npmjs.com/package/lodash-es). This setting is a convention from Webpack: https://webpack.js.org/guides/tree-shaking/. Any files in a package with this setting will not be included in the bundle if they are imported using an ES6 import and then never used. + ## 0.4.5 * Fix a crash with more than 8 entry points ([#162](https://github.com/evanw/esbuild/pull/162)) diff --git a/internal/bundler/bundler.go b/internal/bundler/bundler.go index 1ae62f4955b..dd59ed762a2 100644 --- a/internal/bundler/bundler.go +++ b/internal/bundler/bundler.go @@ -39,6 +39,10 @@ type file struct { // that must be written to the output directory. It's used by the "file" // loader. additionalFile *OutputFile + + // If true, this file was listed as not having side effects by a package.json + // file in one of our containing directories with a "sideEffects" field. + ignoreIfUnused bool } func (f *file) resolveImport(path ast.Path) (uint32, bool) { @@ -57,8 +61,9 @@ type Bundle struct { } type parseFlags struct { - isEntryPoint bool - isDisabled bool + isEntryPoint bool + isDisabled bool + ignoreIfUnused bool } type parseArgs struct { @@ -77,10 +82,9 @@ type parseArgs struct { } type parseResult struct { - source logging.Source - ast ast.AST - ok bool - additionalFile *OutputFile + source logging.Source + file file + ok bool } func parseFile(args parseArgs) { @@ -132,37 +136,44 @@ func parseFile(args parseArgs) { loader = args.bundleOptions.LoaderForStdin } - result := parseResult{source: source, ok: true} + result := parseResult{ + source: source, + file: file{ + ignoreIfUnused: args.flags.ignoreIfUnused, + }, + ok: true, + } + switch loader { case LoaderJS: - result.ast, result.ok = parser.Parse(args.log, source, args.parseOptions) + result.file.ast, result.ok = parser.Parse(args.log, source, args.parseOptions) case LoaderJSX: args.parseOptions.JSX.Parse = true - result.ast, result.ok = parser.Parse(args.log, source, args.parseOptions) + result.file.ast, result.ok = parser.Parse(args.log, source, args.parseOptions) case LoaderTS: args.parseOptions.TS.Parse = true - result.ast, result.ok = parser.Parse(args.log, source, args.parseOptions) + result.file.ast, result.ok = parser.Parse(args.log, source, args.parseOptions) case LoaderTSX: args.parseOptions.TS.Parse = true args.parseOptions.JSX.Parse = true - result.ast, result.ok = parser.Parse(args.log, source, args.parseOptions) + result.file.ast, result.ok = parser.Parse(args.log, source, args.parseOptions) case LoaderJSON: var expr ast.Expr expr, result.ok = parser.ParseJSON(args.log, source, parser.ParseJSONOptions{}) - result.ast = parser.ModuleExportsAST(args.log, source, args.parseOptions, expr) + result.file.ast = parser.ModuleExportsAST(args.log, source, args.parseOptions, expr) case LoaderText: expr := ast.Expr{Data: &ast.EString{lexer.StringToUTF16(source.Contents)}} - result.ast = parser.ModuleExportsAST(args.log, source, args.parseOptions, expr) + result.file.ast = parser.ModuleExportsAST(args.log, source, args.parseOptions, expr) case LoaderBase64: encoded := base64.StdEncoding.EncodeToString([]byte(source.Contents)) expr := ast.Expr{Data: &ast.EString{lexer.StringToUTF16(encoded)}} - result.ast = parser.ModuleExportsAST(args.log, source, args.parseOptions, expr) + result.file.ast = parser.ModuleExportsAST(args.log, source, args.parseOptions, expr) case LoaderDataURL: mimeType := mime.TypeByExtension(extension) @@ -172,7 +183,7 @@ func parseFile(args parseArgs) { encoded := base64.StdEncoding.EncodeToString([]byte(source.Contents)) url := "data:" + mimeType + ";base64," + encoded expr := ast.Expr{Data: &ast.EString{lexer.StringToUTF16(url)}} - result.ast = parser.ModuleExportsAST(args.log, source, args.parseOptions, expr) + result.file.ast = parser.ModuleExportsAST(args.log, source, args.parseOptions, expr) case LoaderFile: // Get the file name, making sure to use the "fs" interface so we do the @@ -195,11 +206,11 @@ func parseFile(args parseArgs) { // Export the resulting relative path as a string expr := ast.Expr{ast.Loc{0}, &ast.EString{lexer.StringToUTF16(baseName)}} - result.ast = parser.ModuleExportsAST(args.log, source, args.parseOptions, expr) + result.file.ast = parser.ModuleExportsAST(args.log, source, args.parseOptions, expr) // Copy the file using an additional file payload to make sure we only copy // the file if the module isn't removed due to tree shaking. - result.additionalFile = &OutputFile{ + result.file.additionalFile = &OutputFile{ AbsPath: args.fs.Join(targetFolder, baseName), Contents: bytes, } @@ -246,7 +257,7 @@ func ScanBundle( runtimeParseOptions.IsBundling = true ast, ok := parser.Parse(log, source, runtimeParseOptions) - results <- parseResult{source: source, ast: ast, ok: ok} + results <- parseResult{source: source, file: file{ast: ast}, ok: ok} }() } @@ -294,11 +305,11 @@ func ScanBundle( } source := result.source - resolvedImports := make(map[string]uint32) + result.file.resolvedImports = make(map[string]uint32) // Don't try to resolve paths if we're not bundling if bundleOptions.IsBundling { - for _, part := range result.ast.Parts { + for _, part := range result.file.ast.Parts { for _, importPath := range part.ImportPaths { // Don't try to resolve imports of the special runtime path if importPath.Path.UseSourceIndex && importPath.Path.SourceIndex == ast.RuntimeSourceIndex { @@ -308,12 +319,16 @@ func ScanBundle( sourcePath := source.AbsolutePath pathText := importPath.Path.Text pathRange := source.RangeOfString(importPath.Path.Loc) + resolveResult := res.Resolve(sourcePath, pathText) - switch path, status := res.Resolve(sourcePath, pathText); status { + switch resolveResult.Status { case resolver.ResolveEnabled, resolver.ResolveDisabled: - flags := parseFlags{isDisabled: status == resolver.ResolveDisabled} - sourceIndex := maybeParseFile(path, source, pathRange, flags) - resolvedImports[pathText] = sourceIndex + flags := parseFlags{ + isDisabled: resolveResult.Status == resolver.ResolveDisabled, + ignoreIfUnused: resolveResult.IgnoreIfUnused, + } + sourceIndex := maybeParseFile(resolveResult.AbsolutePath, source, pathRange, flags) + result.file.resolvedImports[pathText] = sourceIndex case resolver.ResolveMissing: log.AddRangeError(source, pathRange, fmt.Sprintf("Could not resolve %q", pathText)) @@ -323,7 +338,7 @@ func ScanBundle( } sources[source.Index] = source - files[source.Index] = file{result.ast, resolvedImports, result.additionalFile} + files[source.Index] = result.file } return Bundle{fs, sources, files, entryPoints} diff --git a/internal/bundler/bundler_dce_test.go b/internal/bundler/bundler_dce_test.go new file mode 100644 index 00000000000..dc81d139fb9 --- /dev/null +++ b/internal/bundler/bundler_dce_test.go @@ -0,0 +1,593 @@ +package bundler + +import ( + "testing" + + "github.com/evanw/esbuild/internal/parser" +) + +func TestPackageJsonSideEffectsFalseKeepNamedImportES6(t *testing.T) { + expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import {foo} from "demo-pkg" + console.log(foo) + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + export const foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg/package.json": ` + { + "sideEffects": false + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + parseOptions: parser.ParseOptions{ + IsBundling: true, + }, + bundleOptions: BundleOptions{ + IsBundling: true, + AbsOutputFile: "/out.js", + }, + expected: map[string]string{ + "/out.js": `// /Users/user/project/node_modules/demo-pkg/index.js +const foo = 123; +console.log("hello"); + +// /Users/user/project/src/entry.js +console.log(foo); +`, + }, + }) +} + +func TestPackageJsonSideEffectsFalseKeepNamedImportCommonJS(t *testing.T) { + expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import {foo} from "demo-pkg" + console.log(foo) + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + exports.foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg/package.json": ` + { + "sideEffects": false + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + parseOptions: parser.ParseOptions{ + IsBundling: true, + }, + bundleOptions: BundleOptions{ + IsBundling: true, + AbsOutputFile: "/out.js", + }, + expected: map[string]string{ + "/out.js": `// /Users/user/project/node_modules/demo-pkg/index.js +var require_index = __commonJS((exports) => { + exports.foo = 123; + console.log("hello"); +}); + +// /Users/user/project/src/entry.js +const demo_pkg = __toModule(require_index()); +console.log(demo_pkg.foo); +`, + }, + }) +} + +func TestPackageJsonSideEffectsFalseKeepStarImportES6(t *testing.T) { + expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import * as ns from "demo-pkg" + console.log(ns) + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + export const foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg/package.json": ` + { + "sideEffects": false + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + parseOptions: parser.ParseOptions{ + IsBundling: true, + }, + bundleOptions: BundleOptions{ + IsBundling: true, + AbsOutputFile: "/out.js", + }, + expected: map[string]string{ + "/out.js": `// /Users/user/project/node_modules/demo-pkg/index.js +const index_exports = {}; +__export(index_exports, { + foo: () => foo +}); +const foo = 123; +console.log("hello"); + +// /Users/user/project/src/entry.js +console.log(index_exports); +`, + }, + }) +} + +func TestPackageJsonSideEffectsFalseKeepStarImportCommonJS(t *testing.T) { + expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import * as ns from "demo-pkg" + console.log(ns) + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + exports.foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg/package.json": ` + { + "sideEffects": false + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + parseOptions: parser.ParseOptions{ + IsBundling: true, + }, + bundleOptions: BundleOptions{ + IsBundling: true, + AbsOutputFile: "/out.js", + }, + expected: map[string]string{ + "/out.js": `// /Users/user/project/node_modules/demo-pkg/index.js +var require_index = __commonJS((exports) => { + exports.foo = 123; + console.log("hello"); +}); + +// /Users/user/project/src/entry.js +const ns = __toModule(require_index()); +console.log(ns); +`, + }, + }) +} + +func TestPackageJsonSideEffectsTrueKeepES6(t *testing.T) { + expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import "demo-pkg" + console.log('unused import') + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + export const foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg/package.json": ` + { + "sideEffects": true + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + parseOptions: parser.ParseOptions{ + IsBundling: true, + }, + bundleOptions: BundleOptions{ + IsBundling: true, + AbsOutputFile: "/out.js", + }, + expected: map[string]string{ + "/out.js": `// /Users/user/project/node_modules/demo-pkg/index.js +console.log("hello"); + +// /Users/user/project/src/entry.js +console.log("unused import"); +`, + }, + }) +} + +func TestPackageJsonSideEffectsTrueKeepCommonJS(t *testing.T) { + expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import "demo-pkg" + console.log('unused import') + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + exports.foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg/package.json": ` + { + "sideEffects": true + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + parseOptions: parser.ParseOptions{ + IsBundling: true, + }, + bundleOptions: BundleOptions{ + IsBundling: true, + AbsOutputFile: "/out.js", + }, + expected: map[string]string{ + "/out.js": `// /Users/user/project/node_modules/demo-pkg/index.js +var require_index = __commonJS((exports) => { + exports.foo = 123; + console.log("hello"); +}); + +// /Users/user/project/src/entry.js +const demo_pkg = __toModule(require_index()); +console.log("unused import"); +`, + }, + }) +} + +func TestPackageJsonSideEffectsFalseKeepBareImportAndRequireES6(t *testing.T) { + expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import "demo-pkg" + require('demo-pkg') + console.log('unused import') + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + export const foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg/package.json": ` + { + "sideEffects": false + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + parseOptions: parser.ParseOptions{ + IsBundling: true, + }, + bundleOptions: BundleOptions{ + IsBundling: true, + AbsOutputFile: "/out.js", + }, + expected: map[string]string{ + "/out.js": `// /Users/user/project/node_modules/demo-pkg/index.js +var require_index = __commonJS((exports) => { + __export(exports, { + foo: () => foo + }); + const foo = 123; + console.log("hello"); +}); + +// /Users/user/project/src/entry.js +require_index(); +console.log("unused import"); +`, + }, + }) +} + +func TestPackageJsonSideEffectsFalseKeepBareImportAndRequireCommonJS(t *testing.T) { + expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import "demo-pkg" + require('demo-pkg') + console.log('unused import') + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + exports.foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg/package.json": ` + { + "sideEffects": false + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + parseOptions: parser.ParseOptions{ + IsBundling: true, + }, + bundleOptions: BundleOptions{ + IsBundling: true, + AbsOutputFile: "/out.js", + }, + expected: map[string]string{ + "/out.js": `// /Users/user/project/node_modules/demo-pkg/index.js +var require_index = __commonJS((exports) => { + exports.foo = 123; + console.log("hello"); +}); + +// /Users/user/project/src/entry.js +require_index(); +console.log("unused import"); +`, + }, + }) +} + +func TestPackageJsonSideEffectsFalseRemoveBareImportES6(t *testing.T) { + expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import "demo-pkg" + console.log('unused import') + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + export const foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg/package.json": ` + { + "sideEffects": false + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + parseOptions: parser.ParseOptions{ + IsBundling: true, + }, + bundleOptions: BundleOptions{ + IsBundling: true, + AbsOutputFile: "/out.js", + }, + expected: map[string]string{ + "/out.js": `// /Users/user/project/src/entry.js +console.log("unused import"); +`, + }, + }) +} + +func TestPackageJsonSideEffectsFalseRemoveBareImportCommonJS(t *testing.T) { + expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import "demo-pkg" + console.log('unused import') + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + exports.foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg/package.json": ` + { + "sideEffects": false + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + parseOptions: parser.ParseOptions{ + IsBundling: true, + }, + bundleOptions: BundleOptions{ + IsBundling: true, + AbsOutputFile: "/out.js", + }, + expected: map[string]string{ + "/out.js": `// /Users/user/project/src/entry.js +console.log("unused import"); +`, + }, + }) +} + +func TestPackageJsonSideEffectsFalseRemoveNamedImportES6(t *testing.T) { + expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import {foo} from "demo-pkg" + console.log('unused import') + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + export const foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg/package.json": ` + { + "sideEffects": false + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + parseOptions: parser.ParseOptions{ + IsBundling: true, + }, + bundleOptions: BundleOptions{ + IsBundling: true, + AbsOutputFile: "/out.js", + }, + expected: map[string]string{ + "/out.js": `// /Users/user/project/src/entry.js +console.log("unused import"); +`, + }, + }) +} + +func TestPackageJsonSideEffectsFalseRemoveNamedImportCommonJS(t *testing.T) { + expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import {foo} from "demo-pkg" + console.log('unused import') + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + exports.foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg/package.json": ` + { + "sideEffects": false + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + parseOptions: parser.ParseOptions{ + IsBundling: true, + }, + bundleOptions: BundleOptions{ + IsBundling: true, + AbsOutputFile: "/out.js", + }, + expected: map[string]string{ + "/out.js": `// /Users/user/project/src/entry.js +console.log("unused import"); +`, + }, + }) +} + +func TestPackageJsonSideEffectsFalseRemoveStarImportES6(t *testing.T) { + expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import * as ns from "demo-pkg" + console.log('unused import') + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + export const foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg/package.json": ` + { + "sideEffects": false + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + parseOptions: parser.ParseOptions{ + IsBundling: true, + }, + bundleOptions: BundleOptions{ + IsBundling: true, + AbsOutputFile: "/out.js", + }, + expected: map[string]string{ + "/out.js": `// /Users/user/project/src/entry.js +console.log("unused import"); +`, + }, + }) +} + +func TestPackageJsonSideEffectsFalseRemoveStarImportCommonJS(t *testing.T) { + expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import * as ns from "demo-pkg" + console.log('unused import') + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + exports.foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg/package.json": ` + { + "sideEffects": false + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + parseOptions: parser.ParseOptions{ + IsBundling: true, + }, + bundleOptions: BundleOptions{ + IsBundling: true, + AbsOutputFile: "/out.js", + }, + expected: map[string]string{ + "/out.js": `// /Users/user/project/src/entry.js +console.log("unused import"); +`, + }, + }) +} + +func TestPackageJsonSideEffectsArrayRemove(t *testing.T) { + expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import {foo} from "demo-pkg" + console.log('unused import') + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + export const foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg/package.json": ` + { + "sideEffects": [] + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + parseOptions: parser.ParseOptions{ + IsBundling: true, + }, + bundleOptions: BundleOptions{ + IsBundling: true, + AbsOutputFile: "/out.js", + }, + expected: map[string]string{ + "/out.js": `// /Users/user/project/src/entry.js +console.log("unused import"); +`, + }, + }) +} + +func TestPackageJsonSideEffectsArrayKeep(t *testing.T) { + expectBundled(t, bundled{ + files: map[string]string{ + "/Users/user/project/src/entry.js": ` + import {foo} from "demo-pkg" + console.log('unused import') + `, + "/Users/user/project/node_modules/demo-pkg/index.js": ` + export const foo = 123 + console.log('hello') + `, + "/Users/user/project/node_modules/demo-pkg/package.json": ` + { + "sideEffects": ["./index.js"] + } + `, + }, + entryPaths: []string{"/Users/user/project/src/entry.js"}, + parseOptions: parser.ParseOptions{ + IsBundling: true, + }, + bundleOptions: BundleOptions{ + IsBundling: true, + AbsOutputFile: "/out.js", + }, + expected: map[string]string{ + "/out.js": `// /Users/user/project/node_modules/demo-pkg/index.js +console.log("hello"); + +// /Users/user/project/src/entry.js +console.log("unused import"); +`, + }, + }) +} diff --git a/internal/bundler/bundler_test.go b/internal/bundler/bundler_test.go index f6de8e458fc..db16e7f79bc 100644 --- a/internal/bundler/bundler_test.go +++ b/internal/bundler/bundler_test.go @@ -3487,7 +3487,7 @@ func TestMinifiedBundleCommonJS(t *testing.T) { AbsOutputFile: "/out.js", }, expected: map[string]string{ - "/out.js": `var d=c(b=>{b.foo=function(){return 123}});var f=c((b,g)=>{g.exports={test:!0}});const{foo:e}=d();console.log(e(),f()); + "/out.js": `var d=c(b=>{b.foo=function(){return 123}});var f=c((b,h)=>{h.exports={test:!0}});const{foo:e}=d();console.log(e(),f()); `, }, }) diff --git a/internal/bundler/bundler_ts_test.go b/internal/bundler/bundler_ts_test.go index e814862dfaf..02185b49c16 100644 --- a/internal/bundler/bundler_ts_test.go +++ b/internal/bundler/bundler_ts_test.go @@ -706,7 +706,7 @@ func TestTSMinifiedBundleCommonJS(t *testing.T) { AbsOutputFile: "/out.js", }, expected: map[string]string{ - "/out.js": `var d=c(b=>{b.foo=function(){return 123}});var f=c((b,g)=>{g.exports={test:!0}});const{foo:e}=d();console.log(e(),f()); + "/out.js": `var d=c(b=>{b.foo=function(){return 123}});var f=c((b,h)=>{h.exports={test:!0}});const{foo:e}=d();console.log(e(),f()); `, }, }) diff --git a/internal/bundler/linker.go b/internal/bundler/linker.go index 934a4ce936c..6499f2f2e7d 100644 --- a/internal/bundler/linker.go +++ b/internal/bundler/linker.go @@ -1018,30 +1018,36 @@ func (c *linkerContext) includeFile(sourceIndex uint32, entryPoint uint, distanc c.accumulateSymbolCount(file.ast.WrapperRef, 1) } - needsToModule := false for partIndex, part := range file.ast.Parts { - // Include all parts in this file with side effects, or just include - // everything if tree-shaking is disabled. Note that we still want to - // perform tree-shaking on the runtime even if tree-shaking is disabled. - if !part.CanBeRemovedIfUnused || (!part.ForceTreeShaking && !c.options.IsBundling && sourceIndex != ast.RuntimeSourceIndex) { - c.includePart(sourceIndex, uint32(partIndex), entryPoint, distanceFromEntryPoint) - } + canBeRemovedIfUnused := part.CanBeRemovedIfUnused // Also include any statement-level imports for _, importPath := range part.ImportPaths { - switch importPath.Kind { - case ast.ImportStmt, ast.ImportDynamic: - if otherSourceIndex, ok := file.resolveImport(importPath.Path); ok { - c.includeFile(otherSourceIndex, entryPoint, distanceFromEntryPoint) - if c.fileMeta[otherSourceIndex].cjsStyleExports { - // This is an ES6 import of a module that's potentially CommonJS - needsToModule = true - } - } else if !c.options.OutputFormat.KeepES6ImportExportSyntax() { - // This is an ES6 import of an external module that may be CommonJS - needsToModule = true + if importPath.Kind != ast.ImportStmt { + continue + } + + if otherSourceIndex, ok := file.resolveImport(importPath.Path); ok { + // Don't include this module for its side effects if it can be + // considered to have no side effects + if c.files[otherSourceIndex].ignoreIfUnused { + continue } + + // Otherwise, include this module for its side effects + c.includeFile(otherSourceIndex, entryPoint, distanceFromEntryPoint) } + + // If we get here then the import was included for its side effects, so + // we must also keep this part + canBeRemovedIfUnused = false + } + + // Include all parts in this file with side effects, or just include + // everything if tree-shaking is disabled. Note that we still want to + // perform tree-shaking on the runtime even if tree-shaking is disabled. + if !canBeRemovedIfUnused || (!part.ForceTreeShaking && !c.options.IsBundling && sourceIndex != ast.RuntimeSourceIndex) { + c.includePart(sourceIndex, uint32(partIndex), entryPoint, distanceFromEntryPoint) } } @@ -1051,12 +1057,6 @@ func (c *linkerContext) includeFile(sourceIndex uint32, entryPoint uint, distanc c.includePartsForRuntimeSymbol("__commonJS", entryPoint, distanceFromEntryPoint) } - // If there's an ES6 import of a non-ES6 module, then we're going to need the - // "__toModule" symbol from the runtime - if needsToModule { - c.includePartsForRuntimeSymbol("__toModule", entryPoint, distanceFromEntryPoint) - } - // If this is an entry point, include all exports if fileMeta.entryPointStatus != entryPointNone { for _, alias := range fileMeta.sortedAndFilteredExportAliases { @@ -1096,6 +1096,9 @@ func (c *linkerContext) includePart(sourceIndex uint32, partIndex uint32, entryP } partMeta.entryBits.setBit(entryPoint) + // Include the file containing this part + c.includeFile(sourceIndex, entryPoint, distanceFromEntryPoint) + // Accumulate symbol usage counts file := &c.files[sourceIndex] part := &file.ast.Parts[partIndex] @@ -1108,11 +1111,30 @@ func (c *linkerContext) includePart(sourceIndex uint32, partIndex uint32, entryP } // Also include any require() imports + needsToModule := false for _, importPath := range part.ImportPaths { - if importPath.Kind == ast.ImportRequire { - if otherSourceIndex, ok := file.resolveImport(importPath.Path); ok { - c.accumulateSymbolCount(c.files[otherSourceIndex].ast.WrapperRef, 1) - c.includeFile(otherSourceIndex, entryPoint, distanceFromEntryPoint) + if otherSourceIndex, ok := file.resolveImport(importPath.Path); ok { + if importPath.Kind == ast.ImportStmt && !c.fileMeta[otherSourceIndex].cjsStyleExports { + // Skip this since it's not a require() import + continue + } + + // This is a require() import + c.accumulateSymbolCount(c.files[otherSourceIndex].ast.WrapperRef, 1) + c.includeFile(otherSourceIndex, entryPoint, distanceFromEntryPoint) + + // This is an ES6 import of a CommonJS module, so it needs the + // "__toModule" wrapper + if importPath.Kind == ast.ImportStmt || importPath.Kind == ast.ImportDynamic { + needsToModule = true + } + } else { + // This is an external import + if (importPath.Kind == ast.ImportStmt || importPath.Kind == ast.ImportDynamic) && + !c.options.OutputFormat.KeepES6ImportExportSyntax() { + // This is an ES6 import of an external module that may be CommonJS, + // so it needs the "__toModule" wrapper + needsToModule = true } } } @@ -1126,6 +1148,12 @@ func (c *linkerContext) includePart(sourceIndex uint32, partIndex uint32, entryP for _, nonLocalDependency := range partMeta.nonLocalDependencies { c.includePart(nonLocalDependency.sourceIndex, nonLocalDependency.partIndex, entryPoint, distanceFromEntryPoint) } + + // If there's an ES6 import of a non-ES6 module, then we're going to need the + // "__toModule" symbol from the runtime + if needsToModule { + c.includePartsForRuntimeSymbol("__toModule", entryPoint, distanceFromEntryPoint) + } } func (c *linkerContext) computeChunks() []chunkMeta { @@ -1249,12 +1277,21 @@ func (c *linkerContext) chunkFileOrder(chunk chunkMeta) []uint32 { if visited[sourceIndex] { return } + visited[sourceIndex] = true file := &c.files[sourceIndex] fileMeta := &c.fileMeta[sourceIndex] + isFileInThisChunk := chunk.entryBits.equals(fileMeta.entryBits) + for partIndex, part := range file.ast.Parts { + isPartInThisChunk := chunk.entryBits.equals(fileMeta.partMeta[partIndex].entryBits) + if isPartInThisChunk { + isFileInThisChunk = true + } + + // Also traverse any files imported by this part for _, importPath := range part.ImportPaths { - if importPath.Kind == ast.ImportStmt || chunk.entryBits.equals(fileMeta.partMeta[partIndex].entryBits) { + if importPath.Kind == ast.ImportStmt || isPartInThisChunk { if otherSourceIndex, ok := file.resolveImport(importPath.Path); ok { visit(otherSourceIndex) } @@ -1268,10 +1305,12 @@ func (c *linkerContext) chunkFileOrder(chunk chunkMeta) []uint32 { // could end up with one being used before being declared. These wrappers // don't have side effects so an easy fix is to just declare them all // before starting to evaluate them. - if sourceIndex == ast.RuntimeSourceIndex || fileMeta.cjsWrap { - prefixOrder = append(prefixOrder, sourceIndex) - } else { - suffixOrder = append(suffixOrder, sourceIndex) + if isFileInThisChunk { + if sourceIndex == ast.RuntimeSourceIndex || fileMeta.cjsWrap { + prefixOrder = append(prefixOrder, sourceIndex) + } else { + suffixOrder = append(suffixOrder, sourceIndex) + } } } diff --git a/internal/parser/parser.go b/internal/parser/parser.go index 115840a946e..a4c7bb4dde2 100644 --- a/internal/parser/parser.go +++ b/internal/parser/parser.go @@ -8660,6 +8660,12 @@ func (p *parser) stmtsCanBeRemovedIfUnused(stmts []ast.Stmt) bool { case *ast.SFunction, *ast.SEmpty: // These never have side effects + case *ast.SImport: + // Let these be removed if they are unused. Note that we also need to + // check if the imported file is marked as "sideEffects: false" before we + // can remove a SImport statement. Otherwise the import must be kept for + // its side effects. + case *ast.SClass: if !p.classCanBeRemovedIfUnused(s.Class) { return false diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 2ddbffa553e..a27756d067f 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -21,8 +21,17 @@ const ( ResolveExternal ) +type ResolveResult struct { + AbsolutePath string + Status ResolveStatus + + // If true, any ES6 imports to this file can be considered to have no side + // effects. This means they should be removed if unused. + IgnoreIfUnused bool +} + type Resolver interface { - Resolve(sourcePath string, importPath string) (string, ResolveStatus) + Resolve(sourcePath string, importPath string) ResolveResult Read(path string) (string, bool) PrettyPath(path string) string } @@ -74,27 +83,34 @@ func NewResolver(fs fs.FS, log logging.Log, options ResolveOptions) Resolver { } } -func (r *resolver) Resolve(sourcePath string, importPath string) (string, ResolveStatus) { - absolute, status := r.resolveWithoutSymlinks(sourcePath, importPath) +func (r *resolver) Resolve(sourcePath string, importPath string) (result ResolveResult) { + result.AbsolutePath, result.Status = r.resolveWithoutSymlinks(sourcePath, importPath) // If successful, resolve symlinks using the directory info cache - if status == ResolveEnabled || status == ResolveDisabled { - if dirInfo := r.dirInfoCached(r.fs.Dir(absolute)); dirInfo != nil { - base := r.fs.Base(absolute) + if result.Status == ResolveEnabled || result.Status == ResolveDisabled { + if dirInfo := r.dirInfoCached(r.fs.Dir(result.AbsolutePath)); dirInfo != nil { + base := r.fs.Base(result.AbsolutePath) + + // Look up this file in the "sideEffects" map in "package.json" + if dirInfo.packageJson != nil && dirInfo.packageJson.sideEffectsMap != nil { + result.IgnoreIfUnused = !dirInfo.packageJson.sideEffectsMap[result.AbsolutePath] + } // Is this entry itself a symlink? if entry := dirInfo.entries[base]; entry.Symlink != "" { - return entry.Symlink, status + result.AbsolutePath = entry.Symlink + return } // Is there at least one parent directory with a symlink? if dirInfo.absRealPath != "" { - return r.fs.Join(dirInfo.absRealPath, base), status + result.AbsolutePath = r.fs.Join(dirInfo.absRealPath, base) + return } } } - return absolute, status + return } func (r *resolver) resolveWithoutSymlinks(sourcePath string, importPath string) (string, ResolveStatus) { @@ -228,6 +244,18 @@ type packageJson struct { // say almost nothing: https://docs.npmjs.com/files/package.json. browserNonModuleMap map[string]*string browserModuleMap map[string]*string + + // If this is non-nil, each entry in this map is the absolute path of a file + // with side effects. Any entry not in this map should be considered to have + // no side effects, which means import statements for these files can be + // removed if none of the imports are used. This is a convention from Webpack: + // https://webpack.js.org/guides/tree-shaking/. + // + // Note that if a file is included, all statements that can't be proven to be + // free of side effects must be included. This convention does not say + // anything about whether any statements within the file have side effects or + // not. + sideEffectsMap map[string]bool } type tsConfigJson struct { @@ -453,7 +481,7 @@ func (r *resolver) dirInfoUncached(path string) *dirInfo { } func (r *resolver) parsePackageJSON(path string) *packageJson { - json, _, ok := r.parseJSON(r.fs.Join(path, "package.json"), parser.ParseJSONOptions{}) + json, jsonSource, ok := r.parseJSON(r.fs.Join(path, "package.json"), parser.ParseJSONOptions{}) if !ok { return nil } @@ -538,6 +566,36 @@ func (r *resolver) parsePackageJSON(path string) *packageJson { } } + // Read the "sideEffects" property + if sideEffectsJson, _, ok := getProperty(json, "sideEffects"); ok { + switch data := sideEffectsJson.Data.(type) { + case *ast.EBoolean: + if !data.Value { + // Make an empty map for "sideEffects: false", which indicates all + // files in this module can be considered to not have side effects. + packageJson.sideEffectsMap = make(map[string]bool) + } + + case *ast.EArray: + // The "sideEffects: []" format means all files in this module but not in + // the array can be considered to not have side effects. + packageJson.sideEffectsMap = make(map[string]bool) + for _, itemJson := range data.Items { + if item, ok := itemJson.Data.(*ast.EString); ok && item.Value != nil { + absolute := r.fs.Join(path, lexer.UTF16ToString(item.Value)) + packageJson.sideEffectsMap[absolute] = true + } else { + r.log.AddWarning(jsonSource, itemJson.Loc, + "Expected string in array for \"sideEffects\"") + } + } + + default: + r.log.AddWarning(jsonSource, sideEffectsJson.Loc, + "Invalid value for \"sideEffects\"") + } + } + // Delay parsing "main" into an absolute path in case "browser" replaces it if mainPath != "" { // Is it a file? diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go index 81065500655..d4be0c52d38 100644 --- a/internal/runtime/runtime.go +++ b/internal/runtime/runtime.go @@ -4,7 +4,7 @@ const Code = ` let __defineProperty = Object.defineProperty let __hasOwnProperty = Object.prototype.hasOwnProperty let __getOwnPropertySymbols = Object.getOwnPropertySymbols - let __propertyIsEnumerable = Object.propertyIsEnumerable + let __propertyIsEnumerable = Object.prototype.propertyIsEnumerable export let __pow = Math.pow export let __assign = Object.assign diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js index 8c955c4c974..974c76a00e5 100644 --- a/scripts/end-to-end-tests.js +++ b/scripts/end-to-end-tests.js @@ -258,6 +258,51 @@ }), ) + // Test tree shaking + tests.push( + // Keep because used (ES6) + test(['--bundle', 'entry.js', '--outfile=node.js'], { + 'entry.js': `import * as foo from './foo'; if (global.dce0 !== 123 || foo.abc !== 'abc') throw 'fail'`, + 'foo/index.js': `global.dce0 = 123; export const abc = 'abc'`, + 'foo/package.json': `{ "sideEffects": false }`, + }), + + // Remove because unused (ES6) + test(['--bundle', 'entry.js', '--outfile=node.js'], { + 'entry.js': `import * as foo from './foo'; if (global.dce1 !== void 0) throw 'fail'`, + 'foo/index.js': `global.dce1 = 123; export const abc = 'abc'`, + 'foo/package.json': `{ "sideEffects": false }`, + }), + + // Keep because side effects (ES6) + test(['--bundle', 'entry.js', '--outfile=node.js'], { + 'entry.js': `import * as foo from './foo'; if (global.dce2 !== 123) throw 'fail'`, + 'foo/index.js': `global.dce2 = 123; export const abc = 'abc'`, + 'foo/package.json': `{ "sideEffects": true }`, + }), + + // Keep because used (CommonJS) + test(['--bundle', 'entry.js', '--outfile=node.js'], { + 'entry.js': `import foo from './foo'; if (global.dce3 !== 123 || foo.abc !== 'abc') throw 'fail'`, + 'foo/index.js': `global.dce3 = 123; exports.abc = 'abc'`, + 'foo/package.json': `{ "sideEffects": false }`, + }), + + // Remove because unused (CommonJS) + test(['--bundle', 'entry.js', '--outfile=node.js'], { + 'entry.js': `import foo from './foo'; if (global.dce4 !== void 0) throw 'fail'`, + 'foo/index.js': `global.dce4 = 123; exports.abc = 'abc'`, + 'foo/package.json': `{ "sideEffects": false }`, + }), + + // Keep because side effects (CommonJS) + test(['--bundle', 'entry.js', '--outfile=node.js'], { + 'entry.js': `import foo from './foo'; if (global.dce5 !== 123) throw 'fail'`, + 'foo/index.js': `global.dce5 = 123; exports.abc = 'abc'`, + 'foo/package.json': `{ "sideEffects": true }`, + }), + ) + function test(args, files, options) { return async () => { const hasBundle = args.includes('--bundle')