fix(@angular-devkit/build-angular): re-introduce pure_getters

[cexbrayat]

PR #15607 removed the _pure_getters optimization as it no longer has effect on the framework size.
But it does have an effect on third party library.

For example a simple project using @ng-bootstrap/ng-bootstrap goes from 444K uncompressed with CLI 9.0.0-next.5 to 488K with CLI 9.0.0-next.6 which introduced the removal.

[vikerman]

Thanks! We decided to accept this PR as soon as someone from the CLI team approves it

[kzc]

I've got no dog in this hunt, but I've been observing this angular-cli pure_getters back and forth from the sidelines. The reason why ng-bootstrap retains the extra 44K of code is due to it using an old version of rxjs rather than the pure_getters=false optimized version rxjs 6.5.3.

While pure_getters=true does make some applications with third party libraries smaller, it breaks other libraries relying on getters with side effects. Being more cautious and prioritizing correctness over smaller code size would seem to be the more prudent approach. Or default to pure_getters=false and have a CLI opt-in for users wishing to use pure_getters=true or to set any other minify option for that matter.

/cc @filipesilva @alan-agius4

[filipesilva]

@cexbrayat can you try following @kzc suggestion of using a [email protected] please? We had to update it all around as well because that's the version that doesn't introduce code that needs pure_getters.

@kzc we were thinking of having different optimization levels to account for the projects that want to enable/disable pure_getters. I've opened #15761 to track it.

[cexbrayat]

@kcz Thank you for your input. I completely understand. Currently it just introduces a too big size regression across apps tested either internally at Google or externally (@kara is working on size tracking and also spotted a regression, and we think this is it).

I can only talk for my apps, but we are on RxJS 6.5.3. The problem is that transitive deps might not be. These are numbers from our CI (CLI next.7 has the regression, CLI next.8 has pure_getter again) :

medium app using ng-bootstrap
next.7 488K (144K gzip)
next.8 416K (124K gzip)

As v9 is focusing on Ivy release, and fwk team is doing its best to have good size numbers (on par or lower than VE), I think it would be nice to keep pure_getter until the release and study if we can remove after. It makes our investigations hard to do when we are studying Ivy bundle sizes if the CLI changes behavior (see last fwk meeting for those who have access). That's just my 2 cents, @kara has a more informed opinion.

[cexbrayat]

But as @kzc pointed out, an option to manually enable/disable it would be nice

[filipesilva]

The regression using @angular-devkit/[email protected] might have also been related to #15749 (comment), which was fixed in 0.900.0-next.8. It affected all angular/angular integration tests. So that's also a source of potential confusion.

[cexbrayat]

@filipesilva I don't think so. In my particluar example I had the same difference by using cli next.7 and next.7 manually patched with this PR (so not including the fix you mention). And the regression appeared with cli next.6, the first to remove pure_getter. And it also only appears when adding ng-bootstrap, so I'm fairly confident that, for my example app, this was the issue.

[vikerman]

@kara The Angular framework team also requested this be reverted. This needs many different library considerations before changing.

Currently we just went back to the option of "what we always had", so that there are no regressive effects from this change. We have to figure out the plan forward involving everyone.

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