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

Update minifier for core code #79433

Closed
chrmarti opened this issue Aug 19, 2019 · 6 comments
Closed

Update minifier for core code #79433

chrmarti opened this issue Aug 19, 2019 · 6 comments
Assignees
Labels
debt Code quality issues engineering VS Code - Build / issue tracking / etc.

Comments

@chrmarti
Copy link
Contributor

We have seen two instances of a bug in uglify-es (#79044, #74321) that appears to be fixed in the latest version of the minifier. Updating to that caused some issues and the project seems to be abandoned atm (1.5 years old release), so I didn't invest additional time.

We need to look into updating to an actively maintained minifier supporting ES6.

/cc @joaomoreno @bpasero

@chrmarti chrmarti added bug Issue identified by VS Code Team member as probable bug engineering VS Code - Build / issue tracking / etc. labels Aug 19, 2019
@bpasero bpasero added debt Code quality issues and removed bug Issue identified by VS Code Team member as probable bug labels Aug 19, 2019
@bpasero
Copy link
Member

bpasero commented Aug 19, 2019

I am adding you @chrmarti because you did this update already and have learned what the consequences where (I think with some language definitions) that you should share here.

I personally feel we should jump to a minifier that is active and maintained, so there is a bit of research required here. So far I heard of (not necessarely active anymore): uglify-es, uglify-js and terser.

@bpasero bpasero added this to the September 2019 milestone Aug 19, 2019
@joaomoreno
Copy link
Member

Jumping out of this since my engineering plate is as full as my actual plate at a Brazilian BBQ.

@chrmarti
Copy link
Contributor Author

chrmarti commented Sep 2, 2019

Might make sense to use Terser like we do for extensions. That way we would get consistent behavior for all our code.

Article comparing minifiers: https://blog.logrocket.com/uglify-vs-babel-minify-vs-terser-a-mini-battle-royale/.

@bpasero
Copy link
Member

bpasero commented Sep 2, 2019

@chrmarti I have a branch up for using Terser: https://github.com/microsoft/vscode/compare/ben/terser

To workaround the issue we noticed when you tried to go with latest ugilify-es I had to set the compress option hoist_funs: true which is the same what our current version is doing. I do not fully understand why there is the bug when not using it.

Maybe you can verify that your minification issue is resolved with terser.

@bpasero
Copy link
Member

bpasero commented Sep 2, 2019

via #80201

@bpasero bpasero closed this as completed Sep 2, 2019
@bpasero
Copy link
Member

bpasero commented Sep 2, 2019

I am running a build with the new minifier and with restoring await import (89f6794) to see if we can use this pattern now.

@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues engineering VS Code - Build / issue tracking / etc.
Projects
None yet
Development

No branches or pull requests

3 participants