polyfills-es5 doesn't check to see if promise is already added

[willyboy]

🐞 Bug report

Command (mark with an x)

- [ ] new
- [x] build
- [ ] serve
- [ ] test
- [ ] e2e
- [ ] generate
- [ ] add
- [ ] update
- [ ] lint
- [ ] xi18n
- [ ] run
- [ ] config
- [ ] help
- [ ] version
- [ ] doc

Is this a regression?

Yes

Yes, the previous version in which this bug was not present was: ....

7.x

Description

We have a container app that loads Angular apps into it (the architecture is a nightmare).
The code looks like:

$('.container').html('the index.html from angular dist folder');

Prior to Angular 8 and differential loading, you could do this as many times as you wanted - I believe the polyfill that was used checked to see if promise was already polyfilled.
Post Angular 8, the second time you do this, you will see:

VM885:1 Unhandled promise rejection Error: Zone.js has detected that ZoneAwarePromise `(window|global).Promise` has been overwritten.
Most likely cause is that a Promise polyfill has been loaded after Zone.js (Polyfilling Promise api is not necessary when zone.js is loaded. If you must load one, do so before loading zone.js.)

I'm guessing that the polyfills-es5 file assumes that it will only be loaded once and therefore doesn't make the same check to see if it's been polyfilled already.
(as a side note, when using $().html() on a script with "nomodule", the nomodule is ignored so this issue effects all browsers).

🔬 Minimal Reproduction

1. Create a brand new app (I used nx workspaces so: ng g app myapp)
2. Set your tsconfig to es5 not es2015 (we can't go to es2015 because of ES2015 support flex-layout#950)
   NOTE - You can reproduce this with es2015 by removing nomodule but it is simpler to reproduce in es5
3. ng build --prod
4. Since you're probably testing this in Chrome, remove "nomodule" from polyfills-es5 (this is reproducible in IE 11 without removing nomodule)
5. Make a copy of the runtime and polyfills scripts and place them below your original scripts:

<script src="runtime.js"></script>
<script src="polyfills-es5.js"></script>
<script src="polyfills.js"></script>
<script src="main.js"></script>
<script src="runtime.js"></script>
<script src="polyfills-es5.js"></script>

6. Get zoneaware promise error

🔥 Exception or Error

VM885:1 Unhandled promise rejection Error: Zone.js has detected that ZoneAwarePromise `(window|global).Promise` has been overwritten.
Most likely cause is that a Promise polyfill has been loaded after Zone.js (Polyfilling Promise api is not necessary when zone.js is loaded. If you must load one, do so before loading zone.js.)

🌍 Your Environment

Angular CLI: 8.0.4
Node: 12.4.0
OS: darwin x64
Angular: 8.0.2
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.800.1
@angular-devkit/build-angular     0.800.4
@angular-devkit/build-optimizer   0.800.4
@angular-devkit/build-webpack     0.800.4
@angular-devkit/core              8.0.1
@angular-devkit/schematics        8.0.1
@angular/cdk                      8.0.1
@angular/cli                      8.0.4
@angular/flex-layout              8.0.0-beta.26
@ngtools/webpack                  8.0.4
@schematics/angular               8.0.1
@schematics/update                0.800.4
rxjs                              6.5.2
typescript                        3.4.5
webpack                           4.30.0

Anything else relevant?

[alan-agius4]

Hi, I am sorry but I am not understanding the issue here.

It is expected that if you have polyfills.js twice you will get a runtime error because you cannot import zone.js twice. This was the case in previous versions of Angular as well, were zone.js couldn't be imported more than once.

[willyboy]

@alan-agius4 We conditionally load zone.js.

if (!(window as any).Zone) {
  require('zone.js/dist/zone');
}

With differential loading, this no longer prevents that error from showing up.

[willyboy]

I believe this issue is related: #14618

Also, we are currently fixing this with --es5BrowserSupport=false

[willyboy]

@alan-agius4 I updated the reproduction to remove your confusion. polyfills-es5 doesn't include zone and that's the only one you need to load again to trigger the error.

[alan-agius4]

The problem here is that when you import polyfills-es5 the first time, is promise will get polyfilled and zone will patch it, the second time you import this, the promise will get polyfilled again, which is not allowed to happen after zone has patched it.

I’d say your use case is quite an edge case that users would import polyfills multiple times since this also introduces performance hits, such as extra code is downloaded and evaluated.

My recommendation would be only to include the polyfills one time. This would fix the issue and also make your application more performance.

I am assuming that the above error is being thrown because of potential bugs that may be caused when the polyfill is loaded after zone. Chiming in @JiaLiPassion since he is the zone.js guru.

[JiaLiPassion]

@alan-agius4 , thanks for digging into this issue, and @willyboy I will check this issue.

[eplatzek]

@JiaLiPassion @willyboy
Any update on this? I just encountered it after an upgrade.

[JiaLiPassion]

@willyboy, @eplatzek , I tried with your reproduce step, but can not reproduce, could you give me a reproduce repo? Thanks.

[alan-agius4]

Closing this issue as no further information has been provided by author.

If the problem persists, please open a new issue, provide a simple repository reproducing the problem, and describe the difference between the expected and current behavior.