diff --git a/packages/zone.js/lib/zone-impl.ts b/packages/zone.js/lib/zone-impl.ts index 04a914d4ef9..4465bbaadeb 100644 --- a/packages/zone.js/lib/zone-impl.ts +++ b/packages/zone.js/lib/zone-impl.ts @@ -1131,8 +1131,6 @@ export function initZone(): ZoneType { 'eventTask': 0, }; - private _parentDelegate: _ZoneDelegate | null; - private _forkDlgt: _ZoneDelegate | null; private _forkZS: ZoneSpec | null; private _forkCurrZone: Zone | null; @@ -1168,7 +1166,6 @@ export function initZone(): ZoneType { constructor(zone: Zone, parentDelegate: _ZoneDelegate | null, zoneSpec: ZoneSpec | null) { this._zone = zone as ZoneImpl; - this._parentDelegate = parentDelegate; this._forkZS = zoneSpec && (zoneSpec && zoneSpec.onFork ? zoneSpec : parentDelegate!._forkZS); this._forkDlgt = zoneSpec && (zoneSpec.onFork ? parentDelegate : parentDelegate!._forkDlgt); @@ -1438,8 +1435,8 @@ export function initZone(): ZoneType { task.runCount++; return task.zone.runTask(task, target, args); } finally { - if (_numberOfNestedTaskFrames == 1) { - drainMicroTaskQueue(); + if (_numberOfNestedTaskFrames === 1 && !global[enableNativeMicrotaskDraining]) { + drainMicroTaskQueueSynchronously(); } _numberOfNestedTaskFrames--; } @@ -1503,54 +1500,75 @@ export function initZone(): ZoneType { const symbolSetTimeout = __symbol__('setTimeout'); const symbolPromise = __symbol__('Promise'); const symbolThen = __symbol__('then'); + // To prevent any breaking changes resulting from this change, given that + // it was already causing a significant number of failures in g3, we have hidden + // that behavior behind a global configuration flag. Consumers can enable this + // flag explicitly if they want the microtask queue to be drained as defined + // in the specification. + const enableNativeMicrotaskDraining = __symbol__('enable_native_microtask_draining'); let _microTaskQueue: Task[] = []; - let _isDrainingMicrotaskQueue: boolean = false; + let _isDrainingMicrotaskQueue = false; let nativeMicroTaskQueuePromise: any; function nativeScheduleMicroTask(func: Function) { - if (!nativeMicroTaskQueuePromise) { - if (global[symbolPromise]) { - nativeMicroTaskQueuePromise = global[symbolPromise].resolve(0); - } + if (!nativeMicroTaskQueuePromise && global[symbolPromise]) { + nativeMicroTaskQueuePromise = global[symbolPromise].resolve(0); } + if (nativeMicroTaskQueuePromise) { - let nativeThen = nativeMicroTaskQueuePromise[symbolThen]; - if (!nativeThen) { - // native Promise is not patchable, we need to use `then` directly - // issue 1078 - nativeThen = nativeMicroTaskQueuePromise['then']; - } - nativeThen.call(nativeMicroTaskQueuePromise, func); + const thenFn = nativeMicroTaskQueuePromise[symbolThen] ?? nativeMicroTaskQueuePromise['then']; // fallback for non-patchable Promise + // Use the resolved native promise to schedule the microtask + thenFn.call(nativeMicroTaskQueuePromise, func); } else { + // Fallback to setTimeout if native promise is unavailable global[symbolSetTimeout](func, 0); } } function scheduleMicroTask(task?: MicroTask) { - // if we are not running in any task, and there has not been anything scheduled - // we must bootstrap the initial task creation by manually scheduling the drain - if (_numberOfNestedTaskFrames === 0 && _microTaskQueue.length === 0) { - // We are not running in Task, so we need to kickstart the microtask queue. - nativeScheduleMicroTask(drainMicroTaskQueue); + const isNativeDrainingEnabled = global[enableNativeMicrotaskDraining]; + const shouldDrainWithNative = + isNativeDrainingEnabled && _microTaskQueue.length === 0 && !_isDrainingMicrotaskQueue; + const shouldDrainWithoutNative = + !isNativeDrainingEnabled && _numberOfNestedTaskFrames === 0 && _microTaskQueue.length === 0; + + if (shouldDrainWithNative || shouldDrainWithoutNative) { + // Start draining the microtask queue if: + // - Native draining is enabled and not currently in progress, or + // - Native draining is disabled, and there are no nested tasks and no queued microtasks. + nativeScheduleMicroTask(drainMicroTaskQueueSynchronously); + } + + if (task) { + _microTaskQueue.push(task); } - task && _microTaskQueue.push(task); } - function drainMicroTaskQueue() { - if (!_isDrainingMicrotaskQueue) { - _isDrainingMicrotaskQueue = true; - while (_microTaskQueue.length) { - const queue = _microTaskQueue; - _microTaskQueue = []; - for (let i = 0; i < queue.length; i++) { - const task = queue[i]; - try { - task.zone.runTask(task, null, null); - } catch (error) { - _api.onUnhandledError(error as Error); - } + function drainMicroTaskQueueSynchronously() { + if (_isDrainingMicrotaskQueue) { + return; + } + + _isDrainingMicrotaskQueue = true; + + while (_microTaskQueue.length) { + const queue = _microTaskQueue; + _microTaskQueue = []; + + for (const task of queue) { + try { + task.zone.runTask(task, null, null); + } catch (error) { + _api.onUnhandledError(error as Error); } } + } + + // The order matters! + if (global[enableNativeMicrotaskDraining]) { + _isDrainingMicrotaskQueue = false; + _api.microtaskDrainDone(); + } else { _api.microtaskDrainDone(); _isDrainingMicrotaskQueue = false; } @@ -1579,7 +1597,7 @@ export function initZone(): ZoneType { currentZoneFrame: () => _currentZoneFrame, onUnhandledError: noop, microtaskDrainDone: noop, - scheduleMicroTask: scheduleMicroTask, + scheduleMicroTask, showUncaughtError: () => !(ZoneImpl as any)[__symbol__('ignoreConsoleErrorUncaughtError')], patchEventTarget: () => [], patchOnProperties: noop, @@ -1599,7 +1617,7 @@ export function initZone(): ZoneType { attachOriginToPatched: () => noop, _redefineProperty: () => noop, patchCallbacks: () => noop, - nativeScheduleMicroTask: nativeScheduleMicroTask, + nativeScheduleMicroTask, }; let _currentZoneFrame: ZoneFrame = {parent: null, zone: new ZoneImpl(null, null)}; let _currentTask: Task | null = null; diff --git a/packages/zone.js/lib/zone.configurations.api.ts b/packages/zone.js/lib/zone.configurations.api.ts index 21a18b29793..d68589d5c4c 100644 --- a/packages/zone.js/lib/zone.configurations.api.ts +++ b/packages/zone.js/lib/zone.configurations.api.ts @@ -555,6 +555,59 @@ declare global { * the user with a string returned from the event handler. */ __zone_symbol__enable_beforeunload?: boolean; + + /** + * https://github.com/angular/angular/issues/41506 + * https://github.com/angular/angular/issues/44446 + * + * By default, `zone.js` maintains a microtask queue manually, which means the microtask + * queue is drained whenever `zone.js` decides to do so under certain circumstances. + * Typically, `zone.js` invokes a task (e.g., an event task) and, after invoking the task, + * checks whether the number of nested task frames is equal to 1 before calling the microtask + * queue draining. + * As thus, there are cases when the microtask queue may be drained synchronously after an + * event task is invoked (if it’s the very first task in the call stack). + * Tasks may actually schedule other tasks, thereby incrementing the stack frame. + * In that case, the microtask queue might be drained after the last task is invoked. + * + * Given that code: + * ```js + * Zone.current.fork({name: 'child'}).run(() => { + * const div = document.createElement('div'); + * div.style.height = '200px'; + * div.style.width = '200px'; + * div.style.backgroundColor = 'red'; + * document.body.appendChild(div); + * + * function listener() { + * Promise.resolve().then(() => { + * div.style.height = '400px'; + * }); + * } + * + * div.addEventListener('fakeEvent', listener); + * div.dispatchEvent(new Event('fakeEvent')); + * console.log(div.getBoundingClientRect().height); // 400 + * }); + * ``` + * + * We would assume that "200" would be logged. However, with `zone.js`, "400" will + * be logged first because it drains the microtask queue too early, as the `fakeEvent` + * event task is the very top task on the stack. + * + * https://promisesaplus.com/#the-then-method + * According to the spec: `onFulfilled` or `onRejected` must not be called until the + * execution context stack contains only platform code. + * + * You may consider enabling the flag below. This will ensure that microtask draining + * does not happen synchronously and always occurs within a browser microtask. + * + * This is critically important for our code and other third-party code, which is + * beyond our control, to work properly. If a microtask is scheduled within an event + * listener to be executed "later", it should indeed be executed later and not synchronously, + * as this would break the expected flow of code execution. + */ + __zone_symbol__enable_native_microtask_draining?: boolean; } /** diff --git a/packages/zone.js/test/common/zone.spec.ts b/packages/zone.js/test/common/zone.spec.ts index 6d16fcbf8ea..41e51651618 100644 --- a/packages/zone.js/test/common/zone.spec.ts +++ b/packages/zone.js/test/common/zone.spec.ts @@ -5,6 +5,9 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.dev/license */ + +import {isNode} from '../../lib/common/utils'; + describe('Zone Common', function () { it('should have a name', function () { expect(Zone.current.name).toBeDefined(); @@ -432,6 +435,153 @@ describe('Zone Common', function () { log = []; }); + // https://github.com/angular/angular/issues/44446 + // https://github.com/angular/angular/issues/55590 + // https://github.com/angular/angular/issues/51328 + describe('__zone_symbol__enable_native_microtask_draining', () => { + it('should not drain the microtask queue too early without task (if the flag is enabled)', (done) => { + // Regression test for https://github.com/angular/angular/issues/44446. + // Verifies that a microtask scheduled inside an event task is not drained + // synchronously mid-stack when the native draining flag is enabled. + const globalAny = global as any; + globalAny[Zone.__symbol__('enable_native_microtask_draining')] = true; + const zone = Zone.current; + const event = zone.scheduleEventTask( + 'test', + () => { + log.push('eventTask'); + zone.scheduleMicroTask('test', () => log.push('microTask')); + }, + undefined, + noop, + noop, + ); + log.push('after schedule eventTask'); + expect(log).toEqual(['after schedule eventTask']); + event.invoke(); + // At this point, we should not have invoked the microtask. + expect(log).toEqual(['after schedule eventTask', 'eventTask']); + globalAny[Zone.__symbol__('setTimeout')](() => { + expect(log).toEqual(['after schedule eventTask', 'eventTask', 'microTask']); + globalAny[Zone.__symbol__('enable_native_microtask_draining')] = false; + done(); + }); + }); + + it('should not drain the microtask queue too early (if the flag is enabled)', (done) => { + // We need `document` in this test. + if (isNode) { + done(); + return; + } + + // Regression test for https://github.com/angular/angular/issues/44446. + // Verifies that a Promise.then() callback scheduled inside a DOM event listener + // is not drained synchronously before the main stack unwinds. + + const globalAny = global as any; + globalAny[Zone.__symbol__('enable_native_microtask_draining')] = true; + const zone = Zone.current; + + zone.run(() => { + const listener = () => { + Promise.resolve().then(() => log.push('promise.then')); + }; + + document.body.addEventListener('click', listener); + document.body.click(); + log.push('main stack'); + + globalAny[Zone.__symbol__('setTimeout')](() => { + document.body.removeEventListener('click', listener); + expect(log).toEqual(['main stack', 'promise.then']); + globalAny[Zone.__symbol__('enable_native_microtask_draining')] = false; + done(); + }); + }); + }); + + it('should surface unhandled promise rejections via unhandledrejection event (if the flag is enabled)', async () => { + // We need `window` in this test. + if (isNode) { + return; + } + + // Regression test for https://github.com/angular/angular/issues/55590. + // Verifies that unhandled promise rejections originating outside zone.js-patched + // code (e.g. a plain