Skip to content

BuildOptimizer breaks 3rd party library #12096

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

Closed
alexeagle opened this issue Sep 4, 2018 · 7 comments
Closed

BuildOptimizer breaks 3rd party library #12096

alexeagle opened this issue Sep 4, 2018 · 7 comments
Labels
freq1: low Only reported by a handful of users who observe it rarely severity3: broken type: bug/fix
Milestone

Comments

@alexeagle
Copy link
Contributor

From @Enngage on May 18, 2018 8:17

Bug Report or Feature Request (mark with an x)

- [x ] bug report -> please search issues before submitting
- [ ] feature request

Area

- [x] devkit
- [ ] schematics

Versions

npm 5.6.0
node v8.9.4
Windows (10

Repro steps

I have this simple angular library repo where I use masonry js library. Everything works perfectly fine (demo) as long as buildOptimizer is disabled:

"buildOptimizer": false

However, as soon as I enable it, some animations (not all!) are broken. I'm pretty confident this is not the issue in the masonry js library, rather it seems like build optimizer might be tampering with the code in some way.

Is there a way to 'disable' build optimizer for this library only? Or could this be part of a bigger problem somewhere?

The log given by the failure

No errors/warnings logged anywhere.

Desired functionality

Don't break code of external libraries.

Mention any other details that might be useful

Angular.json

Copied from original issue: angular/devkit#937

@alexeagle alexeagle added comp: angular-devkit/build-optimizer freq1: low Only reported by a handful of users who observe it rarely severity3: broken labels Sep 4, 2018
@alexeagle
Copy link
Contributor Author

From @filipesilva on May 18, 2018 13:57

This seems like a bug but we'll need to look at a reproduction to find and fix the problem. Can you setup a minimal repro please?

You can read here why this is needed. A good way to make a minimal repro is to create a new app via ng new repro-app and adding the minimum possible code to show the problem. Then you can push this repository to github and link it here.

@alexeagle
Copy link
Contributor Author

From @Enngage on May 18, 2018 14:39

Thanks for the response! Sure, I understand - this is the minimal code I could come up with (repo generated with ng new).

To reproduce:

npm i
ng serve --prod

You should see new image appear with no animation each second. When you use either ng --serve or disable the buildOptimizer in angular.json you will see each image nicely appear as expected.

Let me know if you need anything else.

@alexeagle
Copy link
Contributor Author

From @clydin on May 21, 2018 16:7

This is not actually due to the build optimizer itself but rather a minification setting that is enabled when the build optimizer is enabled. Specifically the pure_getters option of UglifyES (here). Either masonry or one of its dependencies is using a property getter that contains side effects; or there is a defect within UglifyES that is improperly removing code).

@alexeagle
Copy link
Contributor Author

From @Enngage on May 26, 2018 7:58

Forgive my ignorance, but I wasn't able to find exactly what the pure_getters actually mean in real world. How would you identify such 'invalid' pure getter?

Isn't there a possibility of excluding libraries from the build optimizer? I think it might make sense for you to be able to specify if some library should be skipped so that you can still leverage build optimizer for the rest of your code.

@michaelbaur
Copy link

I guess this is the same problem with pure_getters: true that is discussed in issue 11439, too.

@ngbot ngbot bot modified the milestones: needsTriage, Backlog Sep 7, 2018
skuligowski added a commit to skuligowski/image-gallery that referenced this issue Mar 3, 2019
filipesilva added a commit to filipesilva/angular-cli that referenced this issue Apr 16, 2019
When we first started using Build Optimizer, we saw a lot of the savings were tied to using the Uglify/Terser `pure_getters` option.

This was intimately related with the structure and shape of the Angular codebase. The measurements we did at the time on angular.io showed a significant size reduction, from 1mb to about 600kb. Of these roughly 150kb were tied to using `pure_getters` if I remember correctly.

Meanwhile the Angular codebase has changed significantly and I don't really see these savings anymore, so I don't think it makes sense to keep it on given that it is known to cause problems with some libraries.

Related to angular#9231, angular#11439, angular#12096.
filipesilva added a commit to filipesilva/angular-cli that referenced this issue Apr 22, 2019
When we first started using Build Optimizer, we saw a lot of the savings were tied to using the Uglify/Terser `pure_getters` option.

This was intimately related with the structure and shape of the Angular codebase. The measurements we did at the time on angular.io showed a significant size reduction, from 1mb to about 600kb. Of these roughly 150kb were tied to using `pure_getters` if I remember correctly.

Meanwhile the Angular codebase has changed significantly and I don't really see these savings anymore, so I don't think it makes sense to keep it on given that it is known to cause problems with some libraries.

Closes angular#9231, angular#11439, angular#12096, angular#12128.
filipesilva added a commit to filipesilva/angular-cli that referenced this issue Apr 24, 2019
When we first started using Build Optimizer, we saw a lot of the savings were tied to using the Uglify/Terser `pure_getters` option.

This was intimately related with the structure and shape of the Angular codebase. The measurements we did at the time on angular.io showed a significant size reduction, from 1mb to about 600kb. Of these roughly 150kb were tied to using `pure_getters` if I remember correctly.

Meanwhile the Angular codebase has changed significantly and I don't really see these savings anymore, so I don't think it makes sense to keep it on given that it is known to cause problems with some libraries.

Closes angular#9231, angular#11439, angular#12096, angular#12128.
alexeagle pushed a commit that referenced this issue Apr 24, 2019
When we first started using Build Optimizer, we saw a lot of the savings were tied to using the Uglify/Terser `pure_getters` option.

This was intimately related with the structure and shape of the Angular codebase. The measurements we did at the time on angular.io showed a significant size reduction, from 1mb to about 600kb. Of these roughly 150kb were tied to using `pure_getters` if I remember correctly.

Meanwhile the Angular codebase has changed significantly and I don't really see these savings anymore, so I don't think it makes sense to keep it on given that it is known to cause problems with some libraries.

Closes #9231, #11439, #12096, #12128.
@filipesilva
Copy link
Contributor

Closed via #14187

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
freq1: low Only reported by a handful of users who observe it rarely severity3: broken type: bug/fix
Projects
None yet
Development

No branches or pull requests

3 participants