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 angular#53844 fixes angular#53841 fixes angular#52610 fixes angular#53566 fixes angular#52940 fixes angular#51970 fixes angular#51768 fixes angular#50702 fixes angular#50259 fixes angular#50266 fixes angular#50160 fixes angular#49940 fixes angular#49398 fixes angular#48890 fixes angular#48608 fixes angular#45105 fixes angular#42241 fixes angular#41553 fixes angular#37223 fixes angular#37062 fixes angular#35579 fixes angular#31695 fixes angular#24728 fixes angular#23697 fixes angular#19814 fixes angular#13957 fixes angular#11565 fixes angular#15770 fixes angular#15946 fixes angular#18254 fixes angular#19731 fixes angular#20112 fixes angular#22472 fixes angular#23697 fixes angular#24727 fixes angular#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`.
atscott · Apr 8, 2024 · 8dcdde6 · 8dcdde6
1 parent 07487c8
commit 8dcdde6
Show file tree

Hide file tree

Showing 7 changed files with 13 additions and 94 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
@@ -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';
@@ -22,8 +21,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;
 
@@ -34,13 +32,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 */);
       }
     });
   }
@@ -90,14 +82,11 @@ export function internalProvideZoneChangeDetection(
       }
     },
     {provide: INTERNAL_APPLICATION_ERROR_HANDLER, useFactory: ngZoneApplicationErrorHandlerFactory},
-    // Always disable scheduler whenever explicitly disabled, even if Hybrid was specified elsewhere
+    // Always disable scheduler whenever explicitly disabled, even if another place called `provideZoneChangeDetection` without the 'ignore' option.
     ignoreChangesOutsideZone === true ?
         {provide: ZONELESS_SCHEDULER_DISABLED, useValue: true} :
         [],
-    // Only provide scheduler when explicitly enabled (by not disabling it)
-    ignoreChangesOutsideZone === false ?
-        {provide: ChangeDetectionScheduler, useExisting: ChangeDetectionSchedulerImpl} :
-        [],
+    {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
@@ -27,4 +27,5 @@ export const ZONELESS_ENABLED = new InjectionToken<boolean>(
     {providedIn: 'root', factory: () => false});
 
 export const ZONELESS_SCHEDULER_DISABLED = new InjectionToken<boolean>(
-    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
@@ -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
@@ -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
@@ -520,7 +520,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');
@@ -539,8 +542,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
@@ -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<boolean> {
-    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);
 

diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts
@@ -541,66 +541,6 @@ for (const browserAPI of ['navigation', 'history'] as const) {
       ));
     });
 
-    describe('navigation warning', () => {
-      const isInAngularZoneFn = NgZone.isInAngularZone;
-      let warnings: string[] = [];
-      let isInAngularZone = true;
-
-      class MockConsole {
-        warn(message: string) {
-          warnings.push(message);
-        }
-      }
-
-      beforeEach(() => {
-        warnings = [];
-        isInAngularZone = true;
-        NgZone.isInAngularZone = () => isInAngularZone;
-        TestBed.overrideProvider(Console, {useValue: new MockConsole()});
-      });
-
-      afterEach(() => {
-        NgZone.isInAngularZone = isInAngularZoneFn;
-      });
-
-      describe('with NgZone enabled', () => {
-        it('should warn when triggered outside Angular zone', fakeAsync(
-          inject([Router], (router: Router) => {
-            isInAngularZone = false;
-            router.navigateByUrl('/simple');
-
-            expect(warnings.length).toBe(1);
-            expect(warnings[0]).toBe(
-              `Navigation triggered outside Angular zone, did you forget to call 'ngZone.run()'?`,
-            );
-          }),
-        ));
-
-        it('should not warn when triggered inside Angular zone', fakeAsync(
-          inject([Router], (router: Router) => {
-            router.navigateByUrl('/simple');
-
-            expect(warnings.length).toBe(0);
-          }),
-        ));
-      });
-
-      describe('with NgZone disabled', () => {
-        beforeEach(() => {
-          TestBed.overrideProvider(NgZone, {useValue: new NoopNgZone()});
-        });
-
-        it('should not warn when triggered outside Angular zone', fakeAsync(
-          inject([Router], (router: Router) => {
-            isInAngularZone = false;
-            router.navigateByUrl('/simple');
-
-            expect(warnings.length).toBe(0);
-          }),
-        ));
-      });
-    });
-
     describe('should execute navigations serially', () => {
       let log: Array<string | Params> = [];