Skip to content

Commit

Permalink
fix(core): Ensure OnPush ancestors are marked dirty when events occur (
Browse files Browse the repository at this point in the history
…#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
  • Loading branch information
atscott authored and thePunderWoman committed Nov 25, 2020
1 parent 95ed2b0 commit 01c1bfd
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 2 deletions.
4 changes: 4 additions & 0 deletions packages/core/src/render3/instructions/listener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 37 additions & 2 deletions packages/core/test/acceptance/change_detection_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<string>();

ngOnInit() {
setTimeout(() => {
this.event.emit('new message');
});
}
}


@Component({
selector: 'my-app',
template: '{{message}} <ng-template emitter (event)="message = $event"></ng-template>',
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', () => {
Expand Down

0 comments on commit 01c1bfd

Please sign in to comment.