diff --git a/packages/core/src/change_detection/flags.ts b/packages/core/src/change_detection/flags.ts new file mode 100644 index 00000000000..0c816a7573b --- /dev/null +++ b/packages/core/src/change_detection/flags.ts @@ -0,0 +1,17 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +// TODO(atscott): flip default internally ASAP and externally for v18 (#52928) +let _ensureDirtyViewsAreAlwaysReachable = false; + +export function getEnsureDirtyViewsAreAlwaysReachable() { + return _ensureDirtyViewsAreAlwaysReachable; +} +export function setEnsureDirtyViewsAreAlwaysReachable(v: boolean) { + _ensureDirtyViewsAreAlwaysReachable = v; +} diff --git a/packages/core/src/core_private_export.ts b/packages/core/src/core_private_export.ts index a6a4358734d..4bcbf0bbe41 100644 --- a/packages/core/src/core_private_export.ts +++ b/packages/core/src/core_private_export.ts @@ -11,6 +11,7 @@ export {whenStable as ɵwhenStable} from './application/application_ref'; export {IMAGE_CONFIG as ɵIMAGE_CONFIG, IMAGE_CONFIG_DEFAULTS as ɵIMAGE_CONFIG_DEFAULTS, ImageConfig as ɵImageConfig} from './application/application_tokens'; export {internalCreateApplication as ɵinternalCreateApplication} from './application/create_application'; export {defaultIterableDiffers as ɵdefaultIterableDiffers, defaultKeyValueDiffers as ɵdefaultKeyValueDiffers} from './change_detection/change_detection'; +export {getEnsureDirtyViewsAreAlwaysReachable as ɵgetEnsureDirtyViewsAreAlwaysReachable, setEnsureDirtyViewsAreAlwaysReachable as ɵsetEnsureDirtyViewsAreAlwaysReachable} from './change_detection/flags'; export {Console as ɵConsole} from './console'; export {DeferBlockDetails as ɵDeferBlockDetails, getDeferBlocks as ɵgetDeferBlocks} from './defer/discovery'; export {renderDeferBlockState as ɵrenderDeferBlockState, triggerResourceLoading as ɵtriggerResourceLoading} from './defer/instructions'; diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts index c5ee9dc3b2b..5a30f134c24 100644 --- a/packages/core/src/render3/component_ref.ts +++ b/packages/core/src/render3/component_ref.ts @@ -220,12 +220,12 @@ export class ComponentFactory extends AbstractComponentFactory { hostRenderer, rootSelectorOrNode, this.componentDef.encapsulation, rootViewInjector) : createElementNode(hostRenderer, elementName, getNamespace(elementName)); - // Signal components use the granular "RefreshView" for change detection - const signalFlags = (LViewFlags.SignalView | LViewFlags.IsRoot); - // Non-signal components use the traditional "CheckAlways or OnPush/Dirty" change detection - const nonSignalFlags = this.componentDef.onPush ? LViewFlags.Dirty | LViewFlags.IsRoot : - LViewFlags.CheckAlways | LViewFlags.IsRoot; - const rootFlags = this.componentDef.signals ? signalFlags : nonSignalFlags; + let rootFlags = LViewFlags.IsRoot; + if (this.componentDef.signals) { + rootFlags |= LViewFlags.SignalView; + } else if (!this.componentDef.onPush) { + rootFlags |= LViewFlags.CheckAlways; + } let hydrationInfo: DehydratedView|null = null; if (hostRNode !== null) { diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index 0ad5d165713..ab3bff9c1b7 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -94,7 +94,8 @@ export function createLView( hydrationInfo: DehydratedView|null): LView { const lView = tView.blueprint.slice() as LView; lView[HOST] = host; - lView[FLAGS] = flags | LViewFlags.CreationMode | LViewFlags.Attached | LViewFlags.FirstLViewPass; + lView[FLAGS] = flags | LViewFlags.CreationMode | LViewFlags.Attached | LViewFlags.FirstLViewPass | + LViewFlags.Dirty; if (embeddedViewInjector !== null || (parentLView && (parentLView[FLAGS] & LViewFlags.HasEmbeddedViewInjector))) { lView[FLAGS] |= LViewFlags.HasEmbeddedViewInjector; diff --git a/packages/core/src/render3/util/view_utils.ts b/packages/core/src/render3/util/view_utils.ts index 59f2656e7ba..2648ae98566 100644 --- a/packages/core/src/render3/util/view_utils.ts +++ b/packages/core/src/render3/util/view_utils.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +import {getEnsureDirtyViewsAreAlwaysReachable} from '../../change_detection/flags'; import {RuntimeError, RuntimeErrorCode} from '../../errors'; import {assertDefined, assertGreaterThan, assertGreaterThanOrEqual, assertIndexInRange, assertLessThan} from '../../util/assert'; import {assertTNode, assertTNodeForLView} from '../assert'; @@ -206,6 +207,12 @@ export function requiresRefreshOrTraversal(lView: LView) { * parents above. */ export function updateAncestorTraversalFlagsOnAttach(lView: LView) { + // When we attach a view that's marked `Dirty`, we should ensure that it is reached during the + // next CD traversal so we add the `RefreshView` flag and mark ancestors accordingly. + if (lView[FLAGS] & LViewFlags.Dirty && getEnsureDirtyViewsAreAlwaysReachable()) { + lView[FLAGS] |= LViewFlags.RefreshView; + } + if (!requiresRefreshOrTraversal(lView)) { return; } diff --git a/packages/core/test/acceptance/change_detection_spec.ts b/packages/core/test/acceptance/change_detection_spec.ts index 77efadee300..16ba8f3aaca 100644 --- a/packages/core/test/acceptance/change_detection_spec.ts +++ b/packages/core/test/acceptance/change_detection_spec.ts @@ -8,7 +8,7 @@ import {CommonModule} from '@angular/common'; -import {ApplicationRef, ChangeDetectionStrategy, ChangeDetectorRef, Component, ComponentRef, createComponent, Directive, DoCheck, EmbeddedViewRef, ErrorHandler, EventEmitter, inject, Input, NgModule, OnInit, Output, QueryList, TemplateRef, Type, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core'; +import {ApplicationRef, ChangeDetectionStrategy, ChangeDetectorRef, Component, ComponentRef, Directive, DoCheck, EmbeddedViewRef, ErrorHandler, EventEmitter, inject, Input, NgModule, OnInit, Output, QueryList, TemplateRef, Type, ViewChild, ViewChildren, ViewContainerRef, ɵgetEnsureDirtyViewsAreAlwaysReachable, ɵsetEnsureDirtyViewsAreAlwaysReachable} from '@angular/core'; import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {BehaviorSubject} from 'rxjs'; @@ -184,17 +184,18 @@ describe('change detection', () => { describe('markForCheck', () => { it('should mark OnPush ancestor of dynamically created component views as dirty', () => { + const previous = ɵgetEnsureDirtyViewsAreAlwaysReachable(); + ɵsetEnsureDirtyViewsAreAlwaysReachable(true); @Component({ selector: `test-cmpt`, template: `{{counter}}|`, - changeDetection: ChangeDetectionStrategy.OnPush + changeDetection: ChangeDetectionStrategy.OnPush, + standalone: true, }) class TestCmpt { counter = 0; @ViewChild('vc', {read: ViewContainerRef}) vcRef!: ViewContainerRef; - constructor() {} - createComponentView(cmptType: Type): ComponentRef { return this.vcRef.createComponent(cmptType); } @@ -202,18 +203,14 @@ describe('change detection', () => { @Component({ selector: 'dynamic-cmpt', - template: `dynamic`, + template: `dynamic|{{binding}}`, + standalone: true, changeDetection: ChangeDetectionStrategy.OnPush }) class DynamicCmpt { + @Input() binding = 'binding'; } - @NgModule({declarations: [DynamicCmpt]}) - class DynamicModule { - } - - TestBed.configureTestingModule({imports: [DynamicModule], declarations: [TestCmpt]}); - const fixture = TestBed.createComponent(TestCmpt); // initial CD to have query results @@ -221,20 +218,34 @@ describe('change detection', () => { fixture.detectChanges(false); expect(fixture.nativeElement).toHaveText('0|'); - // insert a dynamic component + // insert a dynamic component, but do not specifically mark parent dirty + // (dynamic components with OnPush flag are created with the `Dirty` flag) const dynamicCmptRef = fixture.componentInstance.createComponentView(DynamicCmpt); fixture.detectChanges(false); - expect(fixture.nativeElement).toHaveText('0|dynamic'); + expect(fixture.nativeElement).toHaveText('0|dynamic|binding'); // update model in the OnPush component - should not update UI fixture.componentInstance.counter = 1; fixture.detectChanges(false); - expect(fixture.nativeElement).toHaveText('0|dynamic'); + expect(fixture.nativeElement).toHaveText('0|dynamic|binding'); // now mark the dynamically inserted component as dirty dynamicCmptRef.changeDetectorRef.markForCheck(); fixture.detectChanges(false); - expect(fixture.nativeElement).toHaveText('1|dynamic'); + expect(fixture.nativeElement).toHaveText('1|dynamic|binding'); + + // Update, mark for check, and detach before change detection, should not update + dynamicCmptRef.setInput('binding', 'updatedBinding'); + dynamicCmptRef.changeDetectorRef.markForCheck(); + dynamicCmptRef.changeDetectorRef.detach(); + fixture.detectChanges(false); + expect(fixture.nativeElement).toHaveText('1|dynamic|binding'); + + // reattaching and run CD from the top should update + dynamicCmptRef.changeDetectorRef.reattach(); + fixture.detectChanges(false); + expect(fixture.nativeElement).toHaveText('1|dynamic|updatedBinding'); + ɵsetEnsureDirtyViewsAreAlwaysReachable(previous); }); it('should support re-enterant change detection', () => { diff --git a/packages/core/test/bundling/defer/bundle.golden_symbols.json b/packages/core/test/bundling/defer/bundle.golden_symbols.json index 4b108157451..0ea35badcc3 100644 --- a/packages/core/test/bundling/defer/bundle.golden_symbols.json +++ b/packages/core/test/bundling/defer/bundle.golden_symbols.json @@ -533,6 +533,9 @@ { "name": "_enable_super_gross_mode_that_will_cause_bad_things" }, + { + "name": "_ensureDirtyViewsAreAlwaysReachable" + }, { "name": "_findMatchingDehydratedViewImpl" }, @@ -1334,6 +1337,9 @@ { "name": "init_fields" }, + { + "name": "init_flags" + }, { "name": "init_forward_ref" }, 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 2597f240222..986f1c70789 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -638,6 +638,9 @@ { "name": "_enable_super_gross_mode_that_will_cause_bad_things" }, + { + "name": "_ensureDirtyViewsAreAlwaysReachable" + }, { "name": "_getInsertInFrontOfRNodeWithI18n" }, 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 7441ab2f4b3..294827acbc2 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 @@ -629,6 +629,9 @@ { "name": "_enable_super_gross_mode_that_will_cause_bad_things" }, + { + "name": "_ensureDirtyViewsAreAlwaysReachable" + }, { "name": "_getInsertInFrontOfRNodeWithI18n" }, diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index 9e438ea37a2..d780911f106 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -863,6 +863,9 @@ { "name": "_enable_super_gross_mode_that_will_cause_bad_things" }, + { + "name": "_ensureDirtyViewsAreAlwaysReachable" + }, { "name": "_getInsertInFrontOfRNodeWithI18n" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index cc6a77eb512..5bc987d4e3e 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -554,6 +554,9 @@ { "name": "_enable_super_gross_mode_that_will_cause_bad_things" }, + { + "name": "_ensureDirtyViewsAreAlwaysReachable" + }, { "name": "_getInsertInFrontOfRNodeWithI18n" }, diff --git a/packages/core/test/linker/change_detection_integration_spec.ts b/packages/core/test/linker/change_detection_integration_spec.ts index 0c98abe4ff6..5cc67f0629d 100644 --- a/packages/core/test/linker/change_detection_integration_spec.ts +++ b/packages/core/test/linker/change_detection_integration_spec.ts @@ -7,7 +7,7 @@ */ import {ResourceLoader} from '@angular/compiler'; -import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, ChangeDetectionStrategy, ChangeDetectorRef, Component, ContentChild, DebugElement, Directive, DoCheck, EventEmitter, HostBinding, Injectable, Input, OnChanges, OnDestroy, OnInit, Output, Pipe, PipeTransform, Provider, RendererFactory2, RendererType2, SimpleChange, SimpleChanges, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/core'; +import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, ChangeDetectionStrategy, ChangeDetectorRef, Component, ContentChild, DebugElement, Directive, DoCheck, EventEmitter, HostBinding, Injectable, Input, OnChanges, OnDestroy, OnInit, Output, Pipe, PipeTransform, Provider, RendererFactory2, RendererType2, SimpleChange, SimpleChanges, TemplateRef, Type, ViewChild, ViewContainerRef, ɵgetEnsureDirtyViewsAreAlwaysReachable, ɵsetEnsureDirtyViewsAreAlwaysReachable} from '@angular/core'; import {ComponentFixture, fakeAsync, TestBed} from '@angular/core/testing'; import {By} from '@angular/platform-browser/src/dom/debug/by'; import {isTextNode} from '@angular/platform-browser/testing/src/browser_util'; @@ -1135,16 +1135,15 @@ describe(`ChangeDetection`, () => { expect(() => ctx.checkNoChanges()).toThrowError(errMsgRegExp); })); - it('should warn when the view has been created in a cd hook', fakeAsync(() => { - const ctx = createCompFixture('
{{ a }}
', TestData); - ctx.componentInstance.a = 1; - expect(() => ctx.detectChanges()) - .toThrowError( - /It seems like the view has been created after its parent and its children have been dirty checked/); - - // subsequent change detection should run without issues - ctx.detectChanges(); - })); + it('should allow view to be created in a cd hook', () => { + const previous = ɵgetEnsureDirtyViewsAreAlwaysReachable(); + ɵsetEnsureDirtyViewsAreAlwaysReachable(true); + const ctx = createCompFixture('
{{ a }}
', TestData); + ctx.componentInstance.a = 1; + ctx.detectChanges(); + expect(ctx.nativeElement.innerText).toEqual('1'); + ɵsetEnsureDirtyViewsAreAlwaysReachable(previous); + }); it('should not throw when two arrays are structurally the same', fakeAsync(() => { const ctx = _bindSimpleValue('a', TestData);