-
Notifications
You must be signed in to change notification settings - Fork 338
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
Enable UglifyJS compatibility workarounds #3137
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
b60dac3
to
2eeebd2
Compare
b194113
to
7a04f3f
Compare
fa7ff88
to
3ec98ff
Compare
3ec98ff
to
70b049e
Compare
63d5a69
to
e010278
Compare
e010278
to
dfcb99a
Compare
dfcb99a
to
8ddb9c7
Compare
0aace99
to
80d9bff
Compare
cd8c444
to
055671d
Compare
a8a6c7a
to
935b242
Compare
Moved to: |
935b242
to
cf79eeb
Compare
4f1d80e
to
e937fcb
Compare
cf79eeb
to
0cbd398
Compare
36degrees
approved these changes
Jan 12, 2023
We’re seeing the `addEventListener` polyfill not applied in IE8 when UglifyJS `inline` optimisations above level 2 (no variables or statements) are added https://github.com/mishoo/UglifyJS#compress-options
0cbd398
to
04a6669
Compare
@36degrees I'm going to merge this in as initial testing looks good Won't focus on it as we're nearly set to replace it with Terser once testing is complete: |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
🐛 bug
Something isn't working the way it should (including incorrect wording in documentation)
javascript
tooling
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR lowers the threshold for function calls being "inlined" in UglifyJS to fix:
dist
errors in IE8 #3136Running our minified bundle in IE8 sees the Event polyfill fail to patch
Element.prototype.addEventListener
due to polyfill code being "inlined" with unexpected side effects.Various unpublished
inline
improvements been made since the lastuglify-js@3.17.4
release:inline
&unused
mishoo/UglifyJS#5725inline
mishoo/UglifyJS#5736inline
mishoo/UglifyJS#5742inline
mishoo/UglifyJS#5760functions
&inline
mishoo/UglifyJS#5767These might fix it, but for now we can disable inlining functions with variables and statements
Before
After
Also included are various flags to opt out of optimisations that can break older browsers
UglifyJS compress options
https://github.com/mishoo/UglifyJS#compress-options
inline
(default:true
) — inline calls to function with simple/return statement: