fix #458: add "--tree-shaking=ignore-annotations"

CHANGELOG.md

@@ -8,6 +8,14 @@
 
     Right now comments are included in this character frequency histogram. This is not a correctness issue but it does mean that documents with the same code but different comments may be minified to different output files. This release fixes this difference by removing comments from the character frequency histogram.
 
+* Add an option to ignore tree-shaking annotations ([#458](https://github.com/evanw/esbuild/issues/458))
+
+    Tree shaking is the term the JavaScript community uses for dead code elimination, a common compiler optimization that automatically removes unreachable code. Since JavaScript is a dynamic language, identifying unused code is sometimes very difficult for a compiler, so the community has developed certain annotations to help tell compilers what code should be considered unused. Currently there two forms of tree-shaking annotations that esbuild supports: inline `/* @__PURE__ */` comments before function calls and the `sideEffects` field in `package.json`.
+
+    These annotations can be problematic because the compiler depends completely on developers for accuracy and the annotations are occasionally incorrect. The `sideEffects` field is particularly error-prone because by default it causes all files in your package to be considered dead code if no imports are used. If you add a new file containing side effects and forget to update that field, your package will break when people try to bundle it.
+
+    This release adds a new flag `--tree-shaking=ignore-annotations` to allow you to bundle code that contains incorrect tree-shaking annotations with esbuild. An example of such code is [@tensorflow/tfjs](https://github.com/tensorflow/tfjs). Ideally the `--tree-shaking=ignore-annotations` flag is only a temporary workaround. You should report these issues to the maintainer of the package to get them fixed since they will trip up other people too.
+
 ## 0.8.9
 
 * Add support for the `mips64le` architecture ([#523](https://github.com/evanw/esbuild/issues/523))

cmd/esbuild/main.go

@@ -17,6 +17,7 @@ const helpText = `
 Usage:
   esbuild [options] [entry points]
 
+
 Options:
   --bundle              Bundle all dependencies into the output files
   --define:K=V          Substitute K with V while parsing
@@ -68,6 +69,8 @@ Advanced options:
   --sourcefile=...          Set the source file for the source map (for stdin)
   --sourcemap=external      Do not link to the source map with a comment
   --sourcemap=inline        Emit the source map with an inline data URL
+  --tree-shaking=...        Set to "ignore-annotations" to work with packages
+                            that have incorrect tree-shaking annotations
   --tsconfig=...            Use this tsconfig.json file instead of other ones
   --version                 Print the current version and exit (` + esbuildVersion + `)
 

internal/bundler/bundler_dce_test.go

@@ -980,3 +980,44 @@ func TestTreeShakingReactElements(t *testing.T) {
 		},
 	})
 }
+
+func TestDisableTreeShaking(t *testing.T) {
+	defines := config.ProcessDefines(map[string]config.DefineData{
+		"pure":    config.DefineData{CallCanBeUnwrappedIfUnused: true},
+		"some.fn": config.DefineData{CallCanBeUnwrappedIfUnused: true},
+	})
+	dce_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.jsx": `
+				import './remove-me'
+				function RemoveMe1() {}
+				let removeMe2 = 0
+				class RemoveMe3 {}
+
+				import './keep-me'
+				function KeepMe1() {}
+				let keepMe2 = <KeepMe1/>
+				function keepMe3() { console.log('side effects') }
+				let keepMe4 = /* @__PURE__ */ keepMe3()
+				let keepMe5 = pure()
+				let keepMe6 = some.fn()
+			`,
+			"/remove-me.js": `
+				export default 'unused'
+			`,
+			"/keep-me/index.js": `
+				console.log('side effects')
+			`,
+			"/keep-me/package.json": `
+				{ "sideEffects": false }
+			`,
+		},
+		entryPaths: []string{"/entry.jsx"},
+		options: config.Options{
+			Mode:                 config.ModeBundle,
+			AbsOutputFile:        "/out.js",
+			IgnoreDCEAnnotations: true,
+			Defines:              &defines,
+		},
+	})
+}

internal/bundler/linker.go

@@ -2194,7 +2194,7 @@ func (c *linkerContext) includeFile(sourceIndex uint32, entryPointBit uint, dist
 
 					// Don't include this module for its side effects if it can be
 					// considered to have no side effects
-					if otherFile := &c.files[otherSourceIndex]; otherFile.ignoreIfUnused {
+					if otherFile := &c.files[otherSourceIndex]; otherFile.ignoreIfUnused && !c.options.IgnoreDCEAnnotations {
 						if record.WasOriginallyBareImport {
 							var notes []logger.MsgData
 							if otherFile.ignoreIfUnusedData != nil {

internal/bundler/snapshots/snapshots_dce.txt

@@ -9,6 +9,23 @@ TestDataURLLoaderRemoveUnused
 // /entry.js
 console.log("unused import");
 
+================================================================================
+TestDisableTreeShaking
+---------- /out.js ----------
+// /keep-me/index.js
+console.log("side effects");
+
+// /entry.jsx
+function KeepMe1() {
+}
+let keepMe2 = React.createElement(KeepMe1, null);
+function keepMe3() {
+  console.log("side effects");
+}
+let keepMe4 = keepMe3();
+let keepMe5 = pure();
+let keepMe6 = some.fn();
+
 ================================================================================
 TestFileLoaderRemoveUnused
 ---------- /out.js ----------

internal/config/config.go

@@ -177,6 +177,7 @@ type Options struct {
 	AvoidTDZ                bool
 	ASCIIOnly               bool
 	KeepNames               bool
+	IgnoreDCEAnnotations    bool
 
 	Defines  *ProcessedDefines
 	TS       TSOptions

internal/js_parser/js_parser.go

@@ -2765,7 +2765,7 @@ func (p *parser) parseExprWithFlags(level js_ast.L, flags exprFlag) js_ast.Expr
 }
 
 func (p *parser) parseExprCommon(level js_ast.L, errors *deferredErrors, flags exprFlag) js_ast.Expr {
-	hadPureCommentBefore := p.lexer.HasPureCommentBefore
+	hadPureCommentBefore := p.lexer.HasPureCommentBefore && !p.IgnoreDCEAnnotations
 	expr := p.parsePrefix(level, errors, flags)
 
 	// There is no formal spec for "__PURE__" comments but from reverse-
@@ -8242,7 +8242,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 				if data.CanBeRemovedIfUnused {
 					e.CanBeRemovedIfUnused = true
 				}
-				if data.CallCanBeUnwrappedIfUnused {
+				if data.CallCanBeUnwrappedIfUnused && !p.IgnoreDCEAnnotations {
 					e.CallCanBeUnwrappedIfUnused = true
 				}
 				if data.WarnAboutLackOfDefine {
@@ -8302,7 +8302,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 			Args:   args,
 
 			// Enable tree shaking
-			CanBeUnwrappedIfUnused: true,
+			CanBeUnwrappedIfUnused: !p.IgnoreDCEAnnotations,
 		}}, exprOut{}
 
 	case *js_ast.ETemplate:
@@ -9006,7 +9006,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 					if define.Data.CanBeRemovedIfUnused {
 						e.CanBeRemovedIfUnused = true
 					}
-					if define.Data.CallCanBeUnwrappedIfUnused {
+					if define.Data.CallCanBeUnwrappedIfUnused && !p.IgnoreDCEAnnotations {
 						e.CallCanBeUnwrappedIfUnused = true
 					}
 					if define.Data.WarnAboutLackOfDefine {

lib/common.ts

@@ -83,6 +83,7 @@ function pushCommonFlags(flags: string[], options: CommonOptions, keys: OptionKe
   let minifyWhitespace = getFlag(options, keys, 'minifyWhitespace', mustBeBoolean);
   let minifyIdentifiers = getFlag(options, keys, 'minifyIdentifiers', mustBeBoolean);
   let charset = getFlag(options, keys, 'charset', mustBeString);
+  let treeShaking = getFlag(options, keys, 'treeShaking', mustBeStringOrBoolean);
   let jsxFactory = getFlag(options, keys, 'jsxFactory', mustBeString);
   let jsxFragment = getFlag(options, keys, 'jsxFragment', mustBeString);
   let define = getFlag(options, keys, 'define', mustBeObject);
@@ -104,6 +105,7 @@ function pushCommonFlags(flags: string[], options: CommonOptions, keys: OptionKe
   if (minifyWhitespace) flags.push('--minify-whitespace');
   if (minifyIdentifiers) flags.push('--minify-identifiers');
   if (charset) flags.push(`--charset=${charset}`);
+  if (treeShaking !== void 0 && treeShaking !== true) flags.push(`--tree-shaking=${treeShaking}`);
 
   if (jsxFactory) flags.push(`--jsx-factory=${jsxFactory}`);
   if (jsxFragment) flags.push(`--jsx-fragment=${jsxFragment}`);

lib/types.ts

@@ -3,6 +3,7 @@ export type Format = 'iife' | 'cjs' | 'esm';
 export type Loader = 'js' | 'jsx' | 'ts' | 'tsx' | 'css' | 'json' | 'text' | 'base64' | 'file' | 'dataurl' | 'binary' | 'default';
 export type LogLevel = 'info' | 'warning' | 'error' | 'silent';
 export type Charset = 'ascii' | 'utf8';
+export type TreeShaking = true | 'ignore-annotations';
 
 interface CommonOptions {
   sourcemap?: boolean | 'inline' | 'external';
@@ -15,6 +16,7 @@ interface CommonOptions {
   minifyIdentifiers?: boolean;
   minifySyntax?: boolean;
   charset?: Charset;
+  treeShaking?: TreeShaking;
 
   jsxFactory?: string;
   jsxFragment?: string;

pkg/api/api.go

@@ -188,6 +188,13 @@ const (
 	CharsetUTF8
 )
 
+type TreeShaking uint8
+
+const (
+	TreeShakingDefault TreeShaking = iota
+	TreeShakingIgnoreAnnotations
+)
+
 ////////////////////////////////////////////////////////////////////////////////
 // Build API
 
@@ -204,6 +211,7 @@ type BuildOptions struct {
 	MinifyIdentifiers bool
 	MinifySyntax      bool
 	Charset           Charset
+	TreeShaking       TreeShaking
 
 	JSXFactory  string
 	JSXFragment string
@@ -280,6 +288,7 @@ type TransformOptions struct {
 	MinifyIdentifiers bool
 	MinifySyntax      bool
 	Charset           Charset
+	TreeShaking       TreeShaking
 
 	JSXFactory  string
 	JSXFragment string

pkg/api/api_impl.go

@@ -102,6 +102,17 @@ func validateASCIIOnly(value Charset) bool {
 	}
 }
 
+func validateIgnoreDCEAnnotations(value TreeShaking) bool {
+	switch value {
+	case TreeShakingDefault:
+		return false
+	case TreeShakingIgnoreAnnotations:
+		return true
+	default:
+		panic("Invalid tree shaking")
+	}
+}
+
 func validateLoader(value Loader) config.Loader {
 	switch value {
 	case LoaderNone:
@@ -470,32 +481,33 @@ func buildImpl(buildOpts BuildOptions) BuildResult {
 			Factory:  validateJSX(log, buildOpts.JSXFactory, "factory"),
 			Fragment: validateJSX(log, buildOpts.JSXFragment, "fragment"),
 		},
-		Defines:           validateDefines(log, buildOpts.Define, buildOpts.Pure),
-		Platform:          validatePlatform(buildOpts.Platform),
-		SourceMap:         validateSourceMap(buildOpts.Sourcemap),
-		MangleSyntax:      buildOpts.MinifySyntax,
-		RemoveWhitespace:  buildOpts.MinifyWhitespace,
-		MinifyIdentifiers: buildOpts.MinifyIdentifiers,
-		ASCIIOnly:         validateASCIIOnly(buildOpts.Charset),
-		ModuleName:        validateGlobalName(log, buildOpts.GlobalName),
-		CodeSplitting:     buildOpts.Splitting,
-		OutputFormat:      validateFormat(buildOpts.Format),
-		AbsOutputFile:     validatePath(log, realFS, buildOpts.Outfile),
-		AbsOutputDir:      validatePath(log, realFS, buildOpts.Outdir),
-		AbsOutputBase:     validatePath(log, realFS, buildOpts.Outbase),
-		AbsMetadataFile:   validatePath(log, realFS, buildOpts.Metafile),
-		OutputExtensions:  validateOutputExtensions(log, buildOpts.OutExtensions),
-		ExtensionToLoader: validateLoaders(log, buildOpts.Loader),
-		ExtensionOrder:    validateResolveExtensions(log, buildOpts.ResolveExtensions),
-		ExternalModules:   validateExternals(log, realFS, buildOpts.External),
-		TsConfigOverride:  validatePath(log, realFS, buildOpts.Tsconfig),
-		MainFields:        buildOpts.MainFields,
-		PublicPath:        buildOpts.PublicPath,
-		AvoidTDZ:          buildOpts.AvoidTDZ,
-		KeepNames:         buildOpts.KeepNames,
-		InjectAbsPaths:    make([]string, len(buildOpts.Inject)),
-		Banner:            buildOpts.Banner,
-		Footer:            buildOpts.Footer,
+		Defines:              validateDefines(log, buildOpts.Define, buildOpts.Pure),
+		Platform:             validatePlatform(buildOpts.Platform),
+		SourceMap:            validateSourceMap(buildOpts.Sourcemap),
+		MangleSyntax:         buildOpts.MinifySyntax,
+		RemoveWhitespace:     buildOpts.MinifyWhitespace,
+		MinifyIdentifiers:    buildOpts.MinifyIdentifiers,
+		ASCIIOnly:            validateASCIIOnly(buildOpts.Charset),
+		IgnoreDCEAnnotations: validateIgnoreDCEAnnotations(buildOpts.TreeShaking),
+		ModuleName:           validateGlobalName(log, buildOpts.GlobalName),
+		CodeSplitting:        buildOpts.Splitting,
+		OutputFormat:         validateFormat(buildOpts.Format),
+		AbsOutputFile:        validatePath(log, realFS, buildOpts.Outfile),
+		AbsOutputDir:         validatePath(log, realFS, buildOpts.Outdir),
+		AbsOutputBase:        validatePath(log, realFS, buildOpts.Outbase),
+		AbsMetadataFile:      validatePath(log, realFS, buildOpts.Metafile),
+		OutputExtensions:     validateOutputExtensions(log, buildOpts.OutExtensions),
+		ExtensionToLoader:    validateLoaders(log, buildOpts.Loader),
+		ExtensionOrder:       validateResolveExtensions(log, buildOpts.ResolveExtensions),
+		ExternalModules:      validateExternals(log, realFS, buildOpts.External),
+		TsConfigOverride:     validatePath(log, realFS, buildOpts.Tsconfig),
+		MainFields:           buildOpts.MainFields,
+		PublicPath:           buildOpts.PublicPath,
+		AvoidTDZ:             buildOpts.AvoidTDZ,
+		KeepNames:            buildOpts.KeepNames,
+		InjectAbsPaths:       make([]string, len(buildOpts.Inject)),
+		Banner:               buildOpts.Banner,
+		Footer:               buildOpts.Footer,
 	}
 	for i, path := range buildOpts.Inject {
 		options.InjectAbsPaths[i] = validatePath(log, realFS, path)
@@ -720,6 +732,7 @@ func transformImpl(input string, transformOpts TransformOptions) TransformResult
 		RemoveWhitespace:        transformOpts.MinifyWhitespace,
 		MinifyIdentifiers:       transformOpts.MinifyIdentifiers,
 		ASCIIOnly:               validateASCIIOnly(transformOpts.Charset),
+		IgnoreDCEAnnotations:    validateIgnoreDCEAnnotations(transformOpts.TreeShaking),
 		AbsOutputFile:           transformOpts.Sourcefile + "-out",
 		AvoidTDZ:                transformOpts.AvoidTDZ,
 		KeepNames:               transformOpts.KeepNames,

pkg/cli/cli_impl.go

@@ -87,6 +87,21 @@ func parseOptionsImpl(osArgs []string, buildOpts *api.BuildOptions, transformOpt
 				return fmt.Errorf("Invalid charset value: %q (valid: ascii, utf8)", name)
 			}
 
+		case strings.HasPrefix(arg, "--tree-shaking="):
+			var value *api.TreeShaking
+			if buildOpts != nil {
+				value = &buildOpts.TreeShaking
+			} else {
+				value = &transformOpts.TreeShaking
+			}
+			name := arg[len("--tree-shaking="):]
+			switch name {
+			case "ignore-annotations":
+				*value = api.TreeShakingIgnoreAnnotations
+			default:
+				return fmt.Errorf("Invalid tree shaking value: %q (valid: ignore-annotations)", name)
+			}
+
 		case arg == "--avoid-tdz":
 			if buildOpts != nil {
 				buildOpts.AvoidTDZ = true