From 68d4a74411cdfcf1f0ed57e1754489170933a7c9 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 24 Nov 2020 17:18:07 -0800 Subject: [PATCH] fix(core): Ensure OnPush ancestors are marked dirty when events occur (#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 --- .../core/src/render3/instructions/listener.ts | 4 ++ .../test/acceptance/change_detection_spec.ts | 39 ++++++++++++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/packages/core/src/render3/instructions/listener.ts b/packages/core/src/render3/instructions/listener.ts index e58106cab8410..50616cc96244b 100644 --- a/packages/core/src/render3/instructions/listener.ts +++ b/packages/core/src/render3/instructions/listener.ts @@ -193,6 +193,10 @@ function listenerInternal( lCleanup.push(listenerFn); tCleanup && tCleanup.push(eventName, idxOrTargetGetter, lCleanupIndex, useCapture); } + } else { + // Even if there is no native listener to add, we still need to wrap the listener so that OnPush + // ancestors are marked dirty when an event occurs. + listenerFn = wrapListener(tNode, lView, listenerFn, false /** preventDefault */); } // subscribe to directive outputs diff --git a/packages/core/test/acceptance/change_detection_spec.ts b/packages/core/test/acceptance/change_detection_spec.ts index 3a34db45106e6..36b07659ed093 100644 --- a/packages/core/test/acceptance/change_detection_spec.ts +++ b/packages/core/test/acceptance/change_detection_spec.ts @@ -8,8 +8,8 @@ import {CommonModule} from '@angular/common'; -import {ApplicationRef, ChangeDetectionStrategy, ChangeDetectorRef, Component, ComponentFactoryResolver, ComponentRef, Directive, DoCheck, EmbeddedViewRef, ErrorHandler, Input, NgModule, OnInit, QueryList, TemplateRef, Type, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core'; -import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {ApplicationRef, ChangeDetectionStrategy, ChangeDetectorRef, Component, ComponentFactoryResolver, ComponentRef, Directive, DoCheck, EmbeddedViewRef, ErrorHandler, EventEmitter, Input, NgModule, OnInit, Output, QueryList, TemplateRef, Type, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core'; +import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {ivyEnabled} from '@angular/private/testing'; import {BehaviorSubject} from 'rxjs'; @@ -461,6 +461,41 @@ describe('change detection', () => { expect(comp.doCheckCount).toEqual(2); expect(fixture.nativeElement.textContent.trim()).toEqual('3 - 2 - Nancy'); }); + + it('should check parent OnPush components when child directive on a template emits event', + fakeAsync(() => { + @Directive({ + selector: '[emitter]', + }) + class Emitter { + @Output() event = new EventEmitter(); + + ngOnInit() { + setTimeout(() => { + this.event.emit('new message'); + }); + } + } + + + @Component({ + selector: 'my-app', + template: '{{message}} ', + changeDetection: ChangeDetectionStrategy.OnPush + }) + class MyApp { + message = 'initial message'; + } + + const fixture = TestBed.configureTestingModule({declarations: [MyApp, Emitter]}) + .createComponent(MyApp); + fixture.detectChanges(); + + expect(fixture.nativeElement.textContent.trim()).toEqual('initial message'); + tick(); + fixture.detectChanges(); + expect(fixture.nativeElement.textContent.trim()).toEqual('new message'); + })); }); describe('ChangeDetectorRef', () => {