fix(ivy): properly coalesce event handler in presence of queries (#29957)

The `TNode.cleanup` data structure can contain sequences of 4-element
sequence of entries (event handlers, directive outputs) mixed with
2-element sequence of entries (QueryList cleanup). Before this fix
we would always skip 4 elements in the `TNode.cleanup` while looking
up event handler cleanups. 4-element skips are not correct in case
of query cleanup presence and this commit corrects the algorithm to
jump 4 or 2 elements depending on a type of cleanup encountered.

PR Close #29957
This commit is contained in:
Pawel Kozlowski 2019-04-17 15:28:54 +02:00 committed by Ben Lesh
parent 1a56cd5c0b
commit 5fee9daa5b
2 changed files with 35 additions and 3 deletions

View File

@ -75,7 +75,8 @@ function findExistingListener(
const tCleanup = tView.cleanup;
if (tCleanup != null) {
for (let i = 0; i < tCleanup.length - 1; i += 2) {
if (tCleanup[i] === eventName && tCleanup[i + 1] === tNodeIdx) {
const cleanupEventName = tCleanup[i];
if (cleanupEventName === eventName && tCleanup[i + 1] === tNodeIdx) {
// We have found a matching event name on the same node but it might not have been
// registered yet, so we must explicitly verify entries in the LView cleanup data
// structures.
@ -88,7 +89,9 @@ function findExistingListener(
// blocks of 4 or 2 items in the tView.cleanup and this is why we iterate over 2 elements
// first and jump another 2 elements if we detect listeners cleanup (4 elements). Also check
// documentation of TView.cleanup for more details of this data structure layout.
i += 2;
if (typeof cleanupEventName === 'string') {
i += 2;
}
}
}
return null;

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {Component, Directive, ErrorHandler, HostListener} from '@angular/core';
import {Component, Directive, ErrorHandler, HostListener, QueryList, ViewChildren} from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
import {onlyInIvy} from '@angular/private/testing';
@ -105,6 +105,35 @@ describe('event listeners', () => {
expect(buttonDebugEls[1].injector.get(MdButton).counter).toBe(1);
});
onlyInIvy('ngDevMode.rendererAddEventListener counters are only available in ivy')
.it('should coalesce multiple event listeners in presence of queries', () => {
@Component({
selector: 'test-cmpt',
template: `<button likes-clicks (click)="counter = counter+1">Click me!</button>`
})
class TestCmpt {
counter = 0;
@ViewChildren('nothing') nothing !: QueryList<any>;
}
TestBed.configureTestingModule({declarations: [TestCmpt, LikesClicks]});
const noOfEventListenersRegisteredSoFar = getNoOfNativeListeners();
const fixture = TestBed.createComponent(TestCmpt);
fixture.detectChanges();
const buttonDebugEl = fixture.debugElement.query(By.css('button'));
// We want to assert that only one native event handler was registered but still all
// directives are notified when an event fires. This assertion can only be verified in
// the ngDevMode (but the coalescing always happens!).
ngDevMode && expect(getNoOfNativeListeners()).toBe(noOfEventListenersRegisteredSoFar + 1);
buttonDebugEl.nativeElement.click();
expect(buttonDebugEl.injector.get(LikesClicks).counter).toBe(1);
expect(fixture.componentInstance.counter).toBe(1);
});
it('should try to execute remaining coalesced listeners if one of the listeners throws', () => {