Production build changes app behavior (some required code is removed from the bundle)

[t-denis]

🐞 bug report

Affected Package

The issue is caused by package @angular/....

Is this a regression?

Not sure

Description

Leaflet library contains the following code:
https://github.com/Leaflet/Leaflet/blob/984fedda1c48d141f018ca45ae06738872d7f5dd/src/layer/tile/GridLayer.js#L393-L394
Looks like it's a kind of hack that forces a browser to update a page layout and to start the animation (css transition).
https://stackoverflow.com/a/24195559/1989716

In dev build everything works as expected.
When using ng build --prod mentioned above lines are removed from the bundle:
this._setZoomTransform(i,r.getCenter(),r.getZoom()),this._onCreateLevel(i))
so the animation is broken.

Leaflet author closed the issue:
Leaflet/Leaflet#7096

Since angular and leaflet are both very popular, it would be great to provide a compatibility.

🌍 Your Environment

Angular Version:

Angular CLI: 9.1.1
Node: 10.16.0
OS: win32 x64

Angular: 9.1.2
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.901.1
@angular-devkit/build-angular     0.901.1
@angular-devkit/build-optimizer   0.901.1
@angular-devkit/build-webpack     0.901.1
@angular-devkit/core              9.1.1
@angular-devkit/schematics        9.1.1
@angular/cli                      9.1.1
@ngtools/webpack                  9.1.1
@schematics/angular               9.1.1
@schematics/update                0.901.1
rxjs                              6.5.4
typescript                        3.7.5
webpack                           4.42.0

Anything else relevant?

[alan-agius4]

Hi @t-denis,

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.

[t-denis]

Hi @alan-agius4
Made a minimal repro. There is a map with a single layer, nothing else.
https://github.com/t-denis/angular-leaflet

Please compare zoom animation in dev and prod builds.

[KevinCathcart]

This is caused by the terser pure_getters option, which angular sets, since it is usually safe, and has a significant impact on the minified code size.

Terser is able to determine that 'Util.falseFn()' of leaflet is a pure function. It therefore knows it is safe to remove calls to it if the result is not used. It also knows that it needs to evaluate any arguments passed to said function, in case those have side effects.

However, the Util.falseFn(level.el.offsetWidth); argument is a property access, and pure_getters causes terser to assume that property accesses do not have side effects. Unfortunately, the leaflet library is using the 'offsetWidth' for its side effect: forcing a page reflow.

Terser does not currently have any way to mark such impure property accesses as having side effects. See terser/terser#464.

A workaround for the time being is to fork leaflet, and modify that line to read:
level.el.getBoundingClientRect(). That has the same side effect, but is a function call rather than a property access, so terser will not remove it. Alternatively, perhaps you can convince the leaflet maintainers to make that change, since it is a cleaner way to ensure the side effect occurs. (The whole use of Util.falseFn, is clearly already an attempt to ensure minifiers don't remove this property access).

[alan-agius4]

Thanks @KevinCathcart, for your great explanation.

At this point there is not much we can do about this.

Let’s continue tracking this issue here: #15761

[KevinCathcart]

Agreed. Nothing more can be done on the Angular side, unless Angular enables an option to turn off pure_getters (as suggested in the linked issue), or some future version of terser implements some new feature to whitelist getters with side-effects. (In which case, Angular could update to that version, and enable said new feature).

[t-denis]

Thanks!

A workaround for the time being is to fork leaflet, and modify that line to read:
level.el.getBoundingClientRect().

For those, who use angular & leaflet.
Same hack is used in many places, for example you can find it in marker-cluster plugin.
So it's safier not to modify this specific falseFn call, but to add side-effects to falseFn.

[angular-automatic-lock-bot]

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.}