fix(elements): elements should run in correct zone (#24181)

[remackgeek]

PR Checklist

Please check if your PR fulfills the following requirements:

• [x ] The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
• [x ] Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• angular.io application / infrastructure changes
• Other... Please describe:

What is the current behavior?

When an angular element is created outside the angular zone, such as in setTimeout(), automatic change detection no longer works

Issue Number: #24181

What is the new behavior?

ComponentNgElementStrategy captures the zone it was created in and insures all methods are run in that zone.

Does this PR introduce a breaking change?

• Yes
• [x ] No

Other information

[kseamon]

As I had commented way back, what this is still missing is exiting the NgZone in outputs - we don't want to leak our app's NgZone to the parent app (which would cause unrelated activities in the parent app to trigger change detection within the wrapped child application).

[gkalpak]

For reference: There is some relevant discussion in #24185.

[gkalpak]

@kseamon, can you give an example of the problem (I don't quite understand how we can "leak" our Zone).
Also, is this issue something that is introduced by this PR? Or is it possible that this "Zone leaking" can happen today (without the PR)?

[kseamon]

It would look something like this:

// Parent app
ngElementNode.addEventListener('customEvent', () => {
  setTimeout(() => {
    doSomethingUnrelated();
    // Whoops we just triggered CD within the Angular Element.
  });
});

It would be possible for this to happen before if the code inside of the Angular Element manually entered the NgZone. This PR just makes it more likely to be an issue, and I think it's something that the framework should take care of for us (just like entering the NgZone in the first place).

[gkalpak]

Since this change is unrelated to this PR, I consider it outside of scope for this PR. Feel free to create an issue about so we can discuss there.

On a quick note:

I wouldn't expect Zones to be leaked the way you described (neither with nor without the changes in this PR). If I am not mistaken, what matters is the Zone in which you subscribe to the observable and this happens in the CustomElement's connectedCallback() and is not affected by the strategy's runInZone().

If anything, we might need to enter the Zone in the event listener callback, if the element is created in a way that causes its connectedCallback() method to be called outside of the Angular Zone 😛

_{I only took a quick look, so I may have missed something. We should take a closer look and look at concrete examples (both with and without CustomElement polyfills).}

[gkalpak]

Thx for working on this, @remackgeek!

I think this is heading into the right direction. I have left a few minor comments.
Can you also please rebase on latest master and squash the commits into one?

BTW, it would be great to have a more realistic test via an actual example app (either in packages/examples/ or in integration/ (similar to or as part of integration/ng_elements/)), but that might be too much to ask for this PR 😁

[gkalpak]

LGTM - Thx, @remackgeek ✨
Can you please update goldens/size-tracking/aio-payloads.json#L15 to the new size?

For reference, this PR increases the angular.io main bundle by 295B (451100B --> 451395B). I inspected the bundle and confirmed that the only difference is the updated ComponentFactoryNgStrategy class.

[jelbourn]

LGTM

Reviewed-for: size-tracking

[petebacondarwin]

Can you rebase and fix the size-tracking conflict so that we can approve for size-tracking?

[remackgeek]

Rebased, broken size tracking changed to match actuals (not sure what else to put there).

[petebacondarwin]

Reviewed-for: size-tracking

[petebacondarwin]

Awesome thanks!

[kyliau]

presubmit: http://test/OCL:321780152:BASE:321780169:1594998439408:2f37a981

[AndrewKushnir]

Presubmit #2 + Global Presubmit.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}