diff --git a/packages/core/src/render3/instructions/change_detection.ts b/packages/core/src/render3/instructions/change_detection.ts index 5e292d8c54770..f5f2ba5ca7728 100644 --- a/packages/core/src/render3/instructions/change_detection.ts +++ b/packages/core/src/render3/instructions/change_detection.ts @@ -13,7 +13,7 @@ import {getComponentViewByInstance} from '../context_discovery'; import {executeCheckHooks, executeInitAndCheckHooks, incrementInitPhaseFlags} from '../hooks'; import {CONTAINER_HEADER_OFFSET, HAS_CHILD_VIEWS_TO_REFRESH, HAS_TRANSPLANTED_VIEWS, LContainer, MOVED_VIEWS} from '../interfaces/container'; import {ComponentTemplate, RenderFlags} from '../interfaces/definition'; -import {CONTEXT, ENVIRONMENT, FLAGS, InitPhaseState, LView, LViewFlags, PARENT, REACTIVE_TEMPLATE_CONSUMER, TVIEW, TView} from '../interfaces/view'; +import {CONTEXT, ENVIRONMENT, FLAGS, InitPhaseState, LView, LViewFlags, PARENT, REACTIVE_TEMPLATE_CONSUMER, TVIEW, TView, TViewType} from '../interfaces/view'; import {enterView, isInCheckNoChangesMode, leaveView, setBindingIndex, setIsInCheckNoChangesMode} from '../state'; import {getFirstLContainer, getNextLContainer} from '../util/view_traversal_utils'; import {getComponentLViewByIndex, isCreationMode, markAncestorsForTraversal, markViewForRefresh, resetPreOrderHookFlags, viewAttachedToChangeDetector} from '../util/view_utils'; @@ -344,6 +344,7 @@ function detectChangesInViewIfAttached(lView: LView, mode: ChangeDetectionMode) * view HasChildViewsToRefresh flag is set. */ function detectChangesInView(lView: LView, mode: ChangeDetectionMode) { + const isInCheckNoChangesPass = ngDevMode && isInCheckNoChangesMode(); const tView = lView[TVIEW]; const flags = lView[FLAGS]; @@ -351,8 +352,14 @@ function detectChangesInView(lView: LView, mode: ChangeDetectionMode) { // necessary. lView[FLAGS] &= ~(LViewFlags.HasChildViewsToRefresh | LViewFlags.RefreshView); - if ((flags & (LViewFlags.CheckAlways | LViewFlags.Dirty) && - mode === ChangeDetectionMode.Global) || + if ((flags & LViewFlags.CheckAlways && mode === ChangeDetectionMode.Global) || + (flags & LViewFlags.Dirty && mode === ChangeDetectionMode.Global && + // CheckNoChanges never worked with `OnPush` components because the `Dirty` flag was cleared + // before checkNoChanges ran. Because there is now a loop for to check for backwards views, + // it gives an opportunity for `OnPush` components to be marked `Dirty` before the + // CheckNoChanges pass. We don't want existing errors that are hidden by the current + // CheckNoChanges bug to surface when making unrelated changes. + !isInCheckNoChangesPass) || flags & LViewFlags.RefreshView) { refreshView(tView, lView, tView.template, lView[CONTEXT]); } else if (flags & LViewFlags.HasChildViewsToRefresh) { diff --git a/packages/core/src/render3/reactive_lview_consumer.ts b/packages/core/src/render3/reactive_lview_consumer.ts index ed7cdedc15aa2..2bb276a3de632 100644 --- a/packages/core/src/render3/reactive_lview_consumer.ts +++ b/packages/core/src/render3/reactive_lview_consumer.ts @@ -8,11 +8,8 @@ import {REACTIVE_NODE, ReactiveNode} from '@angular/core/primitives/signals'; -import {RuntimeError} from '../errors'; -import {assertDefined} from '../util/assert'; - -import {markViewDirty} from './instructions/mark_view_dirty'; import {LView, REACTIVE_HOST_BINDING_CONSUMER, REACTIVE_TEMPLATE_CONSUMER} from './interfaces/view'; +import {markViewDirtyFromSignal} from './util/view_utils'; let currentConsumer: ReactiveLViewConsumer|null = null; export interface ReactiveLViewConsumer extends ReactiveNode { @@ -43,7 +40,7 @@ const REACTIVE_LVIEW_CONSUMER_NODE: Omit ` in ExpressionChangedAfterItHasBeenChecked errors or future updates not working` + ` entirely.`); } - markViewDirty(node.lView); + markViewDirtyFromSignal(node.lView); }, consumerOnSignalRead(this: ReactiveLViewConsumer): void { if (currentConsumer !== this) { diff --git a/packages/core/src/render3/util/view_utils.ts b/packages/core/src/render3/util/view_utils.ts index 8e792824f4b16..da88a183d1153 100644 --- a/packages/core/src/render3/util/view_utils.ts +++ b/packages/core/src/render3/util/view_utils.ts @@ -13,7 +13,7 @@ import {HAS_CHILD_VIEWS_TO_REFRESH, LContainer, TYPE} from '../interfaces/contai import {TConstants, TNode} from '../interfaces/node'; import {RNode} from '../interfaces/renderer_dom'; import {isLContainer, isLView} from '../interfaces/type_checks'; -import {DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, ON_DESTROY_HOOKS, PARENT, PREORDER_HOOK_FLAGS, PreOrderHookFlags, TData, TView} from '../interfaces/view'; +import {DECLARATION_COMPONENT_VIEW, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, ON_DESTROY_HOOKS, PARENT, PREORDER_HOOK_FLAGS, PreOrderHookFlags, TData, TView} from '../interfaces/view'; @@ -241,6 +241,25 @@ export function markAncestorsForTraversal(lView: LView) { } } +/** + * Marks the component or root view of an LView for refresh. + * + * This function locates the declaration component view of a given LView and marks it for refresh. + * With this, we get component-level change detection granularity. Marking the `LView` itself for + * refresh would be view-level granularity. + * + * Note that when an LView is a root view, the DECLARATION_COMPONENT_VIEW will be the root view + * itself. This is a bit confusing since the TView.type is `Root`, rather than `Component`, but this + * is actually what we need for host bindings in a root view. + */ +export function markViewDirtyFromSignal(lView: LView): void { + const declarationComponentView = lView[DECLARATION_COMPONENT_VIEW]; + declarationComponentView[FLAGS] |= LViewFlags.RefreshView; + if (viewAttachedToChangeDetector(declarationComponentView)) { + markAncestorsForTraversal(declarationComponentView); + } +} + /** * Stores a LView-specific destroy callback. */ 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 7595ee41107ff..6fe555c4e8e1c 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 @@ -6,8 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ -import {NgIf} from '@angular/common'; -import {ChangeDetectionStrategy, Component, Directive, Input, signal, ViewChild} from '@angular/core'; +import {NgFor, NgIf} from '@angular/common'; +import {ChangeDetectionStrategy, ChangeDetectorRef, Component, Directive, Input, signal, untracked, ViewChild} from '@angular/core'; import {TestBed} from '@angular/core/testing'; describe('CheckAlways components', () => { @@ -88,6 +88,87 @@ describe('CheckAlways components', () => { fixture.detectChanges(); expect(fixture.nativeElement.textContent.trim()).toBe('new'); }); + + it('continues to refresh views until none are dirty', () => { + const aVal = signal('initial'); + const bVal = signal('initial'); + let updateAValDuringAChangeDetection = false; + + @Component({ + template: '{{val()}}', + standalone: true, + selector: 'a-comp', + }) + class A { + val = aVal; + } + @Component({ + template: '{{val()}}', + standalone: true, + selector: 'b-comp', + }) + class B { + val = bVal; + ngAfterViewChecked() { + // Set value in parent view after this view is checked + // Without signals, this is ExpressionChangedAfterItWasChecked + if (updateAValDuringAChangeDetection) { + aVal.set('new'); + } + } + } + + @Component({template: '-', standalone: true, imports: [A, B]}) + class App { + } + + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + expect(fixture.nativeElement.innerText).toContain('initial-initial'); + + bVal.set('new'); + fixture.detectChanges(); + expect(fixture.nativeElement.innerText).toContain('initial-new'); + + updateAValDuringAChangeDetection = true; + bVal.set('newer'); + fixture.detectChanges(); + expect(fixture.nativeElement.innerText).toContain('new-newer'); + }); + + it('refreshes root view until it is no longer dirty', () => { + const val = signal(0); + let incrementAfterCheckedUntil = 0; + @Component({ + template: '', + selector: 'child', + standalone: true, + }) + class Child { + ngDoCheck() { + // Update signal in parent view every time we check the child view + // (ExpressionChangedAfterItWasCheckedError but not for signals) + if (val() < incrementAfterCheckedUntil) { + val.update(v => ++v); + } + } + } + @Component({template: '{{val()}}', standalone: true, imports: [Child]}) + class App { + val = val; + } + + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + expect(fixture.nativeElement.innerText).toContain('0'); + + incrementAfterCheckedUntil = 10; + fixture.detectChanges(); + expect(fixture.nativeElement.innerText).toContain('10'); + + incrementAfterCheckedUntil = Number.MAX_SAFE_INTEGER; + expect(() => fixture.detectChanges()).toThrowError(/Infinite/); + }); }); @@ -272,6 +353,37 @@ describe('OnPush components with signals', () => { expect(instance.numTemplateExecutions).toBe(1); }); + it('can read a signal in a host binding in root view', () => { + const useBlue = signal(false); + @Component({ + template: `{{incrementTemplateExecutions()}}`, + selector: 'child', + host: {'[class.blue]': 'useBlue()'}, + changeDetection: ChangeDetectionStrategy.OnPush, + standalone: true, + }) + class MyCmp { + useBlue = useBlue; + + numTemplateExecutions = 0; + incrementTemplateExecutions() { + this.numTemplateExecutions++; + return ''; + } + } + + const fixture = TestBed.createComponent(MyCmp); + + fixture.detectChanges(); + expect(fixture.nativeElement.outerHTML).not.toContain('blue'); + expect(fixture.componentInstance.numTemplateExecutions).not.toContain(1); + + useBlue.set(true); + fixture.detectChanges(); + expect(fixture.nativeElement.outerHTML).toContain('blue'); + expect(fixture.componentInstance.numTemplateExecutions).not.toContain(1); + }); + it('can read a signal in a host binding', () => { @Component({ template: `{{incrementTemplateExecutions()}}`, @@ -440,4 +552,206 @@ describe('OnPush components with signals', () => { .toHaveBeenCalledWith(jasmine.stringMatching( /will likely result in ExpressionChangedAfterItHasBeenChecked/)); }); + + it('does not refresh view if signal marked dirty but did not change', () => { + const val = signal('initial', {equal: () => true}); + + @Component({ + template: '{{val()}}{{incrementChecks()}}', + standalone: true, + changeDetection: ChangeDetectionStrategy.OnPush, + }) + class App { + val = val; + templateExecutions = 0; + incrementChecks() { + this.templateExecutions++; + } + } + + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + expect(fixture.componentInstance.templateExecutions).toBe(1); + expect(fixture.nativeElement.innerText).toContain('initial'); + + val.set('new'); + fixture.detectChanges(); + expect(fixture.componentInstance.templateExecutions).toBe(1); + expect(fixture.nativeElement.innerText).toContain('initial'); + }); + + describe('embedded views', () => { + it('refreshes an embedded view in a component', () => { + @Component({ + selector: 'signal-component', + changeDetection: ChangeDetectionStrategy.OnPush, + standalone: true, + imports: [NgIf], + template: `
{{value()}}
`, + }) + class SignalComponent { + value = signal('initial'); + } + + const fixture = TestBed.createComponent(SignalComponent); + fixture.detectChanges(); + fixture.componentInstance.value.set('new'); + fixture.detectChanges(); + expect(trim(fixture.nativeElement.textContent)).toEqual('new'); + }); + + it('refreshes multiple embedded views in a component', () => { + @Component({ + selector: 'signal-component', + changeDetection: ChangeDetectionStrategy.OnPush, + standalone: true, + imports: [NgFor], + template: `
{{value()}}
`, + }) + class SignalComponent { + value = signal('initial'); + } + + const fixture = TestBed.createComponent(SignalComponent); + fixture.detectChanges(); + fixture.componentInstance.value.set('new'); + fixture.detectChanges(); + expect(trim(fixture.nativeElement.textContent)).toEqual('new new new'); + }); + + + it('refreshes entire component, including embedded views, when signal updates', () => { + @Component({ + selector: 'signal-component', + changeDetection: ChangeDetectionStrategy.OnPush, + standalone: true, + imports: [NgIf], + template: ` + {{componentSignal()}} +
{{incrementExecutions()}}
+ `, + }) + class SignalComponent { + embeddedViewExecutions = 0; + componentSignal = signal('initial'); + incrementExecutions() { + this.embeddedViewExecutions++; + return ''; + } + } + + const fixture = TestBed.createComponent(SignalComponent); + fixture.detectChanges(); + expect(fixture.componentInstance.embeddedViewExecutions).toEqual(1); + + fixture.componentInstance.componentSignal.set('new'); + fixture.detectChanges(); + expect(trim(fixture.nativeElement.textContent)).toEqual('new'); + // OnPush/Default components are checked as a whole so the embedded view is also checked again + expect(fixture.componentInstance.embeddedViewExecutions).toEqual(2); + }); + + + it('re-executes deep embedded template if signal updates', () => { + @Component({ + selector: 'signal-component', + standalone: true, + changeDetection: ChangeDetectionStrategy.OnPush, + imports: [NgIf], + template: ` +
+
+
+ {{value()}} +
+
+
+ `, + }) + class SignalComponent { + value = signal('initial'); + } + + const fixture = TestBed.createComponent(SignalComponent); + fixture.detectChanges(); + + fixture.componentInstance.value.set('new'); + fixture.detectChanges(); + expect(trim(fixture.nativeElement.textContent)).toEqual('new'); + }); + }); + + describe('shielded by non-dirty OnPush', () => { + @Component({ + selector: 'signal-component', + changeDetection: ChangeDetectionStrategy.OnPush, + standalone: true, + template: `{{value()}}`, + }) + class SignalComponent { + value = signal('initial'); + afterViewCheckedRuns = 0; + constructor(readonly cdr: ChangeDetectorRef) {} + ngAfterViewChecked() { + this.afterViewCheckedRuns++; + } + } + + @Component({ + selector: 'on-push-parent', + template: `{{incrementChecks()}}`, + changeDetection: ChangeDetectionStrategy.OnPush, + standalone: true, + imports: [SignalComponent], + }) + class OnPushParent { + @ViewChild(SignalComponent) signalChild!: SignalComponent; + viewExecutions = 0; + + constructor(readonly cdr: ChangeDetectorRef) {} + incrementChecks() { + this.viewExecutions++; + } + } + + it('refreshes when signal changes, but does not refresh non-dirty parent', () => { + const fixture = TestBed.createComponent(OnPushParent); + fixture.detectChanges(); + expect(fixture.componentInstance.viewExecutions).toEqual(1); + fixture.componentInstance.signalChild.value.set('new'); + fixture.detectChanges(); + expect(fixture.componentInstance.viewExecutions).toEqual(1); + expect(trim(fixture.nativeElement.textContent)).toEqual('new'); + }); + + it('does not refresh when detached', () => { + const fixture = TestBed.createComponent(OnPushParent); + fixture.detectChanges(); + fixture.componentInstance.signalChild.value.set('new'); + fixture.componentInstance.signalChild.cdr.detach(); + fixture.detectChanges(); + expect(trim(fixture.nativeElement.textContent)).toEqual('initial'); + }); + + // Note: Design decision for signals because that's how the hooks work today + // We have considered actually running a component's `afterViewChecked` hook if it's refreshed + // in targeted mode (meaning the parent did not refresh) and could change this decision. + it('does not run afterViewChecked hooks because parent view was not dirty (those hooks are executed by the parent)', + () => { + const fixture = TestBed.createComponent(OnPushParent); + fixture.detectChanges(); + // hook run once on initialization + expect(fixture.componentInstance.signalChild.afterViewCheckedRuns).toBe(1); + fixture.componentInstance.signalChild.value.set('new'); + fixture.detectChanges(); + expect(trim(fixture.nativeElement.textContent)).toEqual('new'); + // hook did not run again because host view was not refreshed + expect(fixture.componentInstance.signalChild.afterViewCheckedRuns).toBe(1); + }); + }); }); + + +function trim(text: string|null): string { + return text ? text.replace(/[\s\n]+/gm, ' ').trim() : ''; +} diff --git a/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts b/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts index 600b92b2f1871..51c8c0db8d94c 100644 --- a/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts +++ b/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts @@ -7,14 +7,18 @@ */ import {CommonModule} from '@angular/common'; -import {ChangeDetectionStrategy, ChangeDetectorRef, Component, DoCheck, inject, Input, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/core'; +import {ChangeDetectionStrategy, ChangeDetectorRef, Component, computed, Directive, DoCheck, inject, Input, signal, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/core'; import {AfterViewChecked, EmbeddedViewRef} from '@angular/core/src/core'; import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {expect} from '@angular/platform-browser/testing/src/matchers'; describe('change detection for transplanted views', () => { describe('when declaration appears before insertion', () => { - const insertCompTemplate = ` - InsertComp({{greeting}}) + @Component({ + selector: 'onpush-insert-comp', + changeDetection: ChangeDetectionStrategy.OnPush, + template: ` + OnPushInsertComp({{greeting}})
{ [ngTemplateOutletContext]="{$implicit: greeting}">
- `; - @Component({ - selector: 'insert-comp', - changeDetection: ChangeDetectionStrategy.OnPush, - template: insertCompTemplate, + `, }) - class InsertComp implements DoCheck, AfterViewChecked { + abstract class OnPushInsertComp implements DoCheck, AfterViewChecked { get template(): TemplateRef { - return declareComp.myTmpl; + return templateRef; } greeting: string = 'Hello'; constructor(public changeDetectorRef: ChangeDetectorRef) { - if (!(this instanceof InsertForOnPushDeclareComp)) { - insertComp = this; - } + onPushInsertComp = this; } ngDoCheck(): void { logValue = 'Insert'; @@ -46,50 +44,42 @@ describe('change detection for transplanted views', () => { } } - @Component({ - selector: 'insert-for-onpush-declare-comp', - changeDetection: ChangeDetectionStrategy.OnPush, - template: insertCompTemplate, - }) - class InsertForOnPushDeclareComp extends InsertComp { - constructor(changeDetectorRef: ChangeDetectorRef) { - super(changeDetectorRef); - insertForOnPushDeclareComp = this; - } - override get template(): TemplateRef { - return onPushDeclareComp.myTmpl; - } - } - - @Component({ - selector: `declare-comp`, - template: ` - DeclareComp({{name}}) - - {{greeting}} {{logName()}}! - - ` - }) - class DeclareComp implements DoCheck, AfterViewChecked { + @Directive({}) + abstract class DeclareComp implements DoCheck, AfterViewChecked { @ViewChild('myTmpl') myTmpl!: TemplateRef; name: string = 'world'; - constructor(readonly changeDetector: ChangeDetectorRef) { - if (!(this instanceof OnPushDeclareComp)) { - declareComp = this; - } - } + constructor(readonly changeDetector: ChangeDetectorRef) {} ngDoCheck(): void { logValue = 'Declare'; } logName() { // This will log when the embedded view gets CD. The `logValue` will show if the CD was // from `Insert` or from `Declare` component. - log.push(logValue!); + viewExecutionLog.push(logValue!); return this.name; } ngAfterViewChecked(): void { logValue = null; } + ngAfterViewInit() { + templateRef = this.myTmpl; + } + } + + @Component({ + selector: `check-always-declare-comp`, + template: ` + DeclareComp({{name}}) + + {{greeting}} {{logName()}}! + + ` + }) + class CheckAlwaysDeclareComp extends DeclareComp { + constructor(changeDetector: ChangeDetectorRef) { + super(changeDetector); + declareComp = this; + } } @Component({ @@ -98,8 +88,7 @@ describe('change detection for transplanted views', () => { OnPushDeclareComp({{name}}) {{greeting}} {{logName()}}! - - `, + `, changeDetection: ChangeDetectionStrategy.OnPush }) class OnPushDeclareComp extends DeclareComp { @@ -109,180 +98,280 @@ describe('change detection for transplanted views', () => { } } + @Component({ + selector: `signal-onpush-declare-comp`, + template: ` + SignalOnPushDeclareComp({{name()}}) + + {{greeting}} {{surname()}}{{logExecutionContext()}}! + + `, + changeDetection: ChangeDetectionStrategy.OnPush + }) + class SignalOnPushDeclareComp { + @ViewChild('myTmpl') myTmpl!: TemplateRef; + + name = signal('world'); + templateName = signal('templateName'); + + surname = computed(() => { + const name = this.templateName(); + return name; + }); + + logExecutionContext() { + viewExecutionLog.push(logValue); + return ''; + } + + constructor() { + signalDeclareComp = this; + } + + ngAfterViewChecked() { + logValue = null; + } + ngAfterViewInit() { + templateRef = this.myTmpl; + } + } @Component({ template: ` - - - - + + + + + ` }) class AppComp { - showDeclare: boolean = false; - showOnPushDeclare: boolean = false; - showInsert: boolean = false; - showInsertForOnPushDeclare: boolean = false; + showCheckAlwaysDeclare = false; + showSignalOnPushDeclare = false; + showOnPushDeclare = false; + showOnPushInsert = false; constructor() { appComp = this; } } - let log!: Array; + let viewExecutionLog!: Array; let logValue!: string|null; let fixture!: ComponentFixture; let appComp!: AppComp; - let insertComp!: InsertComp; - let insertForOnPushDeclareComp!: InsertForOnPushDeclareComp; - let declareComp!: DeclareComp; + let onPushInsertComp!: OnPushInsertComp; + let declareComp!: CheckAlwaysDeclareComp; + let templateRef: TemplateRef; let onPushDeclareComp!: OnPushDeclareComp; + let signalDeclareComp!: SignalOnPushDeclareComp; beforeEach(() => { TestBed.configureTestingModule({ - declarations: - [InsertComp, DeclareComp, OnPushDeclareComp, InsertForOnPushDeclareComp, AppComp], + declarations: [ + OnPushInsertComp, SignalOnPushDeclareComp, CheckAlwaysDeclareComp, OnPushDeclareComp, + AppComp + ], imports: [CommonModule], }); - log = []; + viewExecutionLog = []; fixture = TestBed.createComponent(AppComp); }); + describe('and declaration component is Onpush with signals and insertion is OnPush', () => { + beforeEach(() => { + fixture.componentInstance.showSignalOnPushDeclare = true; + fixture.componentInstance.showOnPushInsert = true; + fixture.detectChanges(false); + viewExecutionLog.length = 0; + }); + + it('should set up the component under test correctly', () => { + expect(viewExecutionLog.length).toEqual(0); + expect(trim(fixture.nativeElement.textContent)) + .toEqual('SignalOnPushDeclareComp(world) OnPushInsertComp(Hello) Hello templateName!'); + }); + + it('should CD at insertion and declaration', () => { + signalDeclareComp.name.set('Angular'); + fixture.detectChanges(false); + expect(viewExecutionLog).toEqual(['Insert']); + viewExecutionLog.length = 0; + expect(trim(fixture.nativeElement.textContent)) + .withContext( + 'CD did not run on the transplanted template because it is inside an OnPush component and no signal changed') + .toEqual( + 'SignalOnPushDeclareComp(Angular) OnPushInsertComp(Hello) Hello templateName!'); + + onPushInsertComp.greeting = 'Hi'; + fixture.detectChanges(false); + expect(viewExecutionLog).toEqual([]); + viewExecutionLog.length = 0; + expect(trim(fixture.nativeElement.textContent)) + .withContext('Insertion component is OnPush.') + .toEqual( + 'SignalOnPushDeclareComp(Angular) OnPushInsertComp(Hello) Hello templateName!'); + + onPushInsertComp.changeDetectorRef.markForCheck(); + fixture.detectChanges(false); + expect(viewExecutionLog).toEqual(['Insert']); + viewExecutionLog.length = 0; + expect(trim(fixture.nativeElement.textContent)) + .toEqual('SignalOnPushDeclareComp(Angular) OnPushInsertComp(Hi) Hi templateName!'); + + // Destroy insertion should also destroy declaration + appComp.showOnPushInsert = false; + fixture.detectChanges(false); + expect(viewExecutionLog).toEqual([]); + viewExecutionLog.length = 0; + expect(trim(fixture.nativeElement.textContent)).toEqual('SignalOnPushDeclareComp(Angular)'); + + // Restore both + appComp.showOnPushInsert = true; + fixture.detectChanges(false); + expect(viewExecutionLog).toEqual(['Insert']); + viewExecutionLog.length = 0; + expect(trim(fixture.nativeElement.textContent)) + .toEqual( + 'SignalOnPushDeclareComp(Angular) OnPushInsertComp(Hello) Hello templateName!'); + }); + }); + describe('and declaration component is CheckAlways', () => { beforeEach(() => { - fixture.componentInstance.showDeclare = true; - fixture.componentInstance.showInsert = true; + fixture.componentInstance.showCheckAlwaysDeclare = true; + fixture.componentInstance.showOnPushInsert = true; fixture.detectChanges(false); - log.length = 0; + viewExecutionLog.length = 0; }); it('should set up the component under test correctly', () => { - expect(log.length).toEqual(0); + expect(viewExecutionLog.length).toEqual(0); expect(trim(fixture.nativeElement.textContent)) - .toEqual('DeclareComp(world) InsertComp(Hello) Hello world!'); + .toEqual('DeclareComp(world) OnPushInsertComp(Hello) Hello world!'); }); it('should CD at insertion point only', () => { declareComp.name = 'Angular'; fixture.detectChanges(false); - expect(log).toEqual(['Insert']); - log.length = 0; + expect(viewExecutionLog).toEqual(['Insert']); + viewExecutionLog.length = 0; expect(trim(fixture.nativeElement.textContent)) .toEqual( - 'DeclareComp(Angular) InsertComp(Hello) Hello Angular!', + 'DeclareComp(Angular) OnPushInsertComp(Hello) Hello Angular!', 'Expect transplanted LView to be CD because the declaration is CD.'); - insertComp.greeting = 'Hi'; + onPushInsertComp.greeting = 'Hi'; fixture.detectChanges(false); - expect(log).toEqual(['Insert']); - log.length = 0; + expect(viewExecutionLog).toEqual(['Insert']); + viewExecutionLog.length = 0; expect(trim(fixture.nativeElement.textContent)) .toEqual( - 'DeclareComp(Angular) InsertComp(Hello) Hello Angular!', + 'DeclareComp(Angular) OnPushInsertComp(Hello) Hello Angular!', 'expect no change because it is on push.'); - insertComp.changeDetectorRef.markForCheck(); + onPushInsertComp.changeDetectorRef.markForCheck(); fixture.detectChanges(false); - expect(log).toEqual(['Insert']); - log.length = 0; + expect(viewExecutionLog).toEqual(['Insert']); + viewExecutionLog.length = 0; expect(trim(fixture.nativeElement.textContent)) - .toEqual('DeclareComp(Angular) InsertComp(Hi) Hi Angular!'); + .toEqual('DeclareComp(Angular) OnPushInsertComp(Hi) Hi Angular!'); // Destroy insertion should also destroy declaration - appComp.showInsert = false; + appComp.showOnPushInsert = false; fixture.detectChanges(false); - expect(log).toEqual([]); - log.length = 0; + expect(viewExecutionLog).toEqual([]); + viewExecutionLog.length = 0; expect(trim(fixture.nativeElement.textContent)).toEqual('DeclareComp(Angular)'); // Restore both - appComp.showInsert = true; + appComp.showOnPushInsert = true; fixture.detectChanges(false); - expect(log).toEqual(['Insert']); - log.length = 0; + expect(viewExecutionLog).toEqual(['Insert']); + viewExecutionLog.length = 0; expect(trim(fixture.nativeElement.textContent)) - .toEqual('DeclareComp(Angular) InsertComp(Hello) Hello Angular!'); + .toEqual('DeclareComp(Angular) OnPushInsertComp(Hello) Hello Angular!'); // Destroy declaration, But we should still be able to see updates in insertion - appComp.showDeclare = false; - insertComp.greeting = 'Hello'; - insertComp.changeDetectorRef.markForCheck(); + appComp.showCheckAlwaysDeclare = false; + onPushInsertComp.greeting = 'Hello'; + onPushInsertComp.changeDetectorRef.markForCheck(); fixture.detectChanges(false); - expect(log).toEqual(['Insert']); - log.length = 0; - expect(trim(fixture.nativeElement.textContent)).toEqual('InsertComp(Hello) Hello Angular!'); + expect(viewExecutionLog).toEqual(['Insert']); + viewExecutionLog.length = 0; + expect(trim(fixture.nativeElement.textContent)) + .toEqual('OnPushInsertComp(Hello) Hello Angular!'); }); it('is not checked if detectChanges is called in declaration component', () => { declareComp.name = 'Angular'; declareComp.changeDetector.detectChanges(); - expect(log).toEqual([]); - log.length = 0; + expect(viewExecutionLog).toEqual([]); + viewExecutionLog.length = 0; expect(trim(fixture.nativeElement.textContent)) - .toEqual('DeclareComp(Angular) InsertComp(Hello) Hello world!'); + .toEqual('DeclareComp(Angular) OnPushInsertComp(Hello) Hello world!'); }); it('is checked as part of CheckNoChanges pass', () => { fixture.detectChanges(true); - expect(log).toEqual(['Insert', null /* logName set to null afterViewChecked */]); - log.length = 0; + expect(viewExecutionLog) + .toEqual(['Insert', null /* logName set to null afterViewChecked */]); + viewExecutionLog.length = 0; expect(trim(fixture.nativeElement.textContent)) - .toEqual('DeclareComp(world) InsertComp(Hello) Hello world!'); + .toEqual('DeclareComp(world) OnPushInsertComp(Hello) Hello world!'); }); }); - describe('and declaration component is OnPush', () => { + describe('and declaration and insertion components are OnPush', () => { beforeEach(() => { fixture.componentInstance.showOnPushDeclare = true; - fixture.componentInstance.showInsertForOnPushDeclare = true; + fixture.componentInstance.showOnPushInsert = true; fixture.detectChanges(false); - log.length = 0; + viewExecutionLog.length = 0; }); it('should set up component under test correctly', () => { - expect(log.length).toEqual(0); + expect(viewExecutionLog.length).toEqual(0); expect(trim(fixture.nativeElement.textContent)) - .toEqual('OnPushDeclareComp(world) InsertComp(Hello) Hello world!'); + .toEqual('OnPushDeclareComp(world) OnPushInsertComp(Hello) Hello world!'); }); - it('should not check anything no views are dirty', () => { + it('should not check anything when no views are dirty', () => { fixture.detectChanges(false); - expect(log).toEqual([]); + expect(viewExecutionLog).toEqual([]); }); it('should CD at insertion point only', () => { onPushDeclareComp.name = 'Angular'; - insertForOnPushDeclareComp.greeting = 'Hi'; + onPushInsertComp.greeting = 'Hi'; // mark declaration point dirty onPushDeclareComp.changeDetector.markForCheck(); fixture.detectChanges(false); - expect(log).toEqual(['Insert']); - log.length = 0; + expect(viewExecutionLog).toEqual(['Insert']); + viewExecutionLog.length = 0; expect(trim(fixture.nativeElement.textContent)) - .toEqual('OnPushDeclareComp(Angular) InsertComp(Hello) Hello Angular!'); + .toEqual('OnPushDeclareComp(Angular) OnPushInsertComp(Hello) Hello Angular!'); // mark insertion point dirty - insertForOnPushDeclareComp.changeDetectorRef.markForCheck(); + onPushInsertComp.changeDetectorRef.markForCheck(); fixture.detectChanges(false); - expect(log).toEqual(['Insert']); - log.length = 0; + expect(viewExecutionLog).toEqual(['Insert']); + viewExecutionLog.length = 0; expect(trim(fixture.nativeElement.textContent)) - .toEqual('OnPushDeclareComp(Angular) InsertComp(Hi) Hi Angular!'); + .toEqual('OnPushDeclareComp(Angular) OnPushInsertComp(Hi) Hi Angular!'); // mark both insertion and declaration point dirty - insertForOnPushDeclareComp.changeDetectorRef.markForCheck(); + onPushInsertComp.changeDetectorRef.markForCheck(); onPushDeclareComp.changeDetector.markForCheck(); fixture.detectChanges(false); - expect(log).toEqual(['Insert']); - log.length = 0; + expect(viewExecutionLog).toEqual(['Insert']); + viewExecutionLog.length = 0; }); - it('is not checked if detectChanges is called in declaration component', () => { + it('is checked if detectChanges is called in declaration component', () => { onPushDeclareComp.name = 'Angular'; onPushDeclareComp.changeDetector.detectChanges(); - expect(log).toEqual([]); - log.length = 0; expect(trim(fixture.nativeElement.textContent)) - .toEqual('OnPushDeclareComp(Angular) InsertComp(Hello) Hello world!'); + .toEqual('OnPushDeclareComp(Angular) OnPushInsertComp(Hello) Hello world!'); }); // TODO(FW-1774): blocked by https://github.com/angular/angular/pull/34443 @@ -290,21 +379,22 @@ describe('change detection for transplanted views', () => { // mark declaration point dirty onPushDeclareComp.changeDetector.markForCheck(); fixture.detectChanges(false); - expect(log).toEqual(['Insert', null /* logName set to null in afterViewChecked */]); - log.length = 0; + expect(viewExecutionLog) + .toEqual(['Insert', null /* logName set to null in afterViewChecked */]); + viewExecutionLog.length = 0; // mark insertion point dirty - insertForOnPushDeclareComp.changeDetectorRef.markForCheck(); + onPushInsertComp.changeDetectorRef.markForCheck(); fixture.detectChanges(false); - expect(log).toEqual(['Insert', null]); - log.length = 0; + expect(viewExecutionLog).toEqual(['Insert', null]); + viewExecutionLog.length = 0; // mark both insertion and declaration point dirty - insertForOnPushDeclareComp.changeDetectorRef.markForCheck(); + onPushInsertComp.changeDetectorRef.markForCheck(); onPushDeclareComp.changeDetector.markForCheck(); fixture.detectChanges(false); - expect(log).toEqual(['Insert', null]); - log.length = 0; + expect(viewExecutionLog).toEqual(['Insert', null]); + viewExecutionLog.length = 0; }); it('does not cause infinite change detection if transplanted view is dirty and destroyed before refresh', @@ -312,13 +402,13 @@ describe('change detection for transplanted views', () => { // mark declaration point dirty onPushDeclareComp.changeDetector.markForCheck(); // detach insertion so the transplanted view doesn't get refreshed when CD runs - insertForOnPushDeclareComp.changeDetectorRef.detach(); + onPushInsertComp.changeDetectorRef.detach(); // run CD, which will set the `RefreshView` flag on the transplanted view fixture.detectChanges(false); // reattach insertion so the DESCENDANT_VIEWS counters update - insertForOnPushDeclareComp.changeDetectorRef.reattach(); + onPushInsertComp.changeDetectorRef.reattach(); // make it so the insertion is destroyed before getting refreshed - fixture.componentInstance.showInsertForOnPushDeclare = false; + fixture.componentInstance.showOnPushInsert = false; // run CD again. If we didn't clear the flag/counters when destroying the view, this // would cause an infinite CD because the counters will be >1 but we will never reach a // view to refresh and decrement the counters. @@ -614,9 +704,9 @@ describe('change detection for transplanted views', () => { constructor( readonly rootViewContainerRef: ViewContainerRef, readonly cdr: ChangeDetectorRef) {} - templateExecutions = 0; + checks = 0; incrementChecks() { - this.templateExecutions++; + this.checks++; } } @@ -639,13 +729,13 @@ describe('change detection for transplanted views', () => { component.cdr.detectChanges(); // The template should not have been refreshed because it was inserted "above" the component // so `detectChanges` will not refresh it. - expect(component.templateExecutions).toEqual(0); + expect(component.checks).toEqual(0); // Detach view, manually call `detectChanges`, and verify the template was refreshed component.rootViewContainerRef.detach(); viewRef.detectChanges(); // This view is a backwards reference so it's refreshed twice - expect(component.templateExecutions).toEqual(2); + expect(component.checks).toEqual(2); }); it('should work when change detecting detached transplanted view already marked for refresh', @@ -662,7 +752,7 @@ describe('change detection for transplanted views', () => { viewRef.detectChanges(); }).not.toThrow(); // This view is a backwards reference so it's refreshed twice - expect(component.templateExecutions).toEqual(2); + expect(component.checks).toEqual(2); }); it('should work when re-inserting a previously detached transplanted view marked for refresh', @@ -685,7 +775,7 @@ describe('change detection for transplanted views', () => { // The transplanted view gets refreshed twice because it's actually inserted "backwards" // The view is defined in AppComponent but inserted in its ViewContainerRef (as an // embedded view in AppComponent's host view). - expect(component.templateExecutions).toEqual(2); + expect(component.checks).toEqual(2); }); it('should work when detaching an attached transplanted view with the refresh flag', () => {