Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

Zone.js conflicts with Bluebird #455

Closed
lorenzodallavecchia opened this issue Sep 19, 2016 · 15 comments · Fixed by #655
Closed

Zone.js conflicts with Bluebird #455

lorenzodallavecchia opened this issue Sep 19, 2016 · 15 comments · Fixed by #655

Comments

@lorenzodallavecchia
Copy link

lorenzodallavecchia commented Sep 19, 2016

I'm including Zone 0.6.21 in a project that also uses Bluebird as one of the first steps for using Angular 2. Unfortunately, there does not seem to be a clean way for having the two libraries work together.

Bluebird is more than a "Promise shim", since it replaces the Promise implementation entirely, with a more performant one, while also adding many utilities both to the Promise constructor and its prototype.

  • If Bluebird is included before Zone (which I understand is the preferred way), Zone will overwrite the entire Promise constructor, loosing all utilities and the internal implementation of Bluebird. This basically defeats the purpose of including Bluebird in the first place.
  • If Bluebird is included after Zone and is made Zone-aware by replacing its scheduling function then everything works: Bluebird is active and zones work correctly. However the Zone.assertZonePatched that Angular 2 calls will fail, since the Promise constructor is no longer the one from Zone.

Curently I can work around the problem by including Bluebird after Zone and then manually patching each function in the Bluebird API.

Promise.prototype.then = (function(_orig) {
  return function(fulfilledHandler, rejectedHandler) {
    return _orig.call(this, zonify(fulfilledHandler), zonify(rejectedHandler));
  }
})(Promise.prototype.then);

// similar patches for other 25+ functions

// hack to keep Angular happy
Zone.assertZonePatched = function() {};

If you are interested, this Gist shows the full code.

As you can see it is pretty hacky and I would better go without it if it was possible.

Is there a particular reason for Zone introducing an entire Promise constructor instead of just wrapping then, catch and other methods?

@jvbianchi
Copy link

this also causes problems when you use sequelize (which uses bluebird) with angular universal.

@lorenzodallavecchia
Copy link
Author

lorenzodallavecchia commented Oct 6, 2016

Unfortunately, it is not enough to call Bluebird setScheduler as this may cause promises to be settled in the wrong zone (a extremely subtle bug). It looks like the only way is to patch each and every function in the API. I updated the opening comment to reflect this.

@thejoecode
Copy link

I was caught off-guard by this. I wanted to include Angular 2 on a page already utilizing some Bluebird methods. My options are to put Angular in an iFrame or rewrite the Bluebird promises. Neither are great.

@christophmegusta
Copy link

Is there any solution yet to this problem? We are suffering the same fate as we are trying to use ngUpgrade with a existing ng1 application which is using Bluebird.

@andreasrueedlinger
Copy link

@lorenzodallavecchia How do you apply your patch in a SystemJS environment? Could you help me with that?

@lorenzodallavecchia
Copy link
Author

@andreasrueedlinger The project that I am migrating from Angular 1 does not use any module loader yet: it is just including the angular UMD bundles as browser globals (script tags) so it is enough to include the patch after the Zone UMD bundle but before Angular's.

Eventually I am planning to adopt a module loader (still undecided between System.js and WebPack).
A possibility for System.js would be creating a plugin that uses the exports.translate hook to append the patch code after the loaded module code and the associate that plugin with Zone.js using a meta block configuration in System.js.
System.js - Plugin Loaders
System.js - Creating a Plugin

@andreasrueedlinger
Copy link

@lorenzodallavecchia Thanks for your help. It seems to be a bit more complex in my case! I am using electron with sequelize and angular2. For angular I am using systemjs but for the native modules I have to load it using the native module loader. The problem is now that sequelize does not use the global Promise which could be patched but imports the bluebird Promise internally and applies some more shims on it.
The only solution I see right now is to patch either bluebird or sequelize to get zone aware promises in sequelize. Or does anyone know a solution to fool the native module loader to load a patched version of bluebird?

@JiaLiPassion
Copy link
Collaborator

@andreasrueedlinger you can try this one, we use sequelize too, and we use this code to make zone work with sequelize.

sequelize zone patch

The newest comment is the working code.

@andreasrueedlinger
Copy link

Nice, thanks!

@JiaLiPassion
Copy link
Collaborator

I'll make a pull request to make a bluebird patch just like the sequelize zone patch but without import shimmer.js.

