Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enable tree-shaking in transform mode when both --minify and --format flags are specified #1617

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
# Changelog

## Unreleased

* Enable tree-shaking in transform mode when target format is specified ([#1610](https://github.com/evanw/esbuild/issues/1610))

Previously esbuild does not perform tree-shaking when transforming single files because esbuild cannot assume whether the file will be concatenated with other files, and thus it would be unsafe to remove unused bindings.

However, when an output format is explicitly specified, it implies the file is a module so tree-shaking can be safely applied. In addition, the previous behavior when both `--minify` and `--format` are specified was already concatenation-unsafe, since esbuild renames top-level bindings in this case.

Before this release, there was no API that allows the user to force tree-shaking when transforming a single file. As of this release, esbuild will now enable tree-shaking when transforming a single file with both `--minify` and `--format` specified. This behavior can also be individually enabled with the `--minify-unused` flag.

This improvement is important for tools that intend to use esbuild as a non-bundling minifier to replace Terser - for example, a Vite or Rollup plugin that minifies ESM chunks produced by Rollup.

## 0.12.28

* Fix U+30FB and U+FF65 in identifier names in ES5 vs. ES6+ ([#1599](https://github.com/evanw/esbuild/issues/1599))
Expand Down
57 changes: 57 additions & 0 deletions internal/bundler/bundler_dce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1630,6 +1630,63 @@ func TestTreeShakingNoBundleIIFE(t *testing.T) {
})
}

func TestTreeShakingNoBundleMinifyESM(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.js": `
function keep() {}
function unused() {}
keep()
`,
},
entryPaths: []string{"/entry.js"},
options: config.Options{
Mode: config.ModeConvertFormat,
OutputFormat: config.FormatESModule,
RemoveUnused: true,
AbsOutputFile: "/out.js",
},
})
}

func TestTreeShakingNoBundleMinifyIIFE(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.js": `
function keep() {}
function unused() {}
keep()
`,
},
entryPaths: []string{"/entry.js"},
options: config.Options{
Mode: config.ModeConvertFormat,
OutputFormat: config.FormatIIFE,
RemoveUnused: true,
AbsOutputFile: "/out.js",
},
})
}

func TestTreeShakingNoBundleMinifyCJS(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.js": `
function keep() {}
function unused() {}
keep()
`,
},
entryPaths: []string{"/entry.js"},
options: config.Options{
Mode: config.ModeConvertFormat,
OutputFormat: config.FormatCommonJS,
RemoveUnused: true,
AbsOutputFile: "/out.js",
},
})
}

func TestTreeShakingInESMWrapper(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
Expand Down
2 changes: 1 addition & 1 deletion internal/bundler/linker.go
Original file line number Diff line number Diff line change
Expand Up @@ -2571,7 +2571,7 @@ func (c *linkerContext) markFileLiveForTreeShaking(sourceIndex uint32) {

switch repr := file.InputFile.Repr.(type) {
case *graph.JSRepr:
isTreeShakingEnabled := config.IsTreeShakingEnabled(c.options.Mode, c.options.OutputFormat)
isTreeShakingEnabled := config.IsTreeShakingEnabled(c.options.Mode, c.options.OutputFormat, c.options.RemoveUnused)

// If the JavaScript stub for a CSS file is included, also include the CSS file
if repr.CSSSourceIndex.IsValid() {
Expand Down
23 changes: 23 additions & 0 deletions internal/bundler/snapshots/snapshots_dce.txt
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,29 @@ TestTreeShakingNoBundleIIFE
keep();
})();

================================================================================
TestTreeShakingNoBundleMinifyCJS
---------- /out.js ----------
function keep() {
}
keep();

================================================================================
TestTreeShakingNoBundleMinifyESM
---------- /out.js ----------
function keep() {
}
keep();

================================================================================
TestTreeShakingNoBundleMinifyIIFE
---------- /out.js ----------
(() => {
function keep() {
}
keep();
})();

================================================================================
TestTreeShakingReactElements
---------- /out.js ----------
Expand Down
5 changes: 3 additions & 2 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ type Options struct {
RemoveWhitespace bool
MinifyIdentifiers bool
MangleSyntax bool
RemoveUnused bool
ProfilerNames bool
CodeSplitting bool
WatchMode bool
Expand Down Expand Up @@ -384,8 +385,8 @@ func SubstituteTemplate(template []PathTemplate, placeholders PathPlaceholders)
return result
}

func IsTreeShakingEnabled(mode Mode, outputFormat Format) bool {
return mode == ModeBundle || (mode == ModeConvertFormat && outputFormat == FormatIIFE)
func IsTreeShakingEnabled(mode Mode, outputFormat Format, minifyUnused bool) bool {
return mode == ModeBundle || (mode == ModeConvertFormat && (outputFormat == FormatIIFE || minifyUnused))
}

func ShouldCallRuntimeRequire(mode Mode, outputFormat Format) bool {
Expand Down
4 changes: 3 additions & 1 deletion internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ type optionsThatSupportStructuralEquality struct {
keepNames bool
mangleSyntax bool
minifyIdentifiers bool
removeUnused bool
omitRuntimeForTests bool
ignoreDCEAnnotations bool
preserveUnusedImportsTS bool
Expand All @@ -359,6 +360,7 @@ func OptionsFromConfig(options *config.Options) Options {
keepNames: options.KeepNames,
mangleSyntax: options.MangleSyntax,
minifyIdentifiers: options.MinifyIdentifiers,
removeUnused: options.RemoveUnused,
omitRuntimeForTests: options.OmitRuntimeForTests,
ignoreDCEAnnotations: options.IgnoreDCEAnnotations,
preserveUnusedImportsTS: options.PreserveUnusedImportsTS,
Expand Down Expand Up @@ -13830,7 +13832,7 @@ func Parse(log logger.Log, source logger.Source, options Options) (result js_ast
// single pass, but it turns out it's pretty much impossible to do this
// correctly while handling arrow functions because of the grammar
// ambiguities.
if !config.IsTreeShakingEnabled(p.options.mode, p.options.outputFormat) {
if !config.IsTreeShakingEnabled(p.options.mode, p.options.outputFormat, p.options.removeUnused) {
// When not bundling, everything comes in a single part
parts = p.appendPart(parts, stmts)
} else {
Expand Down
2 changes: 2 additions & 0 deletions lib/shared/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ function pushCommonFlags(flags: string[], options: CommonOptions, keys: OptionKe
let minifySyntax = getFlag(options, keys, 'minifySyntax', mustBeBoolean);
let minifyWhitespace = getFlag(options, keys, 'minifyWhitespace', mustBeBoolean);
let minifyIdentifiers = getFlag(options, keys, 'minifyIdentifiers', mustBeBoolean);
let minifyUnused = getFlag(options, keys, 'minifyUnused', mustBeBoolean);
let charset = getFlag(options, keys, 'charset', mustBeString);
let treeShaking = getFlag(options, keys, 'treeShaking', mustBeStringOrBoolean);
let jsx = getFlag(options, keys, 'jsx', mustBeString);
Expand All @@ -130,6 +131,7 @@ function pushCommonFlags(flags: string[], options: CommonOptions, keys: OptionKe
if (minifySyntax) flags.push('--minify-syntax');
if (minifyWhitespace) flags.push('--minify-whitespace');
if (minifyIdentifiers) flags.push('--minify-identifiers');
if (minifyUnused) flags.push('--minify-unused');
if (charset) flags.push(`--charset=${charset}`);
if (treeShaking !== void 0 && treeShaking !== true) flags.push(`--tree-shaking=${treeShaking}`);

Expand Down
1 change: 1 addition & 0 deletions lib/shared/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ interface CommonOptions {
minifyWhitespace?: boolean;
minifyIdentifiers?: boolean;
minifySyntax?: boolean;
minifyUnused?: boolean;
charset?: Charset;
treeShaking?: TreeShaking;

Expand Down
2 changes: 2 additions & 0 deletions pkg/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ type BuildOptions struct {
MinifyWhitespace bool
MinifyIdentifiers bool
MinifySyntax bool
MinifyUnused bool
Charset Charset
TreeShaking TreeShaking
LegalComments LegalComments
Expand Down Expand Up @@ -364,6 +365,7 @@ type TransformOptions struct {
MinifyWhitespace bool
MinifyIdentifiers bool
MinifySyntax bool
MinifyUnused bool
Charset Charset
TreeShaking TreeShaking
LegalComments LegalComments
Expand Down
4 changes: 3 additions & 1 deletion pkg/api/api_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,7 @@ func rebuildImpl(
outJS, outCSS := validateOutputExtensions(log, buildOpts.OutExtensions)
bannerJS, bannerCSS := validateBannerOrFooter(log, "banner", buildOpts.Banner)
footerJS, footerCSS := validateBannerOrFooter(log, "footer", buildOpts.Footer)
minify := buildOpts.MinifyWhitespace && buildOpts.MinifyIdentifiers && buildOpts.MinifySyntax
minify := buildOpts.MinifyWhitespace && buildOpts.MinifyIdentifiers && buildOpts.MinifySyntax && buildOpts.MinifyUnused
defines, injectedDefines := validateDefines(log, buildOpts.Define, buildOpts.Pure, buildOpts.Platform, minify)
options := config.Options{
IsTargetUnconfigured: isTargetUnconfigured,
Expand All @@ -826,6 +826,7 @@ func rebuildImpl(
MangleSyntax: buildOpts.MinifySyntax,
RemoveWhitespace: buildOpts.MinifyWhitespace,
MinifyIdentifiers: buildOpts.MinifyIdentifiers,
RemoveUnused: buildOpts.MinifyUnused,
AllowOverwrite: buildOpts.AllowOverwrite,
ASCIIOnly: validateASCIIOnly(buildOpts.Charset),
IgnoreDCEAnnotations: validateIgnoreDCEAnnotations(buildOpts.TreeShaking),
Expand Down Expand Up @@ -1310,6 +1311,7 @@ func transformImpl(input string, transformOpts TransformOptions) TransformResult
MangleSyntax: transformOpts.MinifySyntax,
RemoveWhitespace: transformOpts.MinifyWhitespace,
MinifyIdentifiers: transformOpts.MinifyIdentifiers,
RemoveUnused: transformOpts.MinifyUnused,
ASCIIOnly: validateASCIIOnly(transformOpts.Charset),
IgnoreDCEAnnotations: validateIgnoreDCEAnnotations(transformOpts.TreeShaking),
AbsOutputFile: transformOpts.Sourcefile + "-out",
Expand Down
9 changes: 9 additions & 0 deletions pkg/cli/cli_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,12 @@ func parseOptionsImpl(
buildOpts.MinifySyntax = true
buildOpts.MinifyWhitespace = true
buildOpts.MinifyIdentifiers = true
buildOpts.MinifyUnused = true
} else {
transformOpts.MinifySyntax = true
transformOpts.MinifyWhitespace = true
transformOpts.MinifyIdentifiers = true
transformOpts.MinifyUnused = true
}

case arg == "--minify-syntax":
Expand All @@ -95,6 +97,13 @@ func parseOptionsImpl(
transformOpts.MinifyIdentifiers = true
}

case arg == "--minify-unused":
if buildOpts != nil {
buildOpts.MinifyUnused = true
} else {
transformOpts.MinifyUnused = true
}

case strings.HasPrefix(arg, "--legal-comments="):
value := arg[len("--legal-comments="):]
var legalComments api.LegalComments
Expand Down
15 changes: 15 additions & 0 deletions scripts/js-api-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -3231,6 +3231,21 @@ let transformTests = {
assert.strictEqual(code, `fn(), React.createElement("div", null);\n`)
},

async treeShakingMinifyWithFormat({ esbuild }) {
const { code } = await esbuild.transform(`let unused = 1`, {
minifyUnused: true,
format: 'esm'
})
assert.strictEqual(code, ``)
},

async treeShakingMinifyWithoutFormat({ esbuild }) {
const { code } = await esbuild.transform(`let unused = 1`, {
minifyUnused: true
})
assert.strictEqual(code, `let unused = 1;\n`)
},

async jsCharsetDefault({ esbuild }) {
const { code } = await esbuild.transform(`let π = 'π'`, {})
assert.strictEqual(code, `let \\u03C0 = "\\u03C0";\n`)
Expand Down