templating icon indicating copy to clipboard operation
templating copied to clipboard

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

Open genadis opened this issue 5 years ago • 5 comments

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.

genadis avatar May 25 '19 16:05 genadis

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.

EisenbergEffect avatar May 25 '19 17:05 EisenbergEffect

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: https://github.com/angular/angular/issues/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 avatar May 25 '19 21:05 genadis

@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);
}

bigopon avatar May 26 '19 00:05 bigopon

@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

genadis avatar May 26 '19 09:05 genadis

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 = ...

bigopon avatar May 26 '19 11:05 bigopon