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

WIP(core): Proposal: can unload, reload patch modules #1073

Closed
wants to merge 0 commits into from

Conversation

JiaLiPassion
Copy link
Collaborator

@JiaLiPassion JiaLiPassion commented Apr 10, 2018

This is a proposal to support unload, reload patch.

Use case: only run specified code into zone.

const zone = Zone.current.fork({name: 'zone'});
zone.run(() => {setTimeout(() => {
  console.log('zone', Zone.current.name);  // will run into zone
})});

setTimeout(() => {
  console.log('zone', Zone.current); // will run into root zone in current version.
}, 100);

for example, in current version, the code above, the 1st setTimeout will run into zone, the 2nd will run into root Zone. But even root Zone still have performance impact. So we may want the 2nd totally run in native setTimeout.

in this PR proposal, only patch all modules such as setTimeout, XHR, promise in zone.run.
and unPatch them when in root zone or by calling some method like 'runOutsideOfZone` explicitly.

Use case: such as angular elements to implement WebComponent. Only run logic with zone.js patched inside `angular elements without impact outside world.

Proposal. Auto patch when zone.run, Auto unpatch after zone.run. Auto repatch when zone.run again.

const zone = Zone.current.fork({name: 'zone'});
zone.run(() => {setTimeout(() => {
  console.log('zone', Zone.current.name);  // will run into zone
})});

setTimeout(() => {
  console.log('zone', Zone.current); // will not in root zone, this is native setTimeout
}, 100);

zone.run(() => {setTimeout(() => {
  console.log('zone', Zone.current.name);  // will run into zone again by rePatch
})});

To make unPatch and rePatch faster, keep patched delegate and native delegate reference when call Zone.__load_patch, so we only reset delegate when unPatch and rePatch.
such as https://github.com/angular/zone.js/compare/master...JiaLiPassion:module?expand=1#diff-75082c639aefcc47b3b8df57c8870867R37

@mhevery , do you think this idea worth a discussion? Thank you.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

1 similar comment
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

@JiaLiPassion
Copy link
Collaborator Author

JiaLiPassion commented Apr 29, 2018

@mhevery , @robwormald , I have send a message to you on twitter, and I also described the information here.

In ng-conf, I described an idea about using angular elements with zone.js without impacting outside world. My meaning is only let zone.js monkey-patch window APIs when enter Angular Elements and restore all APIs to original native one when exit Angular Elements.

I have created a repo here with some basic doc,
https://github.com/JiaLiPassion/angular-elements-with-zone

and a live demo here ,

https://github-zmawnz.stackblitz.io/

the idea detail is here.

AngularElements

A POC of Angular Elements with zone.js.

In ng-conf 2018, robwormald described the use cases of Angular Elements.

  1. If the Angular Elements is used inside an Angular application, then zone.js is already loaded, so user can develop the Angular Elements in normal way.

    • Pro: User can develop Angular Element just like they develop normal Angular Component, and user can easily expose a lot of existing Angular Component to Angular Element.
    • Con: In this case, there is a limitation that the Angular Element need to run in a Angular App Host.
  2. If the Angular Elements is used inside an non-angular application, such as inside a web app which developed with pure js or jquery or react, then we should not use zone.js, because zone.js will monkey-patch a lot of global/window APIs such as setTimeout/Promise, so using zone.js will impact the APIs outside of Angular Elements. Instead, we use noop zone.

    • Pro: Don't need to worry about zone.js, no Window API will be patched, and the bundle size is smaller (zone.js will be 12k).
    • Con: User need to take care of Change Detection themselves, and existing Angular Component will not be easily exported as Angular Element in this way.

So my idea is add a 3rd option.

  1. if there is a way to let zone.js only patch global/window APIs when we enter Angular Elements and restore the original delegate when we exit Angular Elements. We can develop Angular Elements with zone.js without impact outside world.
    • Pro: Still has ngZone, user can still develop Angular Element just like developing normal Angular Component. And of course, the existing Angular Component can easily be exported as Angular Element. And user don't need to worry about zone.js will impact other parts of the webapp which are outside of Angular Element.
    • Con: User still need to load zone.js, which will be 12k bundle.

This is the overview Overview

How it works

  1. First import zone.js/dist/zone.js will not monkey-patch anything until Zone.__init__() is called.
  2. Call Zone.__init__() before bootstrap Angular Elements.
  3. After BootStrap, restore all monkey-patched APIs to native one.
try {
  Zone.__init__(); // monkey patch window APIs
  platformBrowserDynamic()
    .bootstrapModule(AppModule)
    .then(ref => {
      // Ensure Angular destroys itself on hot reloads.
      if (window['ngRef']) {
        window['ngRef'].destroy();
      }
      window['ngRef'] = ref;

      // Otherise, log the boot error
    })
    .catch(err => console.error(err));
} finally {
  Zone.__unloadAll_patch(); // detach the monkey patch, so all window APIs was restored to native one.
}
  1. Inside zone.run()/zone.runGuarded(), reload all patches before run and unload all patches after run.

So monkey patched version of window API only exists inside Angular Elements

Here is the demo link DEMO.

In the demo, there is an angular Element with zone.js, and it will run inside of angular zone, and outside there is a button Click outside of Angular Element, click the button will check eventHandler/setTimeout/Promise is monkey patched or not, and print error stack traces to verify no zone related stack frames are inside.

Please review, thank you very much!

@googlebot
Copy link

CLAs look good, thanks!

1 similar comment
@googlebot
Copy link

CLAs look good, thanks!

@mhevery
Copy link
Contributor

mhevery commented Jun 20, 2018

I am a bit confused. If I understand correctly this PR will remove the setTimeout patch whenever you exit Zone.run. Is my understanding correct?

My concern is that the cost of patching and un-patching is way more than the cost of setTimeout indirection.

Do we have perf numbers that say this makes things better?

@JiaLiPassion
Copy link
Collaborator Author

@mhevery ,

I am a bit confused. If I understand correctly this PR will remove the setTimeout patch whenever you exit Zone.run. Is my understanding correct?

yes, it will remove patching after Zone.run. I will make a benchmark to test the perf cost, the implementation only reset the reference, I believe it will not cost a lot.
And I think we may add a flag to make this patch/unpatch only happens in Angular Elements.

@JiaLiPassion
Copy link
Collaborator Author

@mhevery , I did a benchmark test.

for (let i = 0; i < 1000000; i++) {
  zone.run(function () {});
}

And by average,

  • each time reload all patch will cost 0.00375 ms
  • each time unload all patch will cost 0.00329 ms

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

Successfully merging this pull request may close these issues.

3 participants