diff --git a/packages/core/src/zone/ng_zone.ts b/packages/core/src/zone/ng_zone.ts index bb230c27f7..33152592cb 100644 --- a/packages/core/src/zone/ng_zone.ts +++ b/packages/core/src/zone/ng_zone.ts @@ -148,6 +148,7 @@ export class NgZone { !shouldCoalesceRunChangeDetection && shouldCoalesceEventChangeDetection; self.shouldCoalesceRunChangeDetection = shouldCoalesceRunChangeDetection; self.lastRequestAnimationFrameId = -1; + self.isCheckStableRunning = false; self.nativeRequestAnimationFrame = getNativeRequestAnimationFrame().nativeRequestAnimationFrame; forkInnerZoneWithAngularBehavior(self); } @@ -243,6 +244,17 @@ interface NgZonePrivate extends NgZone { hasPendingMacrotasks: boolean; hasPendingMicrotasks: boolean; lastRequestAnimationFrameId: number; + /** + * A flag to indicate if NgZone is currently inside + * checkStable and to prevent re-entry. The flag is + * needed because it is possible to invoke the change + * detection from within change detection leading to + * incorrect behavior. + * + * For detail, please refer here, + * https://github.com/angular/angular/pull/40540 + */ + isCheckStableRunning: boolean; isStable: boolean; /** * Optionally specify coalescing event change detections or not. @@ -291,7 +303,9 @@ interface NgZonePrivate extends NgZone { } function checkStable(zone: NgZonePrivate) { - if (zone._nesting == 0 && !zone.hasPendingMicrotasks && !zone.isStable) { + if (!zone.isCheckStableRunning && zone._nesting == 0 && !zone.hasPendingMicrotasks && + !zone.isStable) { + zone.isCheckStableRunning = true; try { zone._nesting++; zone.onMicrotaskEmpty.emit(null); @@ -304,12 +318,26 @@ function checkStable(zone: NgZonePrivate) { zone.isStable = true; } } + zone.isCheckStableRunning = false; } } } function delayChangeDetectionForEvents(zone: NgZonePrivate) { - if (zone.lastRequestAnimationFrameId !== -1) { + /** + * We also need to check isCheckStableRunning here + * Consider the following case with shouldCoalesceRunChangeDetection = true + * + * ngZone.run(() => {}); + * ngZone.run(() => {}); + * + * We want the two `ngZone.run()` only trigger one change detection + * when shouldCoalesceRunChangeDetection is true. + * And because in this case, change detection run in async way(requestAnimationFrame), + * so we also need to check the isCheckStableRunning here to prevent multiple + * change detections. + */ + if (zone.isCheckStableRunning || zone.lastRequestAnimationFrameId !== -1) { return; } zone.lastRequestAnimationFrameId = zone.nativeRequestAnimationFrame.call(global, () => { diff --git a/packages/core/test/zone/ng_zone_spec.ts b/packages/core/test/zone/ng_zone_spec.ts index 3f11518a0e..dd9b6cc5ad 100644 --- a/packages/core/test/zone/ng_zone_spec.ts +++ b/packages/core/test/zone/ng_zone_spec.ts @@ -982,6 +982,7 @@ function commonTests() { describe('shouldCoalesceEventChangeDetection = true, shouldCoalesceRunChangeDetection = false', () => { let nativeRequestAnimationFrame: (fn: FrameRequestCallback) => void; + let nativeSetTimeout: any = (global as any)[Zone.__symbol__('setTimeout')]; if (!(global as any).requestAnimationFrame) { nativeRequestAnimationFrame = function(fn: Function) { (global as any)[Zone.__symbol__('setTimeout')](fn, 16); @@ -1042,6 +1043,32 @@ function commonTests() { }); }); + it('should only emit onMicroTaskEmpty once within the same event loop for ngZone.run in onMicrotaskEmpty subscription', + (done: DoneFn) => { + const tasks: Task[] = []; + coalesceZone.onMicrotaskEmpty.subscribe(() => { + coalesceZone.run(() => {}); + }); + coalesceZone.run(() => { + tasks.push(Zone.current.scheduleEventTask('myEvent', () => { + logs.push('eventTask1'); + }, undefined, () => {})); + }); + coalesceZone.run(() => { + tasks.push(Zone.current.scheduleEventTask('myEvent', () => { + logs.push('eventTask2'); + }, undefined, () => {})); + }); + tasks.forEach(t => t.invoke()); + expect(logs).toEqual(['microTask empty', 'microTask empty', 'eventTask1', 'eventTask2']); + nativeSetTimeout(() => { + expect(logs).toEqual([ + 'microTask empty', 'microTask empty', 'eventTask1', 'eventTask2', 'microTask empty' + ]); + done(); + }, 100); + }); + it('should emit onMicroTaskEmpty once within the same event loop for not only event tasks, but event tasks are before other tasks', (done: DoneFn) => { const tasks: Task[] = []; @@ -1094,6 +1121,7 @@ function commonTests() { describe('shouldCoalesceRunChangeDetection = true', () => { let nativeRequestAnimationFrame: (fn: FrameRequestCallback) => void; + let nativeSetTimeout: any = (global as any)[Zone.__symbol__('setTimeout')]; if (!(global as any).requestAnimationFrame) { nativeRequestAnimationFrame = function(fn: Function) { (global as any)[Zone.__symbol__('setTimeout')](fn, 16); @@ -1139,6 +1167,20 @@ function commonTests() { }); }); + it('should only emit onMicroTaskEmpty once within the same event loop for ngZone.run in onMicrotaskEmpty subscription', + (done: DoneFn) => { + coalesceZone.onMicrotaskEmpty.subscribe(() => { + coalesceZone.run(() => {}); + }); + coalesceZone.run(() => {}); + coalesceZone.run(() => {}); + expect(logs).toEqual([]); + nativeSetTimeout(() => { + expect(logs).toEqual(['microTask empty']); + done(); + }, 100); + }); + it('should only emit onMicroTaskEmpty once within the same event loop for multiple tasks', (done: DoneFn) => { const tasks: Task[] = [];