core-js es6.promise conflicts with ZoneAwarePromise

[hmdhk]

Importing core-js/shim after zone.js would cause certain properties of ZoneAwarePromise to be overwritten. Eventually it will end up in the wrong zone.

Example:

import 'zone.js';
import 'core-js/shim';

window.Zone.current.fork({name:'new zone'}).run(function (){
    Promise.resolve().then(resp => {
      console.log(window.Zone.current.name) // it's <root> zone!
    })
})

It seems like importing core-js/shim first would resolve the issue. But I'm not sure about side effects of importing zone.js after core-js/shim.

Could this issue be fixed regardless of importing order?

[hmdhk]

More investigation:

Turns out es6.promise module tries to see if it should use native api or not but gets it "wrong" when zone.js is present.

This piece of code:

var USE_NATIVE = !!function(){
  try {
    // correct subclassing with @@species support
    var promise     = $Promise.resolve(1)
      , FakePromise = (promise.constructor = {})[require('./_wks')('species')] = function(exec){ exec(empty, empty); };
    // unhandled rejections tracking support, NodeJS Promise without it fails @@species test
    return (isNode || typeof PromiseRejectionEvent == 'function') && promise.then(empty) instanceof FakePromise;
  } catch(e){ /* empty */ }
}();

Reference: https://github.com/zloirock/core-js/blob/master/modules/es6.promise.js#L22

USE_NATIVE is false with zone.js in the page and that's why it patches Promise properties like resolve.

I think zone.js should behave in a way that it can not be distinguished from native api.

[JiaLiPassion]

@jahtalab , all modules which provide promise polyfill or implementation should be loaded before zone.js, is there any reason that you want it be loaded after zone.js?

[hmdhk]

@JiaLiPassion , Thanks for your response.
Generally I think it's much simpler to explain to users that "zone.js should be loaded before any other library to avoid unpredictable results". Over time the number of libraries grow and so does the number of exceptions to that general rule (there are already three exceptions in the readme). This makes including libraries complicated.

Furthermore core-js provides a lot of different polyfills and they are all usually loaded together. while the promise polyfill is advised to be loaded before zone.js the rest of them could potentially have conflicts with zone.js if they are loaded before zone.js.

Also in this case we might be able to find a way to make zone.js act like "Native" api.

[JiaLiPassion]

@jahtalab , in my understanding, currently zone.js will be loaded after other polyfills because it will make sure that the async operations or event handler are in zone.

And I agree with you, that zone.js should act like Native API.

Can you provide more details about what conflicts that will occur when load core-js before zone.js?

[michaelbazos]

@JiaLiPassion

While loading core-js before zone.js would get rid of the "Zone.js has detected that ZoneAwarePromise (window|global).Promise has been overwritten" error, it is actually more a workaround than a solution.

There are cases when we simply can't load core-js before zone. Lazy-loaded modules is such a case.

As it is currently, I believe, a rather critical issue, I raised the point to the team of core-js project, specifically regarding why ZoneAwarePromise is not considered compliant and therefore is overriden.
Here you can read their response: core-js #319.

[JiaLiPassion]

@michaelbazos , sure, I will check it!

[tytskyi]

@michaelbazos this issue has been discovered long time ago in angular community.
Can't you include only those polyfills that actually required?
ZoneAwarePromise already works like polyfill.

import 'core-js/es6/symbol';
import 'core-js/es6/object';
import 'core-js/es6/function';
import 'core-js/es6/parse-int';
import 'core-js/es6/parse-float';
import 'core-js/es6/number';
import 'core-js/es6/math';
import 'core-js/es6/string';
import 'core-js/es6/date';
import 'core-js/es6/array';
import 'core-js/es6/regexp';
import 'core-js/es6/map';
import 'core-js/es6/weak-map';
import 'core-js/es6/set';
// etc

[JiaLiPassion]

