From 712d8ca91f504cc76590113828417122d9003429 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 28 Mar 2024 12:25:48 -0700 Subject: [PATCH] fix(core): Angular should not ignore changes that happen outside the zone When Angular receives a clear indication that change detection should run again, this should not be ignored, regardless of what Zone it happened in. This change updates the default change detection scheduling approach of Zone-based applications to ensure a change detection will run when these events happen outside the Angular zone (which includes, for example, updating a signal that's read in a template, setting an input of a `ComponentRef`, attaching a view marked for check, calling `ChangeDetectorRef.markForCheck`, etc.). This does not apply to applications using `NoopNgZone` or those which have a custom `NgZone` implementation without ZoneJS. The impact of this change will most often be seen in existing unit tests. Tests execute outside the Angular Zone and this can mean that state in the test is not fully recognized by Angular. Now that Angular will ensure change detection _does_ run, even when the state update originates from outside the zone, tests may observe additional rounds of change detection compared to the previous behavior. Often, this should be seen as more correct and the test should be updated, but in cases where it is too much effort to debug, the test can revert to the old behavior by adding `provideZoneChangeDetection({schedulingMode: NgZoneSchedulingMode.NgZoneOnly})` to the `TestBed` providers. fixes #55238 fixes #53844 fixes #53841 fixes #52610 fixes #53566 fixes #52940 fixes #51970 fixes #51768 fixes #50702 fixes #50259 fixes #50266 fixes #50160 fixes #49940 fixes #49398 fixes #48890 fixes #48608 fixes #45105 fixes #42241 fixes #41553 fixes #37223 fixes #37062 fixes #35579 fixes #31695 fixes #24728 fixes #23697 fixes #19814 fixes #13957 fixes #11565 fixes #15770 fixes #15946 fixes #18254 fixes #19731 fixes #20112 fixes #22472 fixes #23697 fixes #24727 fixes #47236 BREAKING CHANGE: Angular will ensure change detection runs, even when the state update originates from outside the zone, tests may observe additional rounds of change detection compared to the previous behavior. This change will be more likely to impact existing unit tests. This should usually be seen as more correct and the test should be updated, but in cases where it is too much effort to debug, the test can revert to the old behavior by adding `provideZoneChangeDetection({schedulingMode: NgZoneSchedulingMode.NgZoneOnly})` to the `TestBed` providers. Similarly, applications which may want to update state outside the zone and _not_ trigger change detection can add `provideZoneChangeDetection({schedulingMode: NgZoneSchedulingMode.NgZoneOnly})` to the providers in `bootstrapApplication` or add `schedulingMode: NgZoneSchedulingMode.NgZoneOnly` to the `BootstrapOptions` of `bootstrapModule`. --- .../scheduling/ng_zone_scheduling.ts | 21 ++++++------------- .../scheduling/zoneless_scheduling.ts | 3 ++- .../scheduling/zoneless_scheduling_impl.ts | 3 +-- packages/core/test/application_ref_spec.ts | 3 ++- .../test/change_detection_scheduler_spec.ts | 7 ++++--- packages/router/src/router.ts | 12 ----------- 6 files changed, 15 insertions(+), 34 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 19db8d4a43416f..ba25726ff10c18 100644 --- a/packages/core/src/change_detection/scheduling/ng_zone_scheduling.ts +++ b/packages/core/src/change_detection/scheduling/ng_zone_scheduling.ts @@ -8,7 +8,6 @@ import {Subscription} from 'rxjs'; -import {ApplicationRef} from '../../application/application_ref'; import {ENVIRONMENT_INITIALIZER, EnvironmentProviders, inject, Injectable, InjectionToken, makeEnvironmentProviders, StaticProvider} from '../../di'; import {ErrorHandler, INTERNAL_APPLICATION_ERROR_HANDLER} from '../../error_handler'; import {RuntimeError, RuntimeErrorCode} from '../../errors'; @@ -23,8 +22,7 @@ import {ChangeDetectionSchedulerImpl} from './zoneless_scheduling_impl'; @Injectable({providedIn: 'root'}) export class NgZoneChangeDetectionScheduler { private readonly zone = inject(NgZone); - private readonly changeDetectionScheduler = inject(ChangeDetectionScheduler, {optional: true}); - private readonly applicationRef = inject(ApplicationRef); + private readonly changeDetectionScheduler = inject(ChangeDetectionScheduler); private _onMicrotaskEmptySubscription?: Subscription; @@ -35,13 +33,7 @@ export class NgZoneChangeDetectionScheduler { this._onMicrotaskEmptySubscription = this.zone.onMicrotaskEmpty.subscribe({ next: () => { - this.zone.run(() => { - if (this.changeDetectionScheduler) { - this.changeDetectionScheduler.tick(true /* shouldRefreshViews */); - } else { - this.applicationRef.tick(); - } - }); + this.changeDetectionScheduler.tick(true /* shouldRefreshViews */); } }); } @@ -91,12 +83,11 @@ export function internalProvideZoneChangeDetection( } }, {provide: INTERNAL_APPLICATION_ERROR_HANDLER, useFactory: ngZoneApplicationErrorHandlerFactory}, - // Always disable scheduler whenever explicitly disabled, even if Hybrid was specified elsewhere - ignoreChangesOutsideZone === true ? {provide: ZONELESS_SCHEDULER_DISABLED, useValue: true} : [], - // Only provide scheduler when explicitly not disabled - ignoreChangesOutsideZone === false ? - {provide: ChangeDetectionScheduler, useExisting: ChangeDetectionSchedulerImpl} : + // Always disable scheduler whenever explicitly disabled, even if another place called `provideZoneChangeDetection` without the 'ignore' option. + ignoreChangesOutsideZone === true ? + {provide: ZONELESS_SCHEDULER_DISABLED, useValue: true} : [], + {provide: ChangeDetectionScheduler, useExisting: ChangeDetectionSchedulerImpl}, ]; } diff --git a/packages/core/src/change_detection/scheduling/zoneless_scheduling.ts b/packages/core/src/change_detection/scheduling/zoneless_scheduling.ts index bee9e7c4a0464c..2588e6a4b8bc07 100644 --- a/packages/core/src/change_detection/scheduling/zoneless_scheduling.ts +++ b/packages/core/src/change_detection/scheduling/zoneless_scheduling.ts @@ -27,4 +27,5 @@ export const ZONELESS_ENABLED = new InjectionToken( {providedIn: 'root', factory: () => false}); export const ZONELESS_SCHEDULER_DISABLED = new InjectionToken( - typeof ngDevMode === 'undefined' || ngDevMode ? 'scheduler disabled' : ''); + typeof ngDevMode === 'undefined' || ngDevMode ? 'scheduler disabled' : '', + {factory: () => false}); 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 2b11de7c5f8179..56642a077e04d7 100644 --- a/packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts +++ b/packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts @@ -28,8 +28,7 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler { private runningTick = false; private cancelScheduledCallback: null|(() => void) = null; private readonly zonelessEnabled = inject(ZONELESS_ENABLED); - private readonly disableScheduling = - inject(ZONELESS_SCHEDULER_DISABLED, {optional: true}) ?? false; + private readonly disableScheduling = inject(ZONELESS_SCHEDULER_DISABLED); private readonly zoneIsDefined = typeof Zone !== 'undefined'; constructor() { diff --git a/packages/core/test/application_ref_spec.ts b/packages/core/test/application_ref_spec.ts index 268d7b982d308c..e69dc78b38175b 100644 --- a/packages/core/test/application_ref_spec.ts +++ b/packages/core/test/application_ref_spec.ts @@ -8,7 +8,7 @@ import {DOCUMENT, ɵgetDOM as getDOM} from '@angular/common'; import {ResourceLoader} from '@angular/compiler'; -import {APP_BOOTSTRAP_LISTENER, APP_INITIALIZER, ChangeDetectionStrategy, Compiler, CompilerFactory, Component, EnvironmentInjector, InjectionToken, LOCALE_ID, NgModule, NgZone, PlatformRef, RendererFactory2, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/core'; +import {APP_BOOTSTRAP_LISTENER, APP_INITIALIZER, ChangeDetectionStrategy, Compiler, CompilerFactory, Component, EnvironmentInjector, InjectionToken, LOCALE_ID, NgModule, NgZone, PlatformRef, provideZoneChangeDetection, RendererFactory2, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/core'; import {ErrorHandler} from '@angular/core/src/error_handler'; import {ComponentRef} from '@angular/core/src/linker/component_factory'; import {createEnvironmentInjector, getLocaleId} from '@angular/core/src/render3'; @@ -771,6 +771,7 @@ describe('AppRef', () => { beforeEach(() => { stableCalled = false; TestBed.configureTestingModule({ + providers: [provideZoneChangeDetection({ignoreChangesOutsideZone: true})], declarations: [ SyncComp, MicroTaskComp, MacroTaskComp, MicroMacroTaskComp, MacroMicroTaskComp, ClickComp ], diff --git a/packages/core/test/change_detection_scheduler_spec.ts b/packages/core/test/change_detection_scheduler_spec.ts index d344b8ee34efaf..dcdbd1f717fc4a 100644 --- a/packages/core/test/change_detection_scheduler_spec.ts +++ b/packages/core/test/change_detection_scheduler_spec.ts @@ -548,7 +548,10 @@ describe('Angular with scheduler and ZoneJS', () => { {providers: [{provide: ComponentFixtureAutoDetect, useValue: true}]}); }); - it('current default behavior requires updates inside Angular zone', async () => { + it('requires updates inside Angular zone when using ngZoneOnly', async () => { + TestBed.configureTestingModule({ + providers: [provideZoneChangeDetection({ignoreChangesOutsideZone: true})] + }); @Component({template: '{{thing()}}', standalone: true}) class App { thing = signal('initial'); @@ -567,8 +570,6 @@ describe('Angular with scheduler and ZoneJS', () => { }); it('updating signal outside of zone still schedules update when in hybrid mode', async () => { - TestBed.configureTestingModule( - {providers: [provideZoneChangeDetection({ignoreChangesOutsideZone: false})]}); @Component({template: '{{thing()}}', standalone: true}) class App { thing = signal('initial'); diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts index a3726b830ba72b..665eab460ee527 100644 --- a/packages/router/src/router.ts +++ b/packages/router/src/router.ts @@ -10,7 +10,6 @@ import {Location} from '@angular/common'; import { inject, Injectable, - NgZone, Type, ɵConsole as Console, ɵPendingTasks as PendingTasks, @@ -106,7 +105,6 @@ export class Router { } private disposed = false; private nonRouterCurrentEntryChangeSubscription?: SubscriptionLike; - private isNgZoneEnabled = false; private readonly console = inject(Console); private readonly stateManager = inject(StateManager); @@ -186,8 +184,6 @@ export class Router { readonly componentInputBindingEnabled: boolean = !!inject(INPUT_BINDER, {optional: true}); constructor() { - this.isNgZoneEnabled = inject(NgZone) instanceof NgZone && NgZone.isInAngularZone(); - this.resetConfig(this.config); this.navigationTransitions @@ -522,14 +518,6 @@ export class Router { skipLocationChange: false, }, ): Promise { - if (typeof ngDevMode === 'undefined' || ngDevMode) { - if (this.isNgZoneEnabled && !NgZone.isInAngularZone()) { - this.console.warn( - `Navigation triggered outside Angular zone, did you forget to call 'ngZone.run()'?`, - ); - } - } - const urlTree = isUrlTree(url) ? url : this.parseUrl(url); const mergedTree = this.urlHandlingStrategy.merge(urlTree, this.rawUrlTree);