From fc6a7eea682a76f03e85c4786aed3b02bdb06968 Mon Sep 17 00:00:00 2001 From: arturovt Date: Sat, 11 Apr 2026 00:25:13 +0300 Subject: [PATCH] fix(zone.js): allow draining microtasks in `Promise.then` (through flag) These changes are essentially the same as those introduced in angular#45273, but they include backward compatibility for applications that explicitly rely on the order in which microtasks are drained. 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. The simple code that reproduces the behavior that exists now: ```ts 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 }); ``` The code above logs 400 as the height, but it should actually log 200 because the height is updated in a microtask within the event listener. When using Angular with microfrontend applications, especially when other apps might be using React, zone.js can disrupt the classical order of operations. For example, when using a `react-component/trigger`, it schedules a microtask within an event listener using `Promise.resolve().then(...)` to determine whether the event needs to be re-dispatched. The event is re-dispatched when the layout has changed, which is why a microtask is used. With this change, we introduce a global configuration flag, `__zone_symbol__enable_native_microtask_draining`, to allow consumers to enable microtask draining within a browser microtask. This flag is necessary to prevent any breaking changes resulting from this modification. The previous attempt to address this issue caused a significant number of failures in g3. Therefore, we are hiding that fix behind the configuration flag. Closes angular#44446 Closes angular#55590 Closes angular#51328 --- packages/zone.js/lib/zone-impl.ts | 94 ++++++----- .../zone.js/lib/zone.configurations.api.ts | 53 +++++++ packages/zone.js/test/common/zone.spec.ts | 150 ++++++++++++++++++ 3 files changed, 259 insertions(+), 38 deletions(-) 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