@tytskyi , yeah, I agree with you. ZoneAwarePromise is already a full functional Promise polyfill, so @michaelbazos, you don't need es6/promise any more.

[michaelbazos]

@tytskyi @JiaLiPassion

You seem to miss the point of the issue. I indeed don't need core-js/es6/promise and I'm not loading it per se.

Let me give you a scenario:

• Just bootstrap an angular application from angular-cli. At this point you depend on zone.js, and core-js/es6/promise is not imported.
• Now, import any library which depends on core-js/es6/promise. Bing maps is such a library, but there are plenty others.

And that's it. The application is broken because ZoneAwarePromise is not spec-compliant and will be replaced. So, what do we do? Loading zone after the vendor library, so as to fool the compliance check by core-js? That would work indeed. But what if my maps module is lazy-loaded? Then zone would be loaded before core-js/es6/promise. Surely, we can fool the check by caching ZoneAwarePromise, switching to a spec-compliant implementation, load the module, then switching back to ZoneAwarePromise 🙃.
But, are we really solving the issue? Can't we do better, see what ZoneAwarePromise is missing to be considered compliant by core-js and left untouched? I believe avoiding this hassle with the import order would be a nice achievement.

[JiaLiPassion]

@michaelbazos , I see. thank you for the detail information.
The scenario is

1. load ZoneAwarePromise, now window.Promise = ZoneAwarePromise
2. load es6/promise by importing other libraries such as bing maps. global.Promise=Es6Promise.
3. Angular failed to work.

I am not sure the detail about es6-promise implementation, but angular need ZoneAwarePromise to make ngZone work, so after step2, we have to replace the promise
to ZoneAwarePromise again. If my understanding is correct, I don't know how zone.js can do it automatically.

[DavidKDeutsch]

Just to add my 2 cents: I'm pretty new to the "new" Angular (I have an existing angular.js app that I am setting up to run in hybrid mode while I upgrade it), and zone.js has been giving me fits with this kind of thing; I ran into this problem with core-js, and am now encountering it again due to some widgets that I load on-demand overwriting ZoneAwarePromise with their own polyfills.

In fact, the overall monkey-patching approach of zone.js has caused me problems. For example, there was a template that used interpolation to display an object as JSON, but with zone.js loaded angular.js instead called the object's toString() method, displaying '[Object object]' (or whatever). Turns out that angular.js determines if an object has a custom toString() method by comparing it to a snapshot of the native toString at load time, and since zone.js monkey-patched the method after that snapshot was taken, the comparison didn't work.

This all seems very different from the angular.js philosophy. For example, angular.js does not modify the native timeout() to trigger a digest, it provides the $timeout service which must be called explicitly. This is more me venting than anything else, but it just seems like a very brittle setup; when I loaded zone.js "too late," I got the interpolation error, and when I loaded it "too early," I got the "ZoneAwarePromise has been overwritten" error.

Update: Just to mitigate my whining a bit, it looks like a dependency of a dependency (jsPDF -> html2canvas) of my project uses an ancient version of es6-promise (from over 3 years ago) that polyfills Promise if Promise.cast is not defined (took a bit of spelunking through GitHub to find that change). Whoops.

[michaelbazos]

"we have to replace the promise to ZoneAwarePromise again"
@JiaLiPassion I believe this is not the correct approach to the problem. I think it's better to make the implementation of ZoneAwarePromise comply with the ecma specification of Promises.
Based on this answer on core-js repository, the implementation of ZoneAwarePromise.prototype.then is missing one requirement: chainPromise.constructor[Symbol.species] should returns a promise constructor. At present it returns undefined. This is described in the point 3 of the specification.

By adding couple lines for that missing requirement I can confirm that core-js, or es6-shim, are considering ZoneAwarePromise spec-compliant, and they don't replace them. Therefore the loading order doesn't matter.

[JiaLiPassion]

@michaelbazos, thank you for the detail information, I will try it.