diff --git a/packages/core/src/render3/instructions/change_detection.ts b/packages/core/src/render3/instructions/change_detection.ts index ebdfae5ed8d..daec6e90950 100644 --- a/packages/core/src/render3/instructions/change_detection.ts +++ b/packages/core/src/render3/instructions/change_detection.ts @@ -9,7 +9,9 @@ import { consumerAfterComputation, consumerBeforeComputation, + consumerDestroy, consumerPollProducersForChange, + getActiveConsumer, ReactiveNode, } from '@angular/core/primitives/signals'; @@ -29,12 +31,13 @@ import { REACTIVE_TEMPLATE_CONSUMER, TVIEW, TView, - TViewType, } from '../interfaces/view'; import { + getOrCreateTemporaryConsumer, getOrBorrowReactiveLViewConsumer, maybeReturnReactiveLViewConsumer, ReactiveLViewConsumer, + viewShouldHaveReactiveConsumer, } from '../reactive_lview_consumer'; import { CheckNoChangesMode, @@ -207,11 +210,27 @@ export function refreshView( // - We might already be in a reactive context if this is an embedded view of the host. // - We might be descending into a view that needs a consumer. enterView(lView); + let returnConsumerToPool = true; let prevConsumer: ReactiveNode | null = null; let currentConsumer: ReactiveLViewConsumer | null = null; - if (!isInCheckNoChangesPass && viewShouldHaveReactiveConsumer(tView)) { - currentConsumer = getOrBorrowReactiveLViewConsumer(lView); - prevConsumer = consumerBeforeComputation(currentConsumer); + if (!isInCheckNoChangesPass) { + if (viewShouldHaveReactiveConsumer(tView)) { + currentConsumer = getOrBorrowReactiveLViewConsumer(lView); + prevConsumer = consumerBeforeComputation(currentConsumer); + } else if (getActiveConsumer() === null) { + // If the current view should not have a reactive consumer but we don't have an active consumer, + // we still need to create a temporary consumer to track any signal reads in this template. + // This is a rare case that can happen with `viewContainerRef.createEmbeddedView(...).detectChanges()`. + // This temporary consumer marks the first parent that _should_ have a consumer for refresh. + // Once that refresh happens, the signals will be tracked in the parent consumer and we can destroy + // the temporary one. + returnConsumerToPool = false; + currentConsumer = getOrCreateTemporaryConsumer(lView); + prevConsumer = consumerBeforeComputation(currentConsumer); + } else if (lView[REACTIVE_TEMPLATE_CONSUMER]) { + consumerDestroy(lView[REACTIVE_TEMPLATE_CONSUMER]); + lView[REACTIVE_TEMPLATE_CONSUMER] = null; + } } try { @@ -351,30 +370,14 @@ export function refreshView( } finally { if (currentConsumer !== null) { consumerAfterComputation(currentConsumer, prevConsumer); - maybeReturnReactiveLViewConsumer(currentConsumer); + if (returnConsumerToPool) { + maybeReturnReactiveLViewConsumer(currentConsumer); + } } leaveView(); } } -/** - * Indicates if the view should get its own reactive consumer node. - * - * In the current design, all embedded views share a consumer with the component view. This allows - * us to refresh at the component level rather than at a per-view level. In addition, root views get - * their own reactive node because root component will have a host view that executes the - * component's host bindings. This needs to be tracked in a consumer as well. - * - * To get a more granular change detection than per-component, all we would just need to update the - * condition here so that a given view gets a reactive consumer which can become dirty independently - * from its parent component. For example embedded views for signal components could be created with - * a new type "SignalEmbeddedView" and the condition here wouldn't even need updating in order to - * get granular per-view change detection for signal components. - */ -function viewShouldHaveReactiveConsumer(tView: TView) { - return tView.type !== TViewType.Embedded; -} - /** * Goes over embedded views (ones created through ViewContainerRef APIs) and refreshes * them by executing an associated template function. diff --git a/packages/core/src/render3/reactive_lview_consumer.ts b/packages/core/src/render3/reactive_lview_consumer.ts index 0f18a26bbaa..1f6e431dcde 100644 --- a/packages/core/src/render3/reactive_lview_consumer.ts +++ b/packages/core/src/render3/reactive_lview_consumer.ts @@ -8,13 +8,20 @@ import {REACTIVE_NODE, ReactiveNode} from '@angular/core/primitives/signals'; -import {LView, REACTIVE_TEMPLATE_CONSUMER} from './interfaces/view'; -import {markAncestorsForTraversal} from './util/view_utils'; +import { + LView, + PARENT, + REACTIVE_TEMPLATE_CONSUMER, + TVIEW, + TView, + TViewType, +} from './interfaces/view'; +import {getLViewParent, markAncestorsForTraversal, markViewForRefresh} from './util/view_utils'; +import {assertDefined} from '../util/assert'; -let freeConsumers: ReactiveLViewConsumer[] = []; +let freeConsumers: ReactiveNode[] = []; export interface ReactiveLViewConsumer extends ReactiveNode { lView: LView | null; - slot: typeof REACTIVE_TEMPLATE_CONSUMER; } /** @@ -27,8 +34,7 @@ export function getOrBorrowReactiveLViewConsumer(lView: LView): ReactiveLViewCon } function borrowReactiveLViewConsumer(lView: LView): ReactiveLViewConsumer { - const consumer: ReactiveLViewConsumer = - freeConsumers.pop() ?? Object.create(REACTIVE_LVIEW_CONSUMER_NODE); + const consumer = freeConsumers.pop() ?? Object.create(REACTIVE_LVIEW_CONSUMER_NODE); consumer.lView = lView; return consumer; } @@ -42,7 +48,7 @@ export function maybeReturnReactiveLViewConsumer(consumer: ReactiveLViewConsumer freeConsumers.push(consumer); } -const REACTIVE_LVIEW_CONSUMER_NODE: Omit = { +const REACTIVE_LVIEW_CONSUMER_NODE: Omit = { ...REACTIVE_NODE, consumerIsAlwaysLive: true, consumerMarkedDirty: (node: ReactiveLViewConsumer) => { @@ -52,3 +58,60 @@ const REACTIVE_LVIEW_CONSUMER_NODE: Omit { + let parent = getLViewParent(node.lView!); + while (parent && !viewShouldHaveReactiveConsumer(parent[TVIEW])) { + parent = getLViewParent(parent); + } + if (!parent) { + // If we can't find an appropriate parent that should have a consumer, we + // don't have a way of appropriately refreshing this LView as part of application synchronization. + return; + } + + markViewForRefresh(parent); + }, + consumerOnSignalRead(this: ReactiveLViewConsumer): void { + this.lView![REACTIVE_TEMPLATE_CONSUMER] = this; + }, +}; + +/** + * Indicates if the view should get its own reactive consumer node. + * + * In the current design, all embedded views share a consumer with the component view. This allows + * us to refresh at the component level rather than at a per-view level. In addition, root views get + * their own reactive node because root component will have a host view that executes the + * component's host bindings. This needs to be tracked in a consumer as well. + * + * To get a more granular change detection than per-component, all we would just need to update the + * condition here so that a given view gets a reactive consumer which can become dirty independently + * from its parent component. For example embedded views for signal components could be created with + * a new type "SignalEmbeddedView" and the condition here wouldn't even need updating in order to + * get granular per-view change detection for signal components. + */ +export function viewShouldHaveReactiveConsumer(tView: TView) { + return tView.type !== TViewType.Embedded; +} diff --git a/packages/core/test/acceptance/change_detection_signals_in_zones_spec.ts b/packages/core/test/acceptance/change_detection_signals_in_zones_spec.ts index d37ed0c9bdc..0be7fe065d9 100644 --- a/packages/core/test/acceptance/change_detection_signals_in_zones_spec.ts +++ b/packages/core/test/acceptance/change_detection_signals_in_zones_spec.ts @@ -7,19 +7,16 @@ */ import {NgFor, NgIf} from '@angular/common'; -import {PLATFORM_BROWSER_ID} from '@angular/common/src/platform_id'; import { - afterNextRender, ApplicationRef, ChangeDetectionStrategy, ChangeDetectorRef, Component, computed, Directive, - EnvironmentInjector, + ElementRef, inject, Input, - PLATFORM_ID, signal, TemplateRef, ViewChild, @@ -803,6 +800,73 @@ describe('OnPush components with signals', () => { fixture.detectChanges(); expect(trim(fixture.nativeElement.textContent)).toEqual('new'); }); + + it('tracks signal updates if embedded view is change detected directly', () => { + @Component({ + changeDetection: ChangeDetectionStrategy.OnPush, + template: ` + {{value()}} + `, + standalone: true, + }) + class Test { + value = signal('initial'); + @ViewChild('template', {static: true, read: TemplateRef}) + template!: TemplateRef<{}>; + @ViewChild('template', {static: true, read: ViewContainerRef}) + vcr!: ViewContainerRef; + } + + const fixture = TestBed.createComponent(Test); + const appRef = TestBed.inject(ApplicationRef); + appRef.attachView(fixture.componentRef.hostView); + appRef.tick(); + + const viewRef = fixture.componentInstance.vcr.createEmbeddedView( + fixture.componentInstance.template, + ); + viewRef.detectChanges(); + expect(fixture.nativeElement.innerText).toContain('initial'); + + fixture.componentInstance.value.set('new'); + appRef.tick(); + expect(fixture.nativeElement.innerText).toContain('new'); + }); + + it('tracks signal updates if embedded view is change detected directly before attaching', () => { + @Component({ + changeDetection: ChangeDetectionStrategy.OnPush, + template: ` + {{value()}} + `, + standalone: true, + }) + class Test { + value = signal('initial'); + @ViewChild('template', {static: true, read: TemplateRef}) + template!: TemplateRef<{}>; + @ViewChild('template', {static: true, read: ViewContainerRef}) + vcr!: ViewContainerRef; + element = inject(ElementRef); + } + + const fixture = TestBed.createComponent(Test); + const appRef = TestBed.inject(ApplicationRef); + appRef.attachView(fixture.componentRef.hostView); + appRef.tick(); + + const viewRef = fixture.componentInstance.template.createEmbeddedView( + fixture.componentInstance.template, + ); + fixture.componentInstance.element.nativeElement.appendChild(viewRef.rootNodes[0]); + viewRef.detectChanges(); + expect(fixture.nativeElement.innerText).toContain('initial'); + fixture.componentInstance.vcr.insert(viewRef); + + fixture.componentInstance.value.set('new'); + appRef.tick(); + expect(fixture.nativeElement.innerText).toContain('new'); + }); }); describe('shielded by non-dirty OnPush', () => { 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 7c8a2c303dd..ea0aeddae9e 100644 --- a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json @@ -434,6 +434,9 @@ { "name": "REACTIVE_LVIEW_CONSUMER_NODE" }, + { + "name": "REACTIVE_NODE" + }, { "name": "REMOVE_STYLES_ON_COMPONENT_DESTROY" }, @@ -491,6 +494,9 @@ { "name": "Subscription" }, + { + "name": "TEMPORARY_CONSUMER_NODE" + }, { "name": "TESTABILITY" }, @@ -746,6 +752,12 @@ { "name": "configureViewWithDirective" }, + { + "name": "consumerBeforeComputation" + }, + { + "name": "consumerDestroy" + }, { "name": "consumerIsLive" }, @@ -1454,6 +1466,9 @@ { "name": "viewAttachedToChangeDetector" }, + { + "name": "viewShouldHaveReactiveConsumer" + }, { "name": "visitDslNode" }, diff --git a/packages/core/test/bundling/animations/bundle.golden_symbols.json b/packages/core/test/bundling/animations/bundle.golden_symbols.json index f1d1e965b7a..34a013e0c4b 100644 --- a/packages/core/test/bundling/animations/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations/bundle.golden_symbols.json @@ -473,6 +473,9 @@ { "name": "REACTIVE_LVIEW_CONSUMER_NODE" }, + { + "name": "REACTIVE_NODE" + }, { "name": "REMOVE_STYLES_ON_COMPONENT_DESTROY" }, @@ -533,6 +536,9 @@ { "name": "Subscription" }, + { + "name": "TEMPORARY_CONSUMER_NODE" + }, { "name": "TESTABILITY" }, @@ -803,6 +809,12 @@ { "name": "configureViewWithDirective" }, + { + "name": "consumerBeforeComputation" + }, + { + "name": "consumerDestroy" + }, { "name": "consumerIsLive" }, @@ -1529,6 +1541,9 @@ { "name": "viewAttachedToChangeDetector" }, + { + "name": "viewShouldHaveReactiveConsumer" + }, { "name": "visitDslNode" }, 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 618b478242a..634dcbde86c 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -359,6 +359,9 @@ { "name": "REACTIVE_LVIEW_CONSUMER_NODE" }, + { + "name": "REACTIVE_NODE" + }, { "name": "REMOVE_STYLES_ON_COMPONENT_DESTROY" }, @@ -401,6 +404,9 @@ { "name": "Subscription" }, + { + "name": "TEMPORARY_CONSUMER_NODE" + }, { "name": "TESTABILITY" }, @@ -602,6 +608,12 @@ { "name": "configureViewWithDirective" }, + { + "name": "consumerBeforeComputation" + }, + { + "name": "consumerDestroy" + }, { "name": "consumerIsLive" }, @@ -1229,6 +1241,9 @@ { "name": "viewAttachedToChangeDetector" }, + { + "name": "viewShouldHaveReactiveConsumer" + }, { "name": "walkProviderTree" }, diff --git a/packages/core/test/bundling/defer/bundle.golden_symbols.json b/packages/core/test/bundling/defer/bundle.golden_symbols.json index d623a99285a..6df37bcdaec 100644 --- a/packages/core/test/bundling/defer/bundle.golden_symbols.json +++ b/packages/core/test/bundling/defer/bundle.golden_symbols.json @@ -461,6 +461,9 @@ { "name": "Subscription" }, + { + "name": "TEMPORARY_CONSUMER_NODE" + }, { "name": "TESTABILITY" }, @@ -680,6 +683,12 @@ { "name": "configureViewWithDirective" }, + { + "name": "consumerBeforeComputation" + }, + { + "name": "consumerDestroy" + }, { "name": "consumerIsLive" }, @@ -2468,6 +2477,9 @@ { "name": "viewAttachedToChangeDetector" }, + { + "name": "viewShouldHaveReactiveConsumer" + }, { "name": "walkProviderTree" }, 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 ad6cca802b0..4e4f46162b0 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -491,6 +491,9 @@ { "name": "REACTIVE_LVIEW_CONSUMER_NODE" }, + { + "name": "REACTIVE_NODE" + }, { "name": "REMOVE_STYLES_ON_COMPONENT_DESTROY" }, @@ -557,6 +560,9 @@ { "name": "Subscription" }, + { + "name": "TEMPORARY_CONSUMER_NODE" + }, { "name": "TESTABILITY" }, @@ -848,6 +854,12 @@ { "name": "configureViewWithDirective" }, + { + "name": "consumerBeforeComputation" + }, + { + "name": "consumerDestroy" + }, { "name": "consumerIsLive" }, @@ -1793,6 +1805,9 @@ { "name": "viewAttachedToChangeDetector" }, + { + "name": "viewShouldHaveReactiveConsumer" + }, { "name": "walkProviderTree" }, 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 6e9ff94462c..3561dfac83a 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 @@ -476,6 +476,9 @@ { "name": "REACTIVE_LVIEW_CONSUMER_NODE" }, + { + "name": "REACTIVE_NODE" + }, { "name": "REMOVE_STYLES_ON_COMPONENT_DESTROY" }, @@ -539,6 +542,9 @@ { "name": "Subscription" }, + { + "name": "TEMPORARY_CONSUMER_NODE" + }, { "name": "TESTABILITY" }, @@ -815,6 +821,12 @@ { "name": "configureViewWithDirective" }, + { + "name": "consumerBeforeComputation" + }, + { + "name": "consumerDestroy" + }, { "name": "consumerIsLive" }, @@ -1775,6 +1787,9 @@ { "name": "viewAttachedToChangeDetector" }, + { + "name": "viewShouldHaveReactiveConsumer" + }, { "name": "walkProviderTree" }, 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 158a4098102..e1de6bd3e49 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -278,6 +278,9 @@ { "name": "REACTIVE_LVIEW_CONSUMER_NODE" }, + { + "name": "REACTIVE_NODE" + }, { "name": "RendererFactory2" }, @@ -308,6 +311,9 @@ { "name": "Subscription" }, + { + "name": "TEMPORARY_CONSUMER_NODE" + }, { "name": "TESTABILITY" }, @@ -464,6 +470,12 @@ { "name": "configureViewWithDirective" }, + { + "name": "consumerBeforeComputation" + }, + { + "name": "consumerDestroy" + }, { "name": "consumerIsLive" }, @@ -980,6 +992,9 @@ { "name": "viewAttachedToChangeDetector" }, + { + "name": "viewShouldHaveReactiveConsumer" + }, { "name": "walkProviderTree" }, diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json index 7bbe5a91943..2bd5b091a78 100644 --- a/packages/core/test/bundling/hydration/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hydration/bundle.golden_symbols.json @@ -392,6 +392,9 @@ { "name": "REACTIVE_LVIEW_CONSUMER_NODE" }, + { + "name": "REACTIVE_NODE" + }, { "name": "REFERENCE_NODE_BODY" }, @@ -464,6 +467,9 @@ { "name": "Subscription" }, + { + "name": "TEMPORARY_CONSUMER_NODE" + }, { "name": "TESTABILITY" }, @@ -665,6 +671,12 @@ { "name": "configureViewWithDirective" }, + { + "name": "consumerBeforeComputation" + }, + { + "name": "consumerDestroy" + }, { "name": "consumerIsLive" }, @@ -1367,6 +1379,9 @@ { "name": "viewAttachedToChangeDetector" }, + { + "name": "viewShouldHaveReactiveConsumer" + }, { "name": "walkProviderTree" }, diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index ab00e6e8d98..55f5ee42f6f 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -548,6 +548,9 @@ { "name": "REACTIVE_LVIEW_CONSUMER_NODE" }, + { + "name": "REACTIVE_NODE" + }, { "name": "REMOVE_STYLES_ON_COMPONENT_DESTROY" }, @@ -677,6 +680,9 @@ { "name": "Subscription" }, + { + "name": "TEMPORARY_CONSUMER_NODE" + }, { "name": "TESTABILITY" }, @@ -965,6 +971,12 @@ { "name": "configureViewWithDirective" }, + { + "name": "consumerBeforeComputation" + }, + { + "name": "consumerDestroy" + }, { "name": "consumerIsLive" }, @@ -2093,6 +2105,9 @@ { "name": "viewAttachedToChangeDetector" }, + { + "name": "viewShouldHaveReactiveConsumer" + }, { "name": "walkProviderTree" }, 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 95491d85e83..303e008a7aa 100644 --- a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json +++ b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json @@ -323,6 +323,9 @@ { "name": "REACTIVE_LVIEW_CONSUMER_NODE" }, + { + "name": "REACTIVE_NODE" + }, { "name": "REMOVE_STYLES_ON_COMPONENT_DESTROY" }, @@ -368,6 +371,9 @@ { "name": "Subscription" }, + { + "name": "TEMPORARY_CONSUMER_NODE" + }, { "name": "TESTABILITY" }, @@ -542,6 +548,12 @@ { "name": "configureViewWithDirective" }, + { + "name": "consumerBeforeComputation" + }, + { + "name": "consumerDestroy" + }, { "name": "consumerIsLive" }, @@ -1079,6 +1091,9 @@ { "name": "viewAttachedToChangeDetector" }, + { + "name": "viewShouldHaveReactiveConsumer" + }, { "name": "walkProviderTree" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 3b68034a3a5..9a6e8b851a9 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -386,6 +386,9 @@ { "name": "REACTIVE_LVIEW_CONSUMER_NODE" }, + { + "name": "REACTIVE_NODE" + }, { "name": "REMOVE_STYLES_ON_COMPONENT_DESTROY" }, @@ -434,6 +437,9 @@ { "name": "Subscription" }, + { + "name": "TEMPORARY_CONSUMER_NODE" + }, { "name": "TESTABILITY" }, @@ -713,6 +719,12 @@ { "name": "configureViewWithDirective" }, + { + "name": "consumerBeforeComputation" + }, + { + "name": "consumerDestroy" + }, { "name": "consumerIsLive" }, @@ -1481,6 +1493,9 @@ { "name": "viewAttachedToChangeDetector" }, + { + "name": "viewShouldHaveReactiveConsumer" + }, { "name": "walkProviderTree" },