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.

Sign up for GitHub

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

Jump to bottom

Event listeners do not mark component as dirty when attached to ng-template #39832

Closed
atscott opened this issue Nov 24, 2020 · 1 comment
Closed

Event listeners do not mark component as dirty when attached to ng-template #39832

atscott opened this issue Nov 24, 2020 · 1 comment
Labels
area: core Issues related to the framework runtime core: change detection core: event listeners hotlist: google P2 The issue is important to a large percentage of users, with a workaround
Milestone
Backlog

Comments

@atscott
Copy link
Contributor

atscott commented Nov 24, 2020

Affected Package

@angular/core

Is this a regression?

Yes, this works in View Engine.

Description

Events from a directive do not mark the parent component as dirty if the directive is on a non-rendered element (like an ng-template).

🔬 Minimal Reproduction

https://stackblitz.com/edit/dir-events-on-template?file=src/app/app.component.ts

  • The directive is configured to emit an event every 1 second. You can see in the console log that the assignment is happening every 1 second.
  • Only the component with the counter directive on a div is marked dirty and refreshed.
  • The component with the counter on the ng-template is not updated.
  • If you go to settings and disable Ivy, this both components will get refreshed.

Original report: https://buganizer.corp.google.com/issues/173600938 (Overlay callbacks do not work with Ivy and OnPush)

We only call wrapListener when the tNode is TNodeType.AnyRNode:

angular/packages/core/src/render3/instructions/listener.ts

Line 135 in 23c36a2

if (tNode.type & TNodeType.AnyRNode) {

wrapListener handles the logic for marking the view as dirty:

angular/packages/core/src/render3/instructions/listener.ts

Lines 265 to 267 in 23c36a2

if ((lView[FLAGS] & LViewFlags.ManualOnPush) === 0) {
markViewDirty(startView);
}

In View Engine, events always went through dispatchEvent, which handled the logic for marking the view as dirty:

angular/packages/core/src/view/util.ts

Line 135 in d1ea1f4

markParentViewsForCheck(startView);

@atscott atscott added area: core Issues related to the framework runtime hotlist: google engine: ivy core: event listeners core: change detection P2 The issue is important to a large percentage of users, with a workaround labels Nov 24, 2020
@ngbot ngbot bot added this to the Backlog milestone Nov 24, 2020
atscott added a commit to atscott/angular that referenced this issue Nov 25, 2020
@atscott
fix(core): Ensure OnPush ancestors are marked dirty when events occur
00b4f39 
We currently only wrap the event listener in the function which ensures
ancestors are marked for check when the listener is placed on an element
that has a native method for listening to an event. We actually need to do
this wrapping in all cases so that events that are attached to non-rendered
template items (`ng-template` and `ng-container`) also mark ancestors for check
when they receive the event.

fixes angular#39832
atscott added a commit to atscott/angular that referenced this issue Nov 25, 2020
@atscott
fix(core): Ensure OnPush ancestors are marked dirty when events occur
47a83a4 
We currently only wrap the event listener in the function which ensures
ancestors are marked for check when the listener is placed on an element
that has a native method for listening to an event. We actually need to do
this wrapping in all cases so that events that are attached to non-rendered
template items (`ng-template` and `ng-container`) also mark ancestors for check
when they receive the event.

fixes angular#39832
@atscott atscott mentioned this issue Nov 25, 2020
Closed
jessicajaniuk pushed a commit that referenced this issue Nov 25, 2020
@atscott @thePunderWoman
fix(core): Ensure OnPush ancestors are marked dirty when events occur (…
01c1bfd 
…#39833)

We currently only wrap the event listener in the function which ensures
ancestors are marked for check when the listener is placed on an element
that has a native method for listening to an event. We actually need to do
this wrapping in all cases so that events that are attached to non-rendered
template items (`ng-template` and `ng-container`) also mark ancestors for check
when they receive the event.

fixes #39832

PR Close #39833
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area: core Issues related to the framework runtime core: change detection core: event listeners hotlist: google P2 The issue is important to a large percentage of users, with a workaround
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

fix(core): Ensure OnPush ancestors are marked dirty when events occur atscott/angular
1 participant
@atscott