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

Inline values which are used only once #3568

Closed
benmccann opened this issue Dec 28, 2023 · 3 comments
Closed

Inline values which are used only once #3568

benmccann opened this issue Dec 28, 2023 · 3 comments

Comments

@benmccann
Copy link

playgound link

If you have:

let x = false;
let y = x;
const boolean = y;
var frag = $.template(`<p contenteditable="${boolean}">hello world</p>`);
console.log(frag)

When minimizing, it gets compiled to:

let x=!1,y=x;const boolean=y;var frag=$.template(`<p contenteditable="${boolean}">hello world</p>`);console.log(frag);

It could be simplified to:

console.log(`<p contenteditable="false">hello world</p>`);

While this code is written in an obviously dumb way for reproduction purposes, there's lots of places where this is encountered in less dumb ways. E.g. a number of Vite plugins may produce code that could be simplified in this manner. While we could invest effort into individually improving each plugin, those changes aren't necessarily easy, and so it could be a better use of effort to address it just a single time here.

@evanw
Copy link
Owner

evanw commented Dec 29, 2023

If you put it in a function, it will be minified further:

function f() {
  let x = false;
  let y = x;
  const boolean = y;
  var frag = $.template(`<p contenteditable="${boolean}">hello world</p>`);
  console.log(frag)
}

That turns into this:

function f(){const e=!1;var l=$.template(`<p contenteditable="${e}">hello world</p>`);console.log(l)}

One reason that this only happens within a function in this case is because when minifying a string of raw JavaScript, esbuild doesn't assume that top-level variables (i.e. boolean) will not be accessed by code outside of what esbuild is being shown. For example, some frameworks (e.g. Svelte) might use esbuild to transform something and then append extra code at the end and expect the later code to be able to interact with the earlier code. That's why the variable boolean is not minified to a shorter name here.

And one reason that boolean is not inlined even within a function is because that would reorder the ToString operation past the evaluation of $.template, which may not be safe. Either operation might have side effects and reordering side effects is not allowed by an optimizer. It happens to be safe here but esbuild can't assume that and would have to prove that. Details like that make optimizations like this unlikely to happen in complex cases anyway (cases involving non-primitive values).

Edit: I investigated this. It looks like the single-use expression substitution allows reordering a side-effect past a primitive but not a primitive past a side-effect. I think the second part is an oversight. Adding that case makes this work.

@evanw
Copy link
Owner

evanw commented Dec 29, 2023

I think I can get this to turn into the following code:

function f(){let e=$.template('<p contenteditable="false">hello world</p>');console.log(e)}

But reordering $.template past console.log is unsafe because $.template evaluates code which could potentially change the value of console.log, so reordering these two expressions could change the behavior of the code.

@evanw evanw closed this as completed in f5f8ff8 Dec 29, 2023
@benmccann
Copy link
Author

Thank you so much both for the improvement and informative sharing of details!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants
@benmccann @evanw and others