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

child-observation is broken when Zone.js is present on page and wraps MutationObserver #664

Open
genadis opened this issue May 25, 2019 · 5 comments

Comments

@genadis
Copy link

genadis commented May 25, 2019

I'm submitting a bug report

  • Library Version:
    master

  • Browser:
    all

Current behavior:

As can be seen here:
https://github.com/aurelia/templating/blob/master/src/child-observation.js#L160

MutationObserver returned by DOM.createMutationObserver(onChildChange); is being patched, by adding observer.binders = [];

Later it is expected here:
https://github.com/aurelia/templating/blob/master/src/child-observation.js#L67
That the callback's observer contains the binders array.

This is not always the case, and error of the type "Uncaught TypeError: Cannot read property 'length' of undefined" are thrown.

In my use case, aurelia is used as an application inside an Angular application which uses Zone.js. Zone.js wraps the original MutationObserver. So when new MutationObserver() is called a wrapped instance by Zone.js is returned. To this instance .binders is added by aurelia. But in onChildChange() the observer is a different constructor (the browsers original) which does not contain the binders causing the errors.

Expected/desired behavior:

All should work event if window.MutationObserver is not the native browsers constructor.
child observation should not rely on monkey patching window.MutationObserver.

@EisenbergEffect
Copy link
Contributor

EisenbergEffect commented May 25, 2019

Well, Zones is a pretty nasty hack to begin with. I'm not sure how much we can do. The standard way to associate data is either to add a property like we have already, or use a Map/WeakMap. But Map/WeakMap will have the same issue if the instance is actually different since the key equality check will fail. Does Zones provide an API to unwrap the instance and get the original? That would probably enable a solution.

@genadis
Copy link
Author

genadis commented May 25, 2019

Thank you for the quick response!

@EisenbergEffect I totally agree with you regarding Zones, In our case we unfortunately do not have other choice.

Regarding unwrapping the instance, I believe: window[window.Zone.__symbol__("MutationObserver")] should do the trick based on:
angular/angular#26948
window[window.Zone.__symbol__("WebKitMutationObserver")] works for webkit as well.

So if a fix could be applied to https://github.com/aurelia/pal-browser/blob/master/src/dom.js#L41
It would be great!

@genadis genadis changed the title child-observation depends on monkey patching MutationObserver child-observation is broken when Zone.js is present on page and wraps MutationObserver May 25, 2019
@bigopon
Copy link
Member

bigopon commented May 26, 2019

@genadis If you already know what could be applied to fix Zone, why couldn't you do this:

// main.ts
import { DOM } from 'aurelia-framework';

DOM.createMutationObserver = (...args) => {
  return new window[window.Zone.__symbol__("MutationObserver")](...args);
}

@genadis
Copy link
Author

genadis commented May 26, 2019

@bigopon thank you fo the suggestion.

Our aurelia application is used in different environments the one with Zone.js is only one of them.
Solution that works for us is in bootstrap code to have:

import { PLATFORM, DOM } from 'aurelia-pal';
import { bootstrap } from 'aurelia-bootstrapper';

bootstrap((aurelia: Aurelia) => {
   aurelia.use.basicConfiguration();

  DOM.createMutationObserver = (...args) => {
    const global = window as any;
    let observerConst;
    if (global.Zone && global.Zone.__symbol__) {
      observerConst = global[global.Zone.__symbol__('MutationObserver')] || global[global.Zone.__symbol__('WebKitMutationObserver')];
    }
    return new (observerConst || global.MutationObserver || global.WebKitMutationObserver)(...args);
  }
   // ....
}

Setting the DOM.createMutationObserver after the bootstrap is important because the DOM object is not setup prior to that.

If you are not planning to add this fix, I guess this issue might help others.

Thanks

@bigopon
Copy link
Member

bigopon commented May 26, 2019

Angular is popular, so making Aurelia work seamlessly with it is desirable. But I'd be hesitant to add this. There's no guarantee that Angular would use Zone forever and there's no guarantee that Zone would stay that way forever (extremely speaking only, but I hope it's clear how uneasy one would feel adding something so specific to Angular/Zone into our modules). What we could do is to add a section in our doc, about Angular integration, for this particular use case.

About your applications, if you don't want to use Aurelia bootstrapper module, you can do this:

import { initialize } from 'aurelia-pal-browser';

initialize();

// here DOM.createMutationObserver is ready
DOM.createMutationObserver = ...

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

No branches or pull requests

3 participants