From 769b6e197356817c05db144d7c3587ef17dc9575 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Mon, 5 Aug 2024 08:48:30 -0700 Subject: [PATCH] fix(core): Allow hybrid CD scheduling to support multiple "Angular zones" (#57267) This commit updates the inside/outside NgZone detection of the hybrid CD scheduling to track the actual instance of the NgZone being used rather than the name "Angular" (how `isInsideAngularZone` works). This allows the scheduling to work correctly when there are multiple versions of Angular running on the page. fixes #57261 PR Close #57267 --- .../scheduling/zoneless_scheduling_impl.ts | 11 +++- packages/core/src/zone/ng_zone.ts | 16 +++++- .../bundle.golden_symbols.json | 9 +++ .../animations/bundle.golden_symbols.json | 9 +++ .../cyclic_import/bundle.golden_symbols.json | 9 +++ .../bundling/defer/bundle.golden_symbols.json | 9 +++ .../forms_reactive/bundle.golden_symbols.json | 9 +++ .../bundle.golden_symbols.json | 9 +++ .../hello_world/bundle.golden_symbols.json | 9 +++ .../hydration/bundle.golden_symbols.json | 9 +++ .../router/bundle.golden_symbols.json | 9 +++ .../bundle.golden_symbols.json | 9 +++ .../bundling/todo/bundle.golden_symbols.json | 9 +++ .../test/change_detection_scheduler_spec.ts | 55 ++++++++++++++++++- 14 files changed, 175 insertions(+), 6 deletions(-) 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 9133327e4ec..dc25d6ba730 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,7 @@ import { scheduleCallbackWithRafRace, } from '../../util/callback_scheduler'; import {performanceMarkFeature} from '../../util/performance'; -import {NgZone, NoopNgZone} from '../../zone/ng_zone'; +import {NgZone, NgZonePrivate, NoopNgZone, angularZoneInstanceIdProperty} from '../../zone/ng_zone'; import { ChangeDetectionScheduler, @@ -64,6 +64,9 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler { private readonly zoneIsDefined = typeof Zone !== 'undefined' && !!Zone.root.run; private readonly schedulerTickApplyArgs = [{data: {'__scheduler_tick__': true}}]; private readonly subscriptions = new Subscription(); + private readonly angularZoneId = this.zoneIsDefined + ? (this.ngZone as NgZonePrivate)._inner?.get(angularZoneInstanceIdProperty) + : null; private cancelScheduledCallback: null | (() => void) = null; private shouldRefreshViews = false; @@ -176,7 +179,11 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler { } // If we're inside the zone don't bother with scheduler. Zone will stabilize // eventually and run change detection. - if (!this.zonelessEnabled && this.zoneIsDefined && NgZone.isInAngularZone()) { + if ( + !this.zonelessEnabled && + this.zoneIsDefined && + Zone.current.get(angularZoneInstanceIdProperty + this.angularZoneId) + ) { return false; } diff --git a/packages/core/src/zone/ng_zone.ts b/packages/core/src/zone/ng_zone.ts index 557d48824b9..cb74a4eba1a 100644 --- a/packages/core/src/zone/ng_zone.ts +++ b/packages/core/src/zone/ng_zone.ts @@ -18,6 +18,11 @@ import {AsyncStackTaggingZoneSpec} from './async-stack-tagging'; // ERROR - [JSC_UNDEFINED_VARIABLE] variable Zone is undeclared declare const Zone: any; +const isAngularZoneProperty = 'isAngularZone'; +export const angularZoneInstanceIdProperty = isAngularZoneProperty + '_ID'; + +let ngZoneInstanceId = 0; + /** * An injectable service for executing work inside or outside of the Angular zone. * @@ -173,7 +178,7 @@ export class NgZone { */ static isInAngularZone(): boolean { // Zone needs to be checked, because this method might be called even when NoopNgZone is used. - return typeof Zone !== 'undefined' && Zone.current.get('isAngularZone') === true; + return typeof Zone !== 'undefined' && Zone.current.get(isAngularZoneProperty) === true; } /** @@ -266,7 +271,7 @@ export class NgZone { const EMPTY_PAYLOAD = {}; -interface NgZonePrivate extends NgZone { +export interface NgZonePrivate extends NgZone { _outer: Zone; _inner: Zone; _nesting: number; @@ -394,9 +399,14 @@ function forkInnerZoneWithAngularBehavior(zone: NgZonePrivate) { const delayChangeDetectionForEventsDelegate = () => { delayChangeDetectionForEvents(zone); }; + const instanceId = ngZoneInstanceId++; zone._inner = zone._inner.fork({ name: 'angular', - properties: {'isAngularZone': true}, + properties: { + [isAngularZoneProperty]: true, + [angularZoneInstanceIdProperty]: instanceId, + [angularZoneInstanceIdProperty + instanceId]: true, + }, onInvokeTask: ( delegate: ZoneDelegate, current: Zone, diff --git a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json index bdf28033f4a..caa1cb9631c 100644 --- a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json @@ -653,6 +653,9 @@ { "name": "allocLFrame" }, + { + "name": "angularZoneInstanceIdProperty" + }, { "name": "animate" }, @@ -1118,6 +1121,9 @@ { "name": "invokeQuery" }, + { + "name": "isAngularZoneProperty" + }, { "name": "isApplicationBootstrapConfig" }, @@ -1259,6 +1265,9 @@ { "name": "ngOnChangesSetInput" }, + { + "name": "ngZoneInstanceId" + }, { "name": "nonNull" }, diff --git a/packages/core/test/bundling/animations/bundle.golden_symbols.json b/packages/core/test/bundling/animations/bundle.golden_symbols.json index a5066fab632..c7846d06cc9 100644 --- a/packages/core/test/bundling/animations/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations/bundle.golden_symbols.json @@ -710,6 +710,9 @@ { "name": "allocLFrame" }, + { + "name": "angularZoneInstanceIdProperty" + }, { "name": "animate" }, @@ -1187,6 +1190,9 @@ { "name": "invokeQuery" }, + { + "name": "isAngularZoneProperty" + }, { "name": "isApplicationBootstrapConfig" }, @@ -1325,6 +1331,9 @@ { "name": "ngOnChangesSetInput" }, + { + "name": "ngZoneInstanceId" + }, { "name": "noSideEffects" }, diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json index 60b371852de..0081a901321 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -536,6 +536,9 @@ { "name": "allocLFrame" }, + { + "name": "angularZoneInstanceIdProperty" + }, { "name": "appendChild" }, @@ -941,6 +944,9 @@ { "name": "invokeHostBindingsInCreationMode" }, + { + "name": "isAngularZoneProperty" + }, { "name": "isApplicationBootstrapConfig" }, @@ -1067,6 +1073,9 @@ { "name": "ngOnChangesSetInput" }, + { + "name": "ngZoneInstanceId" + }, { "name": "noSideEffects" }, diff --git a/packages/core/test/bundling/defer/bundle.golden_symbols.json b/packages/core/test/bundling/defer/bundle.golden_symbols.json index e566ff25bb3..1d498fb1c7c 100644 --- a/packages/core/test/bundling/defer/bundle.golden_symbols.json +++ b/packages/core/test/bundling/defer/bundle.golden_symbols.json @@ -602,6 +602,9 @@ { "name": "allocLFrame" }, + { + "name": "angularZoneInstanceIdProperty" + }, { "name": "appendChild" }, @@ -2153,6 +2156,9 @@ { "name": "invokeTriggerCleanupFns" }, + { + "name": "isAngularZoneProperty" + }, { "name": "isApplicationBootstrapConfig" }, @@ -2291,6 +2297,9 @@ { "name": "ngOnChangesSetInput" }, + { + "name": "ngZoneInstanceId" + }, { "name": "nonNull" }, diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json index b477e767ae5..1391e35ffaa 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -758,6 +758,9 @@ { "name": "allocLFrame" }, + { + "name": "angularZoneInstanceIdProperty" + }, { "name": "appendChild" }, @@ -1364,6 +1367,9 @@ { "name": "isAbstractControlOptions" }, + { + "name": "isAngularZoneProperty" + }, { "name": "isApplicationBootstrapConfig" }, @@ -1589,6 +1595,9 @@ { "name": "ngOnChangesSetInput" }, + { + "name": "ngZoneInstanceId" + }, { "name": "noSideEffects" }, diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json index 3d048d165c8..1523e6b2ead 100644 --- a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json @@ -740,6 +740,9 @@ { "name": "allocLFrame" }, + { + "name": "angularZoneInstanceIdProperty" + }, { "name": "appendChild" }, @@ -1322,6 +1325,9 @@ { "name": "invokeHostBindingsInCreationMode" }, + { + "name": "isAngularZoneProperty" + }, { "name": "isApplicationBootstrapConfig" }, @@ -1556,6 +1562,9 @@ { "name": "ngOnChangesSetInput" }, + { + "name": "ngZoneInstanceId" + }, { "name": "noSideEffects" }, diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json index c0826b970ed..2a0732999b6 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -398,6 +398,9 @@ { "name": "allocLFrame" }, + { + "name": "angularZoneInstanceIdProperty" + }, { "name": "applyNodes" }, @@ -749,6 +752,9 @@ { "name": "invokeHostBindingsInCreationMode" }, + { + "name": "isAngularZoneProperty" + }, { "name": "isApplicationBootstrapConfig" }, @@ -839,6 +845,9 @@ { "name": "ngOnChangesSetInput" }, + { + "name": "ngZoneInstanceId" + }, { "name": "noop" }, diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json index 9b9d4b76f61..ed0ee787c94 100644 --- a/packages/core/test/bundling/hydration/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hydration/bundle.golden_symbols.json @@ -587,6 +587,9 @@ { "name": "allocLFrame" }, + { + "name": "angularZoneInstanceIdProperty" + }, { "name": "appendChild" }, @@ -1019,6 +1022,9 @@ { "name": "invokeHostBindingsInCreationMode" }, + { + "name": "isAngularZoneProperty" + }, { "name": "isApplicationBootstrapConfig" }, @@ -1175,6 +1181,9 @@ { "name": "ngOnChangesSetInput" }, + { + "name": "ngZoneInstanceId" + }, { "name": "nonNull" }, diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index d3914100ea1..a4a0f8cffba 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -878,6 +878,9 @@ { "name": "allowSanitizationBypassAndThrow" }, + { + "name": "angularZoneInstanceIdProperty" + }, { "name": "appendChild" }, @@ -1553,6 +1556,9 @@ { "name": "invokeHostBindingsInCreationMode" }, + { + "name": "isAngularZoneProperty" + }, { "name": "isApplicationBootstrapConfig" }, @@ -1784,6 +1790,9 @@ { "name": "ngOnChangesSetInput" }, + { + "name": "ngZoneInstanceId" + }, { "name": "noMatch" }, diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json index ac3a508ab50..67197b70a76 100644 --- a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json +++ b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json @@ -482,6 +482,9 @@ { "name": "allocLFrame" }, + { + "name": "angularZoneInstanceIdProperty" + }, { "name": "appendChild" }, @@ -839,6 +842,9 @@ { "name": "invokeHostBindingsInCreationMode" }, + { + "name": "isAngularZoneProperty" + }, { "name": "isApplicationBootstrapConfig" }, @@ -944,6 +950,9 @@ { "name": "ngOnChangesSetInput" }, + { + "name": "ngZoneInstanceId" + }, { "name": "nonNull" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index ac4a6f94fd8..ea74ff755b6 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -629,6 +629,9 @@ { "name": "allocLFrame" }, + { + "name": "angularZoneInstanceIdProperty" + }, { "name": "appendChild" }, @@ -1124,6 +1127,9 @@ { "name": "invokeHostBindingsInCreationMode" }, + { + "name": "isAngularZoneProperty" + }, { "name": "isApplicationBootstrapConfig" }, @@ -1283,6 +1289,9 @@ { "name": "ngOnChangesSetInput" }, + { + "name": "ngZoneInstanceId" + }, { "name": "noSideEffects" }, diff --git a/packages/core/test/change_detection_scheduler_spec.ts b/packages/core/test/change_detection_scheduler_spec.ts index 196cc7b60bb..a81af495e66 100644 --- a/packages/core/test/change_detection_scheduler_spec.ts +++ b/packages/core/test/change_detection_scheduler_spec.ts @@ -16,7 +16,6 @@ import { Component, createComponent, destroyPlatform, - Directive, ElementRef, EnvironmentInjector, ErrorHandler, @@ -43,6 +42,7 @@ import {filter, take, tap} from 'rxjs/operators'; import {RuntimeError, RuntimeErrorCode} from '../src/errors'; import {scheduleCallbackWithRafRace} from '../src/util/callback_scheduler'; +import {ChangeDetectionSchedulerImpl} from '../src/change_detection/scheduling/zoneless_scheduling_impl'; import {global} from '../src/util/global'; function isStable(injector = TestBed.inject(EnvironmentInjector)): boolean { @@ -737,6 +737,59 @@ describe('Angular with scheduler and ZoneJS', () => { expect(fixture.nativeElement.innerText).toContain('new'); }); + it('updating signal in another "Angular" zone schedules update when in hybrid mode', async () => { + @Component({template: '{{thing()}}', standalone: true}) + class App { + thing = signal('initial'); + } + + const fixture = TestBed.createComponent(App); + await fixture.whenStable(); + expect(fixture.nativeElement.innerText).toContain('initial'); + const differentAngularZone: NgZone = Zone.root.run(() => new NgZone({})); + differentAngularZone.run(() => { + fixture.componentInstance.thing.set('new'); + }); + + expect(fixture.isStable()).toBe(false); + await fixture.whenStable(); + expect(fixture.nativeElement.innerText).toContain('new'); + }); + + it('updating signal in a child zone of Angular does not schedule extra CD', async () => { + @Component({template: '{{thing()}}', standalone: true}) + class App { + thing = signal('initial'); + } + + const fixture = TestBed.createComponent(App); + await fixture.whenStable(); + expect(fixture.nativeElement.innerText).toContain('initial'); + const childZone = TestBed.inject(NgZone).run(() => Zone.current.fork({name: 'child'})); + + childZone.run(() => { + fixture.componentInstance.thing.set('new'); + expect(TestBed.inject(ChangeDetectionSchedulerImpl)['cancelScheduledCallback']).toBeNull(); + }); + }); + + it('updating signal in a child Angular zone of Angular does not schedule extra CD', async () => { + @Component({template: '{{thing()}}', standalone: true}) + class App { + thing = signal('initial'); + } + + const fixture = TestBed.createComponent(App); + await fixture.whenStable(); + expect(fixture.nativeElement.innerText).toContain('initial'); + const childAngularZone = TestBed.inject(NgZone).run(() => new NgZone({})); + + childAngularZone.run(() => { + fixture.componentInstance.thing.set('new'); + expect(TestBed.inject(ChangeDetectionSchedulerImpl)['cancelScheduledCallback']).toBeNull(); + }); + }); + it('should not run change detection twice if notified during AppRef.tick', async () => { TestBed.configureTestingModule({ providers: [