fix: resolve event listeners not correct when registered outside of ngZone (#33711)

Close #33687.

PR Close #33711
This commit is contained in:
JiaLiPassion 2019-11-10 00:48:07 +08:00 committed by Kara Erickson
parent b3c3000004
commit d8be830fce
6 changed files with 28 additions and 221 deletions

View File

@ -3,7 +3,7 @@
"master": { "master": {
"uncompressed": { "uncompressed": {
"runtime-es2015": 1485, "runtime-es2015": 1485,
"main-es2015": 143181, "main-es2015": 141575,
"polyfills-es2015": 36808 "polyfills-es2015": 36808
} }
} }
@ -21,7 +21,7 @@
"master": { "master": {
"uncompressed": { "uncompressed": {
"runtime-es2015": 1485, "runtime-es2015": 1485,
"main-es2015": 149315, "main-es2015": 147719,
"polyfills-es2015": 36808 "polyfills-es2015": 36808
} }
} }
@ -64,4 +64,4 @@
} }
} }
} }
} }

View File

@ -9,6 +9,7 @@
import {CommonModule, NgIfContext, ɵgetDOM as getDOM} from '@angular/common'; import {CommonModule, NgIfContext, ɵgetDOM as getDOM} from '@angular/common';
import {Component, DebugElement, DebugNode, Directive, ElementRef, EmbeddedViewRef, EventEmitter, HostBinding, Injectable, Input, NO_ERRORS_SCHEMA, OnInit, Output, Renderer2, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core'; import {Component, DebugElement, DebugNode, Directive, ElementRef, EmbeddedViewRef, EventEmitter, HostBinding, Injectable, Input, NO_ERRORS_SCHEMA, OnInit, Output, Renderer2, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core';
import {NgZone} from '@angular/core/src/zone';
import {ComponentFixture, TestBed, async} from '@angular/core/testing'; import {ComponentFixture, TestBed, async} from '@angular/core/testing';
import {By} from '@angular/platform-browser/src/dom/debug/by'; import {By} from '@angular/platform-browser/src/dom/debug/by';
import {hasClass} from '@angular/platform-browser/testing/src/browser_util'; import {hasClass} from '@angular/platform-browser/testing/src/browser_util';
@ -795,13 +796,21 @@ class TestCmptWithPropInterpolation {
@Component({template: ''}) @Component({template: ''})
class TestComponent implements OnInit { class TestComponent implements OnInit {
count = 0; count = 0;
eventObj: any; eventObj1: any;
constructor(private renderer: Renderer2, private elementRef: ElementRef) {} eventObj2: any;
constructor(
private renderer: Renderer2, private elementRef: ElementRef, private ngZone: NgZone) {}
ngOnInit() { ngOnInit() {
this.renderer.listen(this.elementRef.nativeElement, 'click', (event: any) => { this.renderer.listen(this.elementRef.nativeElement, 'click', (event: any) => {
this.count++; this.count++;
this.eventObj = event; this.eventObj1 = event;
});
this.ngZone.runOutsideAngular(() => {
this.renderer.listen(this.elementRef.nativeElement, 'click', (event: any) => {
this.count++;
this.eventObj2 = event;
});
}); });
} }
} }
@ -816,8 +825,8 @@ class TestCmptWithPropInterpolation {
const event = {value: true}; const event = {value: true};
fixture.detectChanges(); fixture.detectChanges();
fixture.debugElement.triggerEventHandler('click', event); fixture.debugElement.triggerEventHandler('click', event);
expect(fixture.componentInstance.count).toBe(1); expect(fixture.componentInstance.count).toBe(2);
expect(fixture.componentInstance.eventObj).toBe(event); expect(fixture.componentInstance.eventObj2).toBe(event);
} }
}); });

View File

@ -18,7 +18,7 @@ import {callMostRecentEventListenerHandler, compViewDef, createAndGetRootNodes,
* We map addEventListener to the Zones internal name. This is because we want to be fast * We map addEventListener to the Zones internal name. This is because we want to be fast
* and bypass the zone bookkeeping. We know that we can do the bookkeeping faster. * and bypass the zone bookkeeping. We know that we can do the bookkeeping faster.
*/ */
const addEventListener = '__zone_symbol__addEventListener' as 'addEventListener'; const addEventListener = 'addEventListener';
{ {
describe(`Component Views`, () => { describe(`Component Views`, () => {

View File

@ -20,8 +20,8 @@ import {ARG_TYPE_VALUES, callMostRecentEventListenerHandler, checkNodeInlineOrDy
* We map addEventListener to the Zones internal name. This is because we want to be fast * We map addEventListener to the Zones internal name. This is because we want to be fast
* and bypass the zone bookkeeping. We know that we can do the bookkeeping faster. * and bypass the zone bookkeeping. We know that we can do the bookkeeping faster.
*/ */
const addEventListener = '__zone_symbol__addEventListener' as 'addEventListener'; const addEventListener = 'addEventListener';
const removeEventListener = '__zone_symbol__removeEventListener' as 'removeEventListener'; const removeEventListener = 'removeEventListener';
{ {
describe(`View Elements`, () => { describe(`View Elements`, () => {

View File

@ -6,227 +6,25 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {DOCUMENT, isPlatformServer} from '@angular/common'; import {DOCUMENT} from '@angular/common';
import {Inject, Injectable, NgZone, Optional, PLATFORM_ID} from '@angular/core'; import {Inject, Injectable} from '@angular/core';
import {EventManagerPlugin} from './event_manager'; import {EventManagerPlugin} from './event_manager';
/**
* Detect if Zone is present. If it is then use simple zone aware 'addEventListener'
* since Angular can do much more
* efficient bookkeeping than Zone can, because we have additional information. This speeds up
* addEventListener by 3x.
*/
const __symbol__ =
(() => (typeof Zone !== 'undefined') && (Zone as any)['__symbol__'] ||
function(v: string): string { return '__zone_symbol__' + v; })();
const ADD_EVENT_LISTENER: 'addEventListener' = __symbol__('addEventListener');
const REMOVE_EVENT_LISTENER: 'removeEventListener' = __symbol__('removeEventListener');
const symbolNames: {[key: string]: string} = {};
const FALSE = 'FALSE';
const ANGULAR = 'ANGULAR';
const NATIVE_ADD_LISTENER = 'addEventListener';
const NATIVE_REMOVE_LISTENER = 'removeEventListener';
// use the same symbol string which is used in zone.js
const stopSymbol = '__zone_symbol__propagationStopped';
const stopMethodSymbol = '__zone_symbol__stopImmediatePropagation';
const unpatchedMap: {[key: string]: string}|undefined = (() => {
const unpatchedEvents =
(typeof Zone !== 'undefined') && (Zone as any)[__symbol__('UNPATCHED_EVENTS')];
if (unpatchedEvents) {
const unpatchedEventMap: {[eventName: string]: string} = {};
unpatchedEvents.forEach((eventName: string) => { unpatchedEventMap[eventName] = eventName; });
return unpatchedEventMap;
}
return undefined;
})();
const isUnpatchedEvent = function(eventName: string) {
if (!unpatchedMap) {
return false;
}
return unpatchedMap.hasOwnProperty(eventName);
};
interface TaskData {
zone: any;
handler: Function;
}
// a global listener to handle all dom event,
// so we do not need to create a closure every time
const globalListener = function(this: any, event: Event) {
const symbolName = symbolNames[event.type];
if (!symbolName) {
return;
}
const taskDatas: TaskData[] = this[symbolName];
if (!taskDatas) {
return;
}
const args: any = [event];
if (taskDatas.length === 1) {
// if taskDatas only have one element, just invoke it
const taskData = taskDatas[0];
if (taskData.zone !== Zone.current) {
// only use Zone.run when Zone.current not equals to stored zone
return taskData.zone.run(taskData.handler, this, args);
} else {
return taskData.handler.apply(this, args);
}
} else {
// copy tasks as a snapshot to avoid event handlers remove
// itself or others
const copiedTasks = taskDatas.slice();
for (let i = 0; i < copiedTasks.length; i++) {
// if other listener call event.stopImmediatePropagation
// just break
if ((event as any)[stopSymbol] === true) {
break;
}
const taskData = copiedTasks[i];
if (taskData.zone !== Zone.current) {
// only use Zone.run when Zone.current not equals to stored zone
taskData.zone.run(taskData.handler, this, args);
} else {
taskData.handler.apply(this, args);
}
}
}
};
@Injectable() @Injectable()
export class DomEventsPlugin extends EventManagerPlugin { export class DomEventsPlugin extends EventManagerPlugin {
constructor( constructor(@Inject(DOCUMENT) doc: any) { super(doc); }
@Inject(DOCUMENT) doc: any, private ngZone: NgZone,
@Optional() @Inject(PLATFORM_ID) platformId: {}|null) {
super(doc);
if (!platformId || !isPlatformServer(platformId)) {
this.patchEvent();
}
}
private patchEvent() {
if (typeof Event === 'undefined' || !Event || !Event.prototype) {
return;
}
if ((Event.prototype as any)[stopMethodSymbol]) {
// already patched by zone.js
return;
}
const delegate = (Event.prototype as any)[stopMethodSymbol] =
Event.prototype.stopImmediatePropagation;
Event.prototype.stopImmediatePropagation = function(this: any) {
if (this) {
this[stopSymbol] = true;
}
// We should call native delegate in case in some environment part of
// the application will not use the patched Event. Also we cast the
// "arguments" to any since "stopImmediatePropagation" technically does not
// accept any arguments, but we don't know what developers pass through the
// function and we want to not break these calls.
delegate && delegate.apply(this, arguments as any);
};
}
// This plugin should come last in the list of plugins, because it accepts all // This plugin should come last in the list of plugins, because it accepts all
// events. // events.
supports(eventName: string): boolean { return true; } supports(eventName: string): boolean { return true; }
addEventListener(element: HTMLElement, eventName: string, handler: Function): Function { addEventListener(element: HTMLElement, eventName: string, handler: Function): Function {
/** element.addEventListener(eventName, handler as EventListener, false);
* This code is about to add a listener to the DOM. If Zone.js is present, than return () => this.removeEventListener(element, eventName, handler as EventListener);
* `addEventListener` has been patched. The patched code adds overhead in both
* memory and speed (3x slower) than native. For this reason if we detect that
* Zone.js is present we use a simple version of zone aware addEventListener instead.
* The result is faster registration and the zone will be restored.
* But ZoneSpec.onScheduleTask, ZoneSpec.onInvokeTask, ZoneSpec.onCancelTask
* will not be invoked
* We also do manual zone restoration in element.ts renderEventHandlerClosure method.
*
* NOTE: it is possible that the element is from different iframe, and so we
* have to check before we execute the method.
*/
const self = this;
const zoneJsLoaded = element[ADD_EVENT_LISTENER];
let callback: EventListener = handler as EventListener;
// if zonejs is loaded and current zone is not ngZone
// we keep Zone.current on target for later restoration.
if (zoneJsLoaded && (!NgZone.isInAngularZone() || isUnpatchedEvent(eventName))) {
let symbolName = symbolNames[eventName];
if (!symbolName) {
symbolName = symbolNames[eventName] = __symbol__(ANGULAR + eventName + FALSE);
}
let taskDatas: TaskData[] = (element as any)[symbolName];
const globalListenerRegistered = taskDatas && taskDatas.length > 0;
if (!taskDatas) {
taskDatas = (element as any)[symbolName] = [];
}
const zone = isUnpatchedEvent(eventName) ? Zone.root : Zone.current;
if (taskDatas.length === 0) {
taskDatas.push({zone: zone, handler: callback});
} else {
let callbackRegistered = false;
for (let i = 0; i < taskDatas.length; i++) {
if (taskDatas[i].handler === callback) {
callbackRegistered = true;
break;
}
}
if (!callbackRegistered) {
taskDatas.push({zone: zone, handler: callback});
}
}
if (!globalListenerRegistered) {
element[ADD_EVENT_LISTENER](eventName, globalListener, false);
}
} else {
element[NATIVE_ADD_LISTENER](eventName, callback, false);
}
return () => this.removeEventListener(element, eventName, callback);
} }
removeEventListener(target: any, eventName: string, callback: Function): void { removeEventListener(target: any, eventName: string, callback: Function): void {
let underlyingRemove = target[REMOVE_EVENT_LISTENER]; return target.removeEventListener(eventName, callback as EventListener);
// zone.js not loaded, use native removeEventListener
if (!underlyingRemove) {
return target[NATIVE_REMOVE_LISTENER].apply(target, [eventName, callback, false]);
}
let symbolName = symbolNames[eventName];
let taskDatas: TaskData[] = symbolName && target[symbolName];
if (!taskDatas) {
// addEventListener not using patched version
// just call native removeEventListener
return target[NATIVE_REMOVE_LISTENER].apply(target, [eventName, callback, false]);
}
// fix issue 20532, should be able to remove
// listener which was added inside of ngZone
let found = false;
for (let i = 0; i < taskDatas.length; i++) {
// remove listener from taskDatas if the callback equals
if (taskDatas[i].handler === callback) {
found = true;
taskDatas.splice(i, 1);
break;
}
}
if (found) {
if (taskDatas.length === 0) {
// all listeners are removed, we can remove the globalListener from target
underlyingRemove.apply(target, [eventName, globalListener, false]);
}
} else {
// not found in taskDatas, the callback may be added inside of ngZone
// use native remove listener to remove the callback
target[NATIVE_REMOVE_LISTENER].apply(target, [eventName, callback, false]);
}
} }
} }

View File

@ -23,7 +23,7 @@ import {createMouseEvent, el} from '../../../testing/src/browser_util';
beforeEach(() => { beforeEach(() => {
doc = getDOM().supportsDOMEvents() ? document : getDOM().createHtmlDocument(); doc = getDOM().supportsDOMEvents() ? document : getDOM().createHtmlDocument();
zone = new NgZone({}); zone = new NgZone({});
domEventPlugin = new DomEventsPlugin(doc, zone, null); domEventPlugin = new DomEventsPlugin(doc);
}); });
it('should delegate event bindings to plugins that are passed in from the most generic one to the most specific one', it('should delegate event bindings to plugins that are passed in from the most generic one to the most specific one',
@ -322,7 +322,7 @@ import {createMouseEvent, el} from '../../../testing/src/browser_util';
it('should only trigger one Change detection when bubbling', (done: DoneFn) => { it('should only trigger one Change detection when bubbling', (done: DoneFn) => {
doc = getDOM().supportsDOMEvents() ? document : getDOM().createHtmlDocument(); doc = getDOM().supportsDOMEvents() ? document : getDOM().createHtmlDocument();
zone = new NgZone({shouldCoalesceEventChangeDetection: true}); zone = new NgZone({shouldCoalesceEventChangeDetection: true});
domEventPlugin = new DomEventsPlugin(doc, zone, null); domEventPlugin = new DomEventsPlugin(doc);
const element = el('<div></div>'); const element = el('<div></div>');
const child = el('<div></div>'); const child = el('<div></div>');
element.appendChild(child); element.appendChild(child);