Skip to content

Commit

Permalink
fix(core): Angular should not ignore changes that happen outside the …
Browse files Browse the repository at this point in the history 
…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 #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`.
  • Loading branch information
@atscott
atscott committed Apr 8, 2024
1 parent 07487c8 commit 8dcdde6
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 94 deletions.
19 changes: 4 additions & 15 deletions packages/core/src/change_detection/scheduling/ng_zone_scheduling.ts
View file
Expand Up @@ -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';
Expand All @@ -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;

Expand All @@ -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 */);
}
});
}
Expand Down Expand Up @@ -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},
];
}

Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/change_detection/scheduling/zoneless_scheduling.ts
View file
Expand Up @@ -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});
3 changes: 1 addition & 2 deletions packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts
View file
Expand Up @@ -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() {
Expand Down
3 changes: 2 additions & 1 deletion packages/core/test/application_ref_spec.ts
View file
Expand Up @@ -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';
Expand Down Expand Up @@ -771,6 +771,7 @@ describe('AppRef', () => {
beforeEach(() => {
stableCalled = false;
TestBed.configureTestingModule({
providers: [provideZoneChangeDetection({ignoreChangesOutsideZone: true})],
declarations: [
SyncComp, MicroTaskComp, MacroTaskComp, MicroMacroTaskComp, MacroMicroTaskComp, ClickComp
],
Expand Down
7 changes: 4 additions & 3 deletions packages/core/test/change_detection_scheduler_spec.ts
View file
Expand Up @@ -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');
Expand All @@ -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');
Expand Down
12 changes: 0 additions & 12 deletions packages/router/src/router.ts
View file
Expand Up @@ -10,7 +10,6 @@ import {Location} from '@angular/common';
import {
inject,
Injectable,
NgZone,
Type,
ɵConsole as Console,
ɵPendingTasks as PendingTasks,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down
60 changes: 0 additions & 60 deletions packages/router/test/integration.spec.ts
View file
Expand Up @@ -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> = [];

Expand Down

0 comments on commit 8dcdde6

Please sign in to comment.