From 284123c6baeb87cc5e4a6db4be650e9fe967affd Mon Sep 17 00:00:00 2001 From: JiaLiPassion Date: Thu, 30 Apr 2020 10:51:01 +0900 Subject: [PATCH] fix(core): should fake a top event task when coalescing events to prevent draining microTaskQueue too early. (#36841) Close #36839. This is a known issue of zone.js, ``` (window as any)[(Zone as any).__symbol__('setTimeout')](() => { let log = ''; button.addEventListener('click', () => { Zone.current.scheduleMicroTask('test', () => log += 'microtask;'); log += 'click;'; }); button.click(); expect(log).toEqual('click;microtask;'); done(); }); ``` Since in this case, we use native `setTimeout` which is not a ZoneTask, so zone.js consider the button click handler as the top Task then drain the microTaskQueue after the click at once, which is not correct(too early). This case was an edge case and not reported by the users, until we have the new option ngZoneEventCoalescing, since the event coalescing will happen in native requestAnimationFrame, so it will not be a ZoneTask, and zone.js will consider any Task happen in the change detection stage as the top task, and if there are any microTasks(such as Promise.then) happen in the process, it may be drained earlier than it should be, so to prevent this situation, we need to schedule a fake event task and run the change detection check in this fake event task, so the Task happen in the change detection stage will not be considered as top ZoneTask. PR Close #36841 --- packages/core/src/zone/ng_zone.ts | 24 ++++++++-- .../test/dom/events/event_manager_spec.ts | 46 +++++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/packages/core/src/zone/ng_zone.ts b/packages/core/src/zone/ng_zone.ts index 73f937022d..251eae4c9a 100644 --- a/packages/core/src/zone/ng_zone.ts +++ b/packages/core/src/zone/ng_zone.ts @@ -243,6 +243,10 @@ interface NgZonePrivate extends NgZone { isStable: boolean; shouldCoalesceEventChangeDetection: boolean; nativeRequestAnimationFrame: (callback: FrameRequestCallback) => number; + + // Cache of "fake" top eventTask. This is done so that we don't need to schedule a new task every + // time we want to run a `checkStable`. + fakeTopEventTask: Task; } function checkStable(zone: NgZonePrivate) { @@ -268,9 +272,23 @@ function delayChangeDetectionForEvents(zone: NgZonePrivate) { return; } zone.lastRequestAnimationFrameId = zone.nativeRequestAnimationFrame.call(global, () => { - zone.lastRequestAnimationFrameId = -1; - updateMicroTaskStatus(zone); - checkStable(zone); + // This is a work around for https://github.com/angular/angular/issues/36839. + // The core issue is that when event coalescing is enabled it is possible for microtasks + // to get flushed too early (As is the case with `Promise.then`) between the + // coalescing eventTasks. + // + // To workaround this we schedule a "fake" eventTask before we process the + // coalescing eventTasks. The benefit of this is that the "fake" container eventTask + // will prevent the microtasks queue from getting drained in between the coalescing + // eventTask execution. + if (!zone.fakeTopEventTask) { + zone.fakeTopEventTask = Zone.root.scheduleEventTask('fakeTopEventTask', () => { + zone.lastRequestAnimationFrameId = -1; + updateMicroTaskStatus(zone); + checkStable(zone); + }, undefined, () => {}, () => {}); + } + zone.fakeTopEventTask.invoke(); }); updateMicroTaskStatus(zone); } diff --git a/packages/platform-browser/test/dom/events/event_manager_spec.ts b/packages/platform-browser/test/dom/events/event_manager_spec.ts index 8be341bcc3..f434e6b322 100644 --- a/packages/platform-browser/test/dom/events/event_manager_spec.ts +++ b/packages/platform-browser/test/dom/events/event_manager_spec.ts @@ -370,6 +370,52 @@ describe('EventManager', () => { done(); }); }); + + it('should not drain micro tasks queue too early with shouldCoalesceEventChangeDetection=true', + (done: DoneFn) => { + doc = getDOM().supportsDOMEvents() ? document : getDOM().createHtmlDocument(); + zone = new NgZone({shouldCoalesceEventChangeDetection: true}); + domEventPlugin = new DomEventsPlugin(doc); + const element = el('
'); + const child = el('
'); + doc.body.appendChild(element); + const dispatchedClickEvent = createMouseEvent('click'); + const dispatchedBlurEvent: FocusEvent = + getDOM().getDefaultDocument().createEvent('FocusEvent'); + dispatchedBlurEvent.initEvent('blur', true, true); + let logs: any = []; + const handler = () => {}; + + const blurHandler = (e: any) => { + logs.push('blur'); + }; + const manager = new EventManager([domEventPlugin], zone); + let removerParent: any; + let removerChildFocus: any; + + zone.run(() => { + removerParent = manager.addEventListener(element, 'click', handler); + removerChildFocus = manager.addEventListener(child, 'blur', blurHandler); + }); + const sub = zone.onStable.subscribe(() => { + logs.push('begin'); + Promise.resolve().then(() => { + logs.push('promise resolved'); + }); + element.appendChild(child); + getDOM().dispatchEvent(child, dispatchedBlurEvent); + sub.unsubscribe(); + logs.push('end'); + }); + getDOM().dispatchEvent(element, dispatchedClickEvent); + requestAnimationFrame(() => { + expect(logs).toEqual(['begin', 'blur', 'end', 'promise resolved']); + + removerParent && removerParent(); + removerChildFocus && removerChildFocus(); + done(); + }); + }); }); })();