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

Using esbuild minifier breaks code #2139

Closed
surgicaI opened this issue Mar 28, 2022 · 2 comments
Closed

Using esbuild minifier breaks code #2139

surgicaI opened this issue Mar 28, 2022 · 2 comments

Comments

@surgicaI
Copy link

We are using 1.8.38 version of gojs library and the library(which is valid javascript code) breaks after minification with esbuild.

Here is repo that can be used to reproduce the issue. Instructions to build and run code can be found in the README file in the repo.
https://github.com/surgicaI/esbuild-gojs-minify-bug

In the above repo, rollup is used to generate javascript bundle from source code and rollup config can be found here. Gojs library is used to create diagrams and notice that when the generated javascript bundle is minified using minify module from esbuild, the code breaks and no diagram is rendered in the app.

Please look into the issue

@hyrious
Copy link

hyrious commented Mar 29, 2022

This could be caused by minify-syntax. You can turn it off by adding options to the rollup plugin:

minify({
  minify: false,
  minifyWhitespace: true,
  minifyIdentifiers: true,
})

However it is hard to tell what's wrong in the minified code, keepNames does not work too. Searching the source code to see if it uses eval or Function() also does not give me any clue. We have to investigate it more deeply to see if it relies on a function's toString().

Update: I've tried some older versions of esbuild (from 0.10.1, since the rollup plugin specified that version range) and there's no luck.

Update2: I've noticed gojs' latest version 2.2.5 (current 1.8.38), just tried upgrade that package and now it all works. The oldest working version most close to 1.8.38 is 2.0.2.

@evanw
Copy link
Owner

evanw commented Mar 29, 2022

Thanks for the report. I've traced it down to this optimization:

if len(s.Stmts) == 1 && !statementCaresAboutScope(s.Stmts[0]) {
// Unwrap blocks containing a single statement
stmt = s.Stmts[0]

The issue is with if statements of the following form:

if (a)
  b: {
    if (c) {
      break b
    }
  }
else if (d)
  e()

That code is incorrectly minified into this:

if (a)
  b:
    if (c)
      break b;
else
  d && e();

The problem is that the AST printer doesn't add braces to avoid the inner if merging with the outer else when the inner if is the child of a label. I will change esbuild to print the AST this way instead:

if (a) {
  b:
    if (c)
      break b;
} else
  d && e();

@evanw evanw closed this as completed in d8cbaa0 Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants