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

BUG: new 'Mangle variables to reduce code size' no longer does ANY minifying/mangling within IIFE #31

Open
DarrenSem opened this issue Mar 3, 2023 · 5 comments

Comments

@DarrenSem
Copy link

With both of these checkboxes selected...
[x] Wrap in an IIFE (anonymizing function).
[x] Mangle variables to reduce code size. (formerly called "Minify code using Babel Minify."

...there seems to be a regressive bug introduced by today's update.

Test case:

  var key = "href";
  var foo = location[key];
  var bar = foo + "#";
  foo += "?";
  console.log(bar);;;

Result from newly-updated version @ https://chriszarate.github.io/bookmarkleter/

javascript:void function () {var key="href",foo=location.href,bar=foo+"#";foo+="?",console.log(bar);}();

Compare to result from previous version e.g. https://alanhogan.github.io/bookmarkleter/

javascript:void function(){var a=location.href,b=a+"#";a+="?",console.log(b)}();
@chriszarate
Copy link
Owner

There's a bit of a catch-22. Babel won't mangle vars in the global scope, because they could be needed in other code. Wrapping the code in a IIFE means they aren't in the global scope, but we have already passed the code through the minifier—because if we don't, we run into syntax issues. I think you were on to something with the line break, but I need to test a few things out.

@DarrenSem
Copy link
Author

My suggestion: remove the latest "comments: false" commit, and then just make ${code}}(); become ${code}\n}(); -- then I can test a few examples to confirm that fixes the minor annoyance WITHOUT completely breaking minifying :-)

@DarrenSem
Copy link
Author

There's a bit of a catch-22. Babel won't mangle vars in the global scope, because they could be needed in other code. Wrapping the code in a IIFE means they aren't in the global scope, but we have already passed the code through the minifier—because if we don't, we run into syntax issues. I think you were on to something with the line break, but I need to test a few things out.

I would like to add a couple of tests so the mangling/minifying can be validated as working with and without ending // (and also with and without its own wrapping IIFE). Should I edit one of the existing files in the /test/ folder or create a new one?

@chriszarate
Copy link
Owner

I've just pushed up some changes that I think put things in a better place, including your original suggestion of adding the line break to the IIFE. Mangling (now linked with the deadcode option) is now the default and should follow the previous behavior.

I agree that more and better tests are needed, I'd be grateful if you wanted to contribute some. Thanks for all the feedback.

@DarrenSem
Copy link
Author

Great! Sooo nice to see you spending time on getting this handiest-ever tool updated with some modern additions to JS. I'm glad my comments have been helpful and not overwhelming or annoying :-)

I saw a test file called "mangleVars.js" so I think that will be what I add to.

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

2 participants