@christophmegusta
Copy link

christophmegusta commented Mar 10, 2017

Hi thanks a lot for the patch, @JiaLiPassion .

Unfortunately with current Zone 0.7.8 it doesnt work for us. The patch is loaded and patching the Bluebird, but we still get the error on startup of our angular2 app.

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.) at Function.Zone.assertZonePatched

We need to execute this line to get rid of it:

Zone.assertZonePatched = function() {};

but is this really the wanted behaviour?
That is how our polyfills file looks (webpack2 browser environment):

import 'core-js/es6';
import 'core-js/es7/reflect';

import 'zone.js/dist/zone';

if(process.env.NODE_ENV != 'production') {
  Error['stackTraceLimit'] = Infinity;
  require('zone.js/dist/long-stack-trace-zone');
}

import Bluebird from 'bluebird';
Bluebird.config({
    warnings: false,
    longStackTraces: false,
    cancellation: true,
    monitoring: false
});

import 'zone.js/dist/zone-bluebird';
Zone[Zone['__symbol__']('bluebird')](Bluebird);
// Zone.assertZonePatched = function() {};

EDIT:
also @lorenzodallavecchia was stating that setting the scheduler, like the zone-bluebird patch currently does, is not enough:

Unfortunately, it is not enough to call Bluebird setScheduler as this may cause promises to be    settled in the wrong zone (a extremely subtle bug). It looks like the only way is to patch each and     every function in the API. I updated the opening comment to reflect this.

@JiaLiPassion
Copy link
Collaborator

@christophmegusta , can you provide a reproduce repo or plunker? I would like to find out why zone.js complain

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.) at Function.Zone.assertZonePatched

And could you tell me more about

Unfortunately, it is not enough to call Bluebird setScheduler as this may cause promises to be settled in the wrong zone (a extremely subtle bug). It looks like the only way is to patch each and every function in the API. I updated the opening comment to reflect this.

a reproduce code or repo will help, thank you.

@lorenzodallavecchia
Copy link
Author

setting the scheduler, like the zone-bluebird patch currently does, is not enough:

I searched the very first fix that I was using, based on setScheduler. It is actually a bit different than the one in #655.

var originalScheduler = Promise.setScheduler(function(fn) {
    originalScheduler(Zone.current.wrap(fn));
});

The idea is to wrap the task callback in the current Zone.

If I recall correctly, the problem was that sometimes the scheduler function is called outside of every zone (in the root zone that is). As a result Zone.current means "root zone", causing the scheduled task to run outside of the Angular zone.

The new fix in #655 is using Zone.current.scheduleMicroTask. While I am not sure about scheduleMicroTask, the fact that it refers to Zone.current makes me suspect that it might suffer from the same issue.

Unfortunately I have not much spare time for building a proper repro. It should probably involve a then callback that delegates its result to the fulfillment of another promise. I think in this situation Bluebird "short circuits" the two promises (for performance) causing the problemating call to the scheduler.

@JiaLiPassion
Copy link
Collaborator

@lorenzodallavecchia , yeah, in my first fix, I patch _then to do the work, maybe that solution will resolve the issue, but first could you give some code sample about how to reproduce the outside(root) zone case?

@FrogTheFrog
Copy link

FrogTheFrog commented Jul 20, 2017

@lorenzodallavecchia, @JiaLiPassion I can't get bluebird to work with Angular too. Fortunately, I had time to make a repo, however it uses Electron as a base, but it shouldn't be a problem. Simply install dependencies and run npm run build followed by npm run start. npm run build:native rebuilds renderer with native Promise and npm run build:bluebird rebuilds renderer with bluebird Promise (injected by webpack). Webpack watcher modes are also provided for faster rebuilding. Finally, I also provided vscode configurations for debugging, in case you are using vscode as your preferred IDE.

With native Promise one should see "Works" in Electron window, whereas with bluebird Promise it should display "Does not work". Console log should have "resolved" in it to prove that promises still work.

Edit: Just to clarify, I want to assign bluebird to global.Promise. bluebird works by itself, but Promise will not work if it is replaced by bluebird. Assigning bluebird to global.Promise (with native build) will throw Error: Zone.js has detected that ZoneAwarePromise (window|global).Promise has been overwritten.

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

Successfully merging a pull request may close this issue.

7 participants