Skip to content

Commit

Permalink
fix #3511: @__NO_SIDE_EFFECTS__ with templates
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Dec 18, 2023
1 parent 00c4ebe commit d968af2
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 1 deletion.
22 changes: 22 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,28 @@
With this release, esbuild will now attempt to terminate the Go GC in this edge case by calling `clearTimeout()` on these pending timeouts.
* Apply `/* @__NO_SIDE_EFFECTS__ */` on tagged template literals ([#3511](https://github.com/evanw/esbuild/issues/3511))
Tagged template literals that reference functions annotated with a `@__NO_SIDE_EFFECTS__` comment are now able to be removed via tree-shaking if the result is unused. This is a convention from [Rollup](https://github.com/rollup/rollup/pull/5024). Here is an example:
```js
// Original code
const html = /* @__NO_SIDE_EFFECTS__ */ (a, ...b) => ({ a, b })
html`<a>remove</a>`
x = html`<b>keep</b>`

// Old output (with --tree-shaking=true)
const html = /* @__NO_SIDE_EFFECTS__ */ (a, ...b) => ({ a, b });
html`<a>remove</a>`;
x = html`<b>keep</b>`;

// New output (with --tree-shaking=true)
const html = /* @__NO_SIDE_EFFECTS__ */ (a, ...b) => ({ a, b });
x = html`<b>keep</b>`;
```
Note that this feature currently only works within a single file, so it's not especially useful. This feature does not yet work across separate files. I still recommend using `@__PURE__` annotations instead of this feature, as they have wider tooling support. The drawback of course is that `@__PURE__` annotations need to be added at each call site, not at the declaration, and for non-call expressions such as template literals you need to wrap the expression in an IIFE (immediately-invoked function expression) to create a call expression to apply the `@__PURE__` annotation to.
* Publish builds for IBM AIX PowerPC 64-bit ([#3549](https://github.com/evanw/esbuild/issues/3549))
This release publishes a binary executable to npm for IBM AIX PowerPC 64-bit, which means that in theory esbuild can now be installed in that environment with `npm install esbuild`. This hasn't actually been tested yet. If you have access to such a system, it would be helpful to confirm whether or not doing this actually works.
Expand Down
25 changes: 25 additions & 0 deletions internal/bundler_tests/bundler_dce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1222,6 +1222,31 @@ func TestRemoveUnusedPureCommentCalls(t *testing.T) {
})
}

func TestRemoveUnusedNoSideEffectsTaggedTemplates(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.js": `
// @__NO_SIDE_EFFECTS__
function foo() {}
foo` + "`remove`" + `;
foo` + "`remove${null}`" + `;
foo` + "`remove${123}`" + `;
use(foo` + "`keep`" + `);
foo` + "`remove this part ${keep} and this ${alsoKeep}`" + `;
` + "`remove this part ${keep} and this ${alsoKeep}`" + `;
`,
},
entryPaths: []string{"/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
MinifySyntax: true,
},
})
}

func TestTreeShakingReactElements(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
Expand Down
11 changes: 11 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_dce.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2456,6 +2456,17 @@ TestRemoveUnusedImportsEvalTS
---------- /out.js ----------
eval("foo(a, b, c)");

================================================================================
TestRemoveUnusedNoSideEffectsTaggedTemplates
---------- /out.js ----------
// entry.js
// @__NO_SIDE_EFFECTS__
function foo() {
}
use(foo`keep`);
keep, alsoKeep;
`${keep}${alsoKeep}`;

================================================================================
TestRemoveUnusedPureCommentCalls
---------- /out.js ----------
Expand Down
7 changes: 7 additions & 0 deletions internal/js_ast/js_ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,13 @@ type ETemplate struct {
HeadLoc logger.Loc
LegacyOctalLoc logger.Loc

// True if this is a tagged template literal with a comment that indicates
// this function call can be removed if the result is unused. Note that the
// arguments are not considered to be part of the call. If the call itself
// is removed due to this annotation, the arguments must remain if they have
// side effects (including the string conversions).
CanBeUnwrappedIfUnused bool

// If the tag is present, it is expected to be a function and is called. If
// the tag is a syntactic property access, then the value for "this" in the
// function call is the object whose property was accessed (e.g. in "a.b``"
Expand Down
12 changes: 11 additions & 1 deletion internal/js_ast/js_ast_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,16 @@ func (ctx HelperContext) SimplifyUnusedExpr(expr Expr, unsupportedFeatures compa
comma = JoinWithComma(comma, Expr{Loc: templateLoc, Data: template})
}
return comma
} else if e.CanBeUnwrappedIfUnused {
// If the function call was annotated as being able to be removed if the
// result is unused, then we can remove it and just keep the arguments.
// Note that there are no implicit "ToString" operations for tagged
// template literals.
var comma Expr
for _, part := range e.Parts {
comma = JoinWithComma(comma, ctx.SimplifyUnusedExpr(part.Value, unsupportedFeatures))
}
return comma
}

case *EArray:
Expand Down Expand Up @@ -2413,7 +2423,7 @@ func (ctx HelperContext) ExprCanBeRemovedIfUnused(expr Expr) bool {
// A template can be removed if it has no tag and every value has no side
// effects and results in some kind of primitive, since all primitives
// have a "ToString" operation with no side effects.
if e.TagOrNil.Data == nil {
if e.TagOrNil.Data == nil || e.CanBeUnwrappedIfUnused {
for _, part := range e.Parts {
if !ctx.ExprCanBeRemovedIfUnused(part.Value) || KnownPrimitiveType(part.Value.Data) == PrimitiveUnknown {
return false
Expand Down
5 changes: 5 additions & 0 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -13237,6 +13237,11 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
tagThisFunc = tagOut.thisArgFunc
tagWrapFunc = tagOut.thisArgWrapFunc

// Copy the call side effect flag over if this is a known target
if id, ok := tag.Data.(*js_ast.EIdentifier); ok && p.symbols[id.Ref.InnerIndex].Flags.Has(ast.CallCanBeUnwrappedIfUnused) {
e.CanBeUnwrappedIfUnused = true
}

// The value of "this" must be manually preserved for private member
// accesses inside template tag expressions such as "this.#foo``".
// The private member "this.#foo" must see the value of "this".
Expand Down

0 comments on commit d968af2

Please sign in to comment.