Skip to content
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

Avoid conflicts between Promise from core-js and Promise from zone.js #608

Closed
wants to merge 1 commit into from

Conversation

marcospont
Copy link

When loaded, zone.js will patch global Promise.
When loaded, core-js will run a detection routine to determine if it needs to patch global Promise.

If core-js is loaded before zone.js, everything works fine.
If core-js is loaded after zone.js and the order of these events cannot be changed, core-js will fail to detect a patched global Promise, causing its detection code to patch it again.

This pull request aims to improve the detection routine to avoid conflicts between core-js Promise polyfill and zone.js Promise polyfill.

@zloirock
Copy link
Owner

zloirock commented Jul 26, 2019

Sorry, I'm not sure how this PR could help in this situation. core-js should be loaded before any other scripts since it's a polyfill. zone.js Promise polyfill is not complete - in the zone.js code I don't see even @@species - so it can't and shouldn't pass core-js feature detection.

@marcospont
Copy link
Author

marcospont commented Jul 26, 2019

I know that core-js should always be loaded first since it is a polyfill. However, in this scenario, the load order cannot be changed.

Application A is written in Angular and loads zone.js. Application A has the ability to load other applications and embed them. One of these applications is Application B, which uses AngularJS transpiled with Babel and core-js.

Assuming that Application A cannot be changed to load core-js before zone.js, the issue cannot be avoided.

In this PR, the code change avoids the conflict by testing if promise.constructor is an object just like it was assigned while creating "FakePromise". In the scenario where the conflict happens, promise.constructor will be a function just like defined by the patched Promise from zone.js.

If zone.js is loaded before core-js, calling "promise.then(empty)" will throw an error because the "then" method implemented in zone.js tries to call "new this.constructor(null)". Since the constructor is not a function, it cannot be called using "new".

@zloirock
Copy link
Owner

So, maybe, you should use this?

@luisrudge
Copy link

This is specially hard for library maintainers. Consumers of libraries expect the final bundle to be working without extra polyfills, so I added this polyfill to the project, but not people using zone.js get this error 😬 I think it's reasonable to check if there's a Promise available before adding the polyfill, no?

@zloirock
Copy link
Owner

zloirock commented Aug 27, 2019

@luisrudge

  • zone.js should be used after core-js. Otherwise, you will get other problems as well.
  • If usage of zone.js before core-js is required for any reason - I mentioned a solution.
  • zone.js promises do not match the spec, so they don't pass core-js feature detection. It should be fixed on zone.js side.
  • This PR definitely is an incorrect fix for this problem since it just breaks core-js feature detection. Since nothing was fixed, I'm closing it.
  • It's a bad idea to add global polyfills to libraries. Try to use a core-js version without global pollution, see core-js-pure / @babel/runtime-corejs3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants