From d15cde12c8f749edc4ff80658a1b07fe2fab00c5 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 15 Aug 2024 13:14:07 -0700 Subject: [PATCH] fix(core): Ensure ComponentFixture.detectChanges causes synchronous ApplicationRef.tick When ZoneJS is enabled, `ComponentFixture.detectChanges` _may_ cause a synchronous `ApplicationRef.tick` if the zone is stable after the `ngZone.run` call. If the zone is not stable, `ApplicationRef.tick` will not happen until it `onMicrotaskEmpty` emits. Render hooks execute within `ApplicationRef.tick` so it is important that these are predictably run when calling `ComponentFixture.detectChanges`. This change completes the mering of behaviors between zone-based and zoneless fixtures. The only difference now is the default value of autoDetect, which is true for zoneless and false for zones. --- .../scheduling/ng_zone_scheduling.ts | 9 ++---- .../scheduling/zoneless_scheduling_impl.ts | 21 ++++++-------- packages/core/src/core_private_export.ts | 1 + packages/core/src/zone/ng_zone.ts | 18 ++++++++++++ .../core/testing/src/component_fixture.ts | 29 ++++++++----------- 5 files changed, 42 insertions(+), 36 deletions(-) diff --git a/packages/core/src/change_detection/scheduling/ng_zone_scheduling.ts b/packages/core/src/change_detection/scheduling/ng_zone_scheduling.ts index 99e9a28aeebdd7..90c22fddcddec4 100644 --- a/packages/core/src/change_detection/scheduling/ng_zone_scheduling.ts +++ b/packages/core/src/change_detection/scheduling/ng_zone_scheduling.ts @@ -22,10 +22,9 @@ import {RuntimeError, RuntimeErrorCode} from '../../errors'; import {PendingTasks} from '../../pending_tasks'; import {performanceMarkFeature} from '../../util/performance'; import {NgZone} from '../../zone'; -import {InternalNgZoneOptions} from '../../zone/ng_zone'; +import {shouldTickOnMicrotaskEmpty, InternalNgZoneOptions} from '../../zone/ng_zone'; import { - ChangeDetectionScheduler, ZONELESS_SCHEDULER_DISABLED, ZONELESS_ENABLED, SCHEDULE_IN_ROOT_ZONE, @@ -35,7 +34,6 @@ import {SCHEDULE_IN_ROOT_ZONE_DEFAULT} from './flags'; @Injectable({providedIn: 'root'}) export class NgZoneChangeDetectionScheduler { private readonly zone = inject(NgZone); - private readonly changeDetectionScheduler = inject(ChangeDetectionScheduler); private readonly applicationRef = inject(ApplicationRef); private _onMicrotaskEmptySubscription?: Subscription; @@ -47,10 +45,7 @@ export class NgZoneChangeDetectionScheduler { this._onMicrotaskEmptySubscription = this.zone.onMicrotaskEmpty.subscribe({ next: () => { - // `onMicroTaskEmpty` can happen _during_ the zoneless scheduler change detection because - // zone.run(() => {}) will result in `checkStable` at the end of the `zone.run` closure - // and emit `onMicrotaskEmpty` synchronously if run coalsecing is false. - if (this.changeDetectionScheduler.runningTick) { + if (!shouldTickOnMicrotaskEmpty()) { return; } this.zone.run(() => { diff --git a/packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts b/packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts index ade6e448da0dd1..005d71e425e2a1 100644 --- a/packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts +++ b/packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts @@ -20,7 +20,13 @@ import { scheduleCallbackWithRafRace, } from '../../util/callback_scheduler'; import {performanceMarkFeature} from '../../util/performance'; -import {NgZone, NgZonePrivate, NoopNgZone, angularZoneInstanceIdProperty} from '../../zone/ng_zone'; +import { + NgZone, + NgZonePrivate, + NoopNgZone, + angularZoneInstanceIdProperty, + runTickInZoneAndPreventDuplicate, +} from '../../zone/ng_zone'; import { ChangeDetectionScheduler, @@ -203,23 +209,14 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler { * render hooks when `false`. */ private tick(shouldRefreshViews: boolean): void { - // When ngZone.run below exits, onMicrotaskEmpty may emit if the zone is - // stable. We want to prevent double ticking so we track whether the tick is - // already running and skip it if so. if (this.runningTick || this.appRef.destroyed) { return; } const task = this.taskService.add(); try { - this.ngZone.run( - () => { - this.runningTick = true; - this.appRef._tick(shouldRefreshViews); - }, - undefined, - this.schedulerTickApplyArgs, - ); + this.runningTick = true; + runTickInZoneAndPreventDuplicate(this.ngZone, this.appRef); } catch (e: unknown) { this.taskService.remove(task); throw e; diff --git a/packages/core/src/core_private_export.ts b/packages/core/src/core_private_export.ts index d462ba8ccd018c..2e570cb2d3c4de 100644 --- a/packages/core/src/core_private_export.ts +++ b/packages/core/src/core_private_export.ts @@ -11,6 +11,7 @@ export { detectChangesInViewIfRequired as ɵdetectChangesInViewIfRequired, whenStable as ɵwhenStable, } from './application/application_ref'; +export {runTickInZoneAndPreventDuplicate as ɵrunAndPreventDuplicateTick} from './zone/ng_zone'; export {INTERNAL_APPLICATION_ERROR_HANDLER as ɵINTERNAL_APPLICATION_ERROR_HANDLER} from './error_handler'; export { IMAGE_CONFIG as ɵIMAGE_CONFIG, diff --git a/packages/core/src/zone/ng_zone.ts b/packages/core/src/zone/ng_zone.ts index b806edb919599c..f894c0a885b0b6 100644 --- a/packages/core/src/zone/ng_zone.ts +++ b/packages/core/src/zone/ng_zone.ts @@ -7,6 +7,7 @@ */ import {SCHEDULE_IN_ROOT_ZONE_DEFAULT} from '../change_detection/scheduling/flags'; +import {ApplicationRef} from '../core'; import {RuntimeError, RuntimeErrorCode} from '../errors'; import {EventEmitter} from '../event_emitter'; import {scheduleCallbackWithRafRace} from '../util/callback_scheduler'; @@ -542,6 +543,23 @@ function onLeave(zone: NgZonePrivate) { checkStable(zone); } +// Used to prevent double ApplicationRef.tick when calling `ngZone.run(() => applicationRef.tick())` +let _tickOnMicrotaskEmpty = true; +export function shouldTickOnMicrotaskEmpty() { + return _tickOnMicrotaskEmpty; +} + +const schedulerTickApplyArgs = [{data: {'__scheduler_tick__': true}}]; +export function runTickInZoneAndPreventDuplicate(zone: NgZone, appRef: ApplicationRef) { + const previous = _tickOnMicrotaskEmpty; + _tickOnMicrotaskEmpty = false; + try { + zone.run(() => appRef.tick(), undefined, schedulerTickApplyArgs); + } finally { + _tickOnMicrotaskEmpty = previous; + } +} + /** * Provides a noop implementation of `NgZone` which does nothing. This zone requires explicit calls * to framework to perform rendering. diff --git a/packages/core/testing/src/component_fixture.ts b/packages/core/testing/src/component_fixture.ts index 25a7d80993ff12..c229f3c06428c6 100644 --- a/packages/core/testing/src/component_fixture.ts +++ b/packages/core/testing/src/component_fixture.ts @@ -25,6 +25,7 @@ import { ɵNoopNgZone as NoopNgZone, ɵZONELESS_ENABLED as ZONELESS_ENABLED, ɵPendingTasks as PendingTasks, + ɵrunAndPreventDuplicateTick, } from '@angular/core'; import {Subscription} from 'rxjs'; @@ -34,7 +35,6 @@ import {TestBedApplicationErrorHandler} from './application_error_handler'; interface TestAppRef { externalTestViews: Set; - skipCheckNoChangesForExternalTestViews: Set; } /** @@ -132,27 +132,22 @@ export class ComponentFixture { */ detectChanges(checkNoChanges = true): void { this._effectRunner.flush(); + const originalCheckNoChanges = this.componentRef.changeDetectorRef.checkNoChanges; - try { - if (!checkNoChanges) { - this.componentRef.changeDetectorRef.checkNoChanges = () => {}; - } + if (!checkNoChanges) { + this.componentRef.changeDetectorRef.checkNoChanges = () => {}; + } - if (this.zonelessEnabled) { - this._appRef.tick(); - } else { - // Run the change detection inside the NgZone so that any async tasks as part of the change - // detection are captured by the zone and can be waited for in isStable. - // Run any effects that were created/dirtied during change detection. Such effects might become - // dirty in response to input signals changing. - this._ngZone.run(() => { - this.changeDetectorRef.detectChanges(); - this.checkNoChanges(); - }); - } + try { + this._testAppRef.externalTestViews.add(this.componentRef.hostView); + ɵrunAndPreventDuplicateTick(this._ngZone, this._appRef); } finally { + if (!this.autoDetect) { + this._testAppRef.externalTestViews.delete(this.componentRef.hostView); + } this.componentRef.changeDetectorRef.checkNoChanges = originalCheckNoChanges; } + this._effectRunner.flush(); }