Elgato_dark/angular
1
0
Fork 0
mirror of https://github.com/angular/angular synced 2026-05-24 09:28:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

refactor(core): Use flag instead of counter for dirty child transplanted views (#51515)

Browse source 
This commit updates the tracking of dirty child views to be a flag
rather than a counter. This is a much more simple method and less likely
to get into the same 'always-wrong' situation that could happen with the
counter (if it is off by 1 once, it's off by 1 forever and you either
get infinite change detection or your view is never refreshed).

PR Close #51515
This commit is contained in:
Andrew Scott 2023-04-27 14:53:42 -07:00 committed by Andrew Scott
parent 11bb19cafc
commit 0ec66b85e6
18 changed files with 451 additions and 190 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

47
packages/core/src/render3/instructions/change_detection.ts
View file

@ -10,12 +10,12 @@ import {assertDefined, assertEqual} from '../../util/assert';
import {assertLContainer} from '../assert';
import {getComponentViewByInstance} from '../context_discovery';
import {executeCheckHooks, executeInitAndCheckHooks, incrementInitPhaseFlags} from '../hooks';
import {CONTAINER_HEADER_OFFSET, HAS_TRANSPLANTED_VIEWS, LContainer, MOVED_VIEWS} from '../interfaces/container';
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, DESCENDANT_VIEWS_TO_REFRESH, ENVIRONMENT, FLAGS, InitPhaseState, LView, LViewFlags, PARENT, TVIEW, TView} from '../interfaces/view';
import {CONTEXT, ENVIRONMENT, FLAGS, InitPhaseState, LView, LViewFlags, PARENT, TVIEW, TView} from '../interfaces/view';
import {enterView, isInCheckNoChangesMode, leaveView, setBindingIndex, setIsInCheckNoChangesMode} from '../state';
import {getFirstLContainer, getNextLContainer} from '../util/view_traversal_utils';
import {clearViewRefreshFlag, getComponentLViewByIndex, isCreationMode, markViewForRefresh, resetPreOrderHookFlags, viewAttachedToChangeDetector} from '../util/view_utils';
import {getComponentLViewByIndex, isCreationMode, markAncestorsForTraversal, markViewForRefresh, resetPreOrderHookFlags, viewAttachedToChangeDetector} from '../util/view_utils';
import {executeTemplate, executeViewQueryFn, handleError, processHostBindingOpCodes, refreshContentQueries} from './shared';
@ -228,7 +228,15 @@ export function refreshView<T>(
if (!isInCheckNoChangesPass) {
lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass);
}
clearViewRefreshFlag(lView);
lView[FLAGS] &= ~LViewFlags.RefreshView;
} catch (e) {
// If refreshing a view causes an error, we need to remark the ancestors as needing traversal
// because the error might have caused a situation where views below the current location are
// dirty but will be unreachable because the "has dirty children" flag in the ancestors has been
// cleared during change detection and we failed to run to completion.
markAncestorsForTraversal(lView);
throw e;
} finally {
leaveView();
}
@ -241,9 +249,10 @@ export function refreshView<T>(
function detectChangesInEmbeddedViews(lView: LView, mode: ChangeDetectionMode) {
for (let lContainer = getFirstLContainer(lView); lContainer !== null;
lContainer = getNextLContainer(lContainer)) {
lContainer[HAS_CHILD_VIEWS_TO_REFRESH] = false;
for (let i = CONTAINER_HEADER_OFFSET; i < lContainer.length; i++) {
const embeddedLView = lContainer[i];
detectChangesInView(embeddedLView, mode);
detectChangesInViewIfAttached(embeddedLView, mode);
}
}
}
@ -279,34 +288,44 @@ function detectChangesInComponent(
hostLView: LView, componentHostIdx: number, mode: ChangeDetectionMode): void {
ngDevMode && assertEqual(isCreationMode(hostLView), false, 'Should be run in update mode');
const componentView = getComponentLViewByIndex(componentHostIdx, hostLView);
detectChangesInView(componentView, mode);
detectChangesInViewIfAttached(componentView, mode);
}
/**
* Visits a view as part of change detection traversal.
*
* - If the view is detached, no additional traversal happens.
* If the view is detached, no additional traversal happens.
*/
function detectChangesInViewIfAttached(lView: LView, mode: ChangeDetectionMode) {
if (!viewAttachedToChangeDetector(lView)) {
return;
}
detectChangesInView(lView, mode);
}
/**
* Visits a view as part of change detection traversal.
*
* The view is refreshed if:
* - If the view is CheckAlways or Dirty and ChangeDetectionMode is `Global`
* - If the view has the `RefreshTransplantedView` flag
*
* The view is not refreshed, but descendants are traversed in `ChangeDetectionMode.Targeted` if the
* view has a non-zero TRANSPLANTED_VIEWS_TO_REFRESH counter.
*
* view HasChildViewsToRefresh flag is set.
*/
function detectChangesInView(lView: LView, mode: ChangeDetectionMode) {
if (!viewAttachedToChangeDetector(lView)) {
return;
}
const tView = lView[TVIEW];
const flags = lView[FLAGS];
// Flag cleared before change detection runs so that the view can be re-marked for traversal if
// necessary.
lView[FLAGS] &= ~LViewFlags.HasChildViewsToRefresh;
if ((flags & (LViewFlags.CheckAlways | LViewFlags.Dirty) &&
mode === ChangeDetectionMode.Global) ||
flags & LViewFlags.RefreshView) {
refreshView(tView, lView, tView.template, lView[CONTEXT]);
} else if (lView[DESCENDANT_VIEWS_TO_REFRESH] > 0) {
} else if (flags & LViewFlags.HasChildViewsToRefresh) {
detectChangesInEmbeddedViews(lView, ChangeDetectionMode.Targeted);
const components = tView.components;
if (components !== null) {

2
packages/core/src/render3/instructions/shared.ts
View file

@ -1414,8 +1414,8 @@ export function createLContainer(
false, // has transplanted views
currentView, // parent
null, // next
0, // transplanted views to refresh count
tNode, // t_host
false, // has child views to refresh
native, // native,
null, // view refs
null, // moved views

16
packages/core/src/render3/interfaces/container.ts
View file

@ -10,7 +10,7 @@ import {DehydratedContainerView} from '../../hydration/interfaces';
import {TNode} from './node';
import {RComment, RElement} from './renderer_dom';
import {DESCENDANT_VIEWS_TO_REFRESH, HOST, LView, NEXT, PARENT, T_HOST} from './view';
import {HOST, LView, NEXT, PARENT, T_HOST} from './view';
@ -37,18 +37,18 @@ export const TYPE = 1;
*/
export const HAS_TRANSPLANTED_VIEWS = 2;
// PARENT, NEXT, DESCENDANT_VIEWS_TO_REFRESH are indices 3, 4, and 5
// PARENT and NEXT are indices 3 and 4
// As we already have these constants in LView, we don't need to re-create them.
// T_HOST is index 6
// T_HOST is index 5
// We already have this constants in LView, we don't need to re-create it.
export const HAS_CHILD_VIEWS_TO_REFRESH = 6;
export const NATIVE = 7;
export const VIEW_REFS = 8;
export const MOVED_VIEWS = 9;
export const DEHYDRATED_VIEWS = 10;
/**
* Size of LContainer's header. Represents the index after which all views in the
* container will be inserted. We need to keep a record of current views so we know
@ -101,12 +101,10 @@ export interface LContainer extends Array<any> {
[NEXT]: LView|LContainer|null;
/**
* The number of direct transplanted views which need a refresh or have descendants themselves
* that need a refresh but have not marked their ancestors as Dirty. This tells us that during
* change detection we should still descend to find those children to refresh, even if the parents
* are not `Dirty`/`CheckAlways`.
* Indicates that this LContainer has a view underneath it that needs to be refreshed during
* change detection.
*/
[DESCENDANT_VIEWS_TO_REFRESH]: number;
[HAS_CHILD_VIEWS_TO_REFRESH]: boolean;
/**
* A collection of views created based on the underlying `<ng-template>` element but inserted into

31
packages/core/src/render3/interfaces/view.ts
View file

@ -33,10 +33,14 @@ import {TDeferBlockDetails} from './defer';
export const HOST = 0;
export const TVIEW = 1;
export const FLAGS = 2;
// Shared with LContainer
export const PARENT = 3;
export const NEXT = 4;
export const DESCENDANT_VIEWS_TO_REFRESH = 5;
export const T_HOST = 6;
export const T_HOST = 5;
// End shared with LContainer
export const HYDRATION = 6;
export const CLEANUP = 7;
export const CONTEXT = 8;
export const INJECTOR = 9;
@ -53,9 +57,9 @@ export const QUERIES = 18;
export const ID = 19;
export const EMBEDDED_VIEW_INJECTOR = 20;
export const ON_DESTROY_HOOKS = 21;
export const HYDRATION = 22;
export const REACTIVE_TEMPLATE_CONSUMER = 23;
export const REACTIVE_HOST_BINDING_CONSUMER = 24;
/**
* Size of LView's header. Necessary to adjust for it when setting slots.
*
@ -318,14 +322,6 @@ export interface LView<T = unknown> extends Array<any> {
*/
[PREORDER_HOOK_FLAGS]: PreOrderHookFlags;
/**
* The number of direct transplanted views which need a refresh or have descendants themselves
* that need a refresh but have not marked their ancestors as Dirty. This tells us that during
* change detection we should still descend to find those children to refresh, even if the parents
* are not `Dirty`/`CheckAlways`.
*/
[DESCENDANT_VIEWS_TO_REFRESH]: number;
/** Unique ID of the view. Used for `__ngContext__` lookups in the `LView` registry. */
[ID]: number;
@ -424,8 +420,8 @@ export const enum LViewFlags {
/**
* Whether this moved LView was needs to be refreshed. Similar to the Dirty flag, but used for
* transplanted and signal views where the parent/ancestor views are not marked dirty as well.
* i.e. "Refresh just this view". Used in conjunction with the DESCENDANT_VIEWS_TO_REFRESH
* counter.
* i.e. "Refresh just this view". Used in conjunction with the HAS_CHILD_VIEWS_TO_REFRESH
* flag.
*/
RefreshView = 1 << 10,
@ -435,10 +431,17 @@ export const enum LViewFlags {
/** Indicates that the view was created with `signals: true`. */
SignalView = 1 << 12,
/**
* Indicates that this LView has a view underneath it that needs to be refreshed during change
* detection. This flag indicates that even if this view is not dirty itself, we still need to
* traverse its children during change detection.
*/
HasChildViewsToRefresh = 1 << 13,
/**
* This is the count of the bits the 1 was shifted above (base 10)
*/
IndexWithinInitPhaseShift = 13,
IndexWithinInitPhaseShift = 14,
/**
* Index of the current init phase on last 21 bits

8
packages/core/src/render3/node_manipulation.ts
View file

@ -31,7 +31,7 @@ import {assertTNodeType} from './node_assert';
import {profiler, ProfilerEvent} from './profiler';
import {setUpAttributes} from './util/attrs_utils';
import {getLViewParent} from './util/view_traversal_utils';
import {clearViewRefreshFlag, getNativeByTNode, unwrapRNode} from './util/view_utils';
import {getNativeByTNode, unwrapRNode, updateAncestorTraversalFlagsOnAttach} from './util/view_utils';
const enum WalkTNodeTreeAction {
/** node create in the native environment. Run on initial creation. */
@ -275,6 +275,7 @@ export function insertView(tView: TView, lView: LView, lContainer: LContainer, i
lQueries.insertView(tView);
}
updateAncestorTraversalFlagsOnAttach(lView);
// Sets the attached flag
lView[FLAGS] |= LViewFlags.Attached;
}
@ -316,11 +317,6 @@ function detachMovedView(declarationContainer: LContainer, lView: LView) {
const declarationViewIndex = movedViews.indexOf(lView);
const insertionLContainer = lView[PARENT] as LContainer;
ngDevMode && assertLContainer(insertionLContainer);
// If the view was marked for refresh but then detached before it was checked (where the flag
// would be cleared and the counter decremented), we need to update the status here.
clearViewRefreshFlag(lView);
movedViews.splice(declarationViewIndex, 1);
}

69
packages/core/src/render3/util/view_utils.ts
View file

@ -9,11 +9,11 @@
import {RuntimeError, RuntimeErrorCode} from '../../errors';
import {assertDefined, assertGreaterThan, assertGreaterThanOrEqual, assertIndexInRange, assertLessThan} from '../../util/assert';
import {assertTNode, assertTNodeForLView} from '../assert';
import {LContainer, TYPE} from '../interfaces/container';
import {HAS_CHILD_VIEWS_TO_REFRESH, LContainer, TYPE} from '../interfaces/container';
import {TConstants, TNode} from '../interfaces/node';
import {RNode} from '../interfaces/renderer_dom';
import {isLContainer, isLView} from '../interfaces/type_checks';
import {DECLARATION_VIEW, DESCENDANT_VIEWS_TO_REFRESH, FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, ON_DESTROY_HOOKS, PARENT, PREORDER_HOOK_FLAGS, PreOrderHookFlags, TData, TView} from '../interfaces/view';
import {DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, ON_DESTROY_HOOKS, PARENT, PREORDER_HOOK_FLAGS, PreOrderHookFlags, TData, TView} from '../interfaces/view';
@ -165,24 +165,16 @@ export function resetPreOrderHookFlags(lView: LView) {
}
/**
* Adds the `RefreshView` flag from the lView and updates DESCENDANT_VIEWS_TO_REFRESH counters of
* Adds the `RefreshView` flag from the lView and updates HAS_CHILD_VIEWS_TO_REFRESH flag of
* parents.
*/
export function markViewForRefresh(lView: LView) {
if ((lView[FLAGS] & LViewFlags.RefreshView) === 0) {
lView[FLAGS] |= LViewFlags.RefreshView;
updateViewsToRefresh(lView, 1);
}
}
/**
* Removes the `RefreshView` flag from the lView and updates DESCENDANT_VIEWS_TO_REFRESH counters of
* parents.
*/
export function clearViewRefreshFlag(lView: LView) {
if (lView[FLAGS] & LViewFlags.RefreshView) {
lView[FLAGS] &= ~LViewFlags.RefreshView;
updateViewsToRefresh(lView, -1);
return;
}
lView[FLAGS] |= LViewFlags.RefreshView;
if (viewAttachedToChangeDetector(lView)) {
markAncestorsForTraversal(lView);
}
}
@ -210,20 +202,45 @@ export function walkUpViews(nestingLevel: number, currentView: LView): LView {
* 1. counter goes from 0 to 1, indicating that there is a new child that has a view to refresh
* or
* 2. counter goes from 1 to 0, indicating there are no more descendant views to refresh
* When attaching/re-attaching an `LView` to the change detection tree, we need to ensure that the
* views above it are traversed during change detection if this one is marked for refresh or has
* some child or descendant that needs to be refreshed.
*/
function updateViewsToRefresh(lView: LView, amount: 1|- 1) {
let parent: LView|LContainer|null = lView[PARENT];
export function updateAncestorTraversalFlagsOnAttach(lView: LView) {
if (lView[FLAGS] & (LViewFlags.RefreshView | LViewFlags.HasChildViewsToRefresh)) {
markAncestorsForTraversal(lView);
}
}
/**
* Ensures views above the given `lView` are traversed during change detection even when they are
* not dirty.
*
* This is done by setting the `HAS_CHILD_VIEWS_TO_REFRESH` flag up to the root, stopping when the
* flag is already `true` or the `lView` is detached.
*/
export function markAncestorsForTraversal(lView: LView) {
let parent = lView[PARENT];
if (parent === null) {
return;
}
parent[DESCENDANT_VIEWS_TO_REFRESH] += amount;
let viewOrContainer: LView|LContainer = parent;
parent = parent[PARENT];
while (parent !== null &&
((amount === 1 && viewOrContainer[DESCENDANT_VIEWS_TO_REFRESH] === 1) ||
(amount === -1 && viewOrContainer[DESCENDANT_VIEWS_TO_REFRESH] === 0))) {
parent[DESCENDANT_VIEWS_TO_REFRESH] += amount;
viewOrContainer = parent;
while (parent !== null) {
// We stop adding markers to the ancestors once we reach one that already has the marker. This
// is to avoid needlessly traversing all the way to the root when the marker already exists.
if ((isLContainer(parent) && parent[HAS_CHILD_VIEWS_TO_REFRESH] ||
(isLView(parent) && parent[FLAGS] & LViewFlags.HasChildViewsToRefresh))) {
break;
}
if (isLContainer(parent)) {
parent[HAS_CHILD_VIEWS_TO_REFRESH] = true;
} else {
parent[FLAGS] |= LViewFlags.HasChildViewsToRefresh;
if (!viewAttachedToChangeDetector(parent)) {
break;
}
}
parent = parent[PARENT];
}
}

3
packages/core/src/render3/view_ref.ts
View file

@ -19,7 +19,7 @@ import {CONTAINER_HEADER_OFFSET, VIEW_REFS} from './interfaces/container';
import {isLContainer} from './interfaces/type_checks';
import {CONTEXT, FLAGS, LView, LViewFlags, PARENT, TVIEW} from './interfaces/view';
import {destroyLView, detachView, detachViewFromDOM} from './node_manipulation';
import {storeLViewOnDestroy} from './util/view_utils';
import {storeLViewOnDestroy, updateAncestorTraversalFlagsOnAttach} from './util/view_utils';
// Needed due to tsickle downleveling where multiple `implements` with classes creates
@ -258,6 +258,7 @@ export class ViewRef<T> implements EmbeddedViewRef<T>, InternalViewRef, ChangeDe
* ```
*/
reattach(): void {
updateAncestorTraversalFlagsOnAttach(this._lView);
this._lView[FLAGS] |= LViewFlags.Attached;
}

303
packages/core/test/acceptance/change_detection_transplanted_view_spec.ts
View file

@ -7,8 +7,8 @@
*/
import {CommonModule} from '@angular/common';
import {ChangeDetectionStrategy, ChangeDetectorRef, Component, DoCheck, Input, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/core';
import {AfterViewChecked} from '@angular/core/src/core';
import {ChangeDetectionStrategy, ChangeDetectorRef, Component, DoCheck, inject, Input, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/core';
import {AfterViewChecked, EmbeddedViewRef} from '@angular/core/src/core';
import {ComponentFixture, TestBed} from '@angular/core/testing';
describe('change detection for transplanted views', () => {
@ -306,12 +306,27 @@ describe('change detection for transplanted views', () => {
expect(log).toEqual(['Insert', null]);
log.length = 0;
});
it('does not cause infinite change detection if transplanted view is dirty and destroyed before refresh',
() => {
// mark declaration point dirty
onPushDeclareComp.changeDetector.markForCheck();
// detach insertion so the transplanted view doesn't get refreshed when CD runs
insertForOnPushDeclareComp.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();
// make it so the insertion is destroyed before getting refreshed
fixture.componentInstance.showInsertForOnPushDeclare = 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.
fixture.detectChanges(false);
});
});
});
// Note that backwards references are not handled well in VE or Ivy at the moment.
// These tests assert the current behavior. Some of these would need to be updated
// if future work refreshes backwards referenced transplanted views.
describe('backwards references', () => {
@Component({
selector: 'insertion',
@ -383,7 +398,7 @@ describe('change detection for transplanted views', () => {
});
it('should not update declaration view when there is a change in the declaration and insertion is marked dirty',
it('should update declaration view when there is a change in the declaration and insertion is marked dirty',
() => {
appComponent.declaration.name = 'new name';
appComponent.insertion.changeDetectorRef.markForCheck();
@ -416,33 +431,15 @@ describe('change detection for transplanted views', () => {
expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(1);
});
it('should not update anything when there is a change to insertion and declaration is marked dirty',
() => {
appComponent.insertion.name = 'new name';
appComponent.declaration.changeDetectorRef.markForCheck();
fixture.detectChanges(false);
// Name should not update in insertion view because only declaration was marked dirty
// Context name also does not update in the template because the insertion view needs to be
// checked to update the `ngTemplateOutletContext` input.
expect(fixture.nativeElement.textContent)
.toEqual(
'Insertion(initial)TemplateDeclaration(initial)TemplateContext(initial)Declaration(initial)',
'Name should not update in insertion view because only declaration was marked dirty\n' +
'Context name also does not update in the template because the insertion view needs to be' +
'checked to update the `ngTemplateOutletContext` input.');
// Note here that if backwards references were better supported, we would be able to
// refresh the transplanted view in the first `detectChanges` call but because the
// insertion point was already checked, we need to call detectChanges again to refresh it.
expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(0);
fixture.detectChanges(false);
expect(fixture.nativeElement.textContent)
.toEqual(
'Insertion(initial)TemplateDeclaration(initial)TemplateContext(initial)Declaration(initial)',
'Expected bindings to still be initial values. Again, TemplateContext can only update if ' +
'insertion point is dirty and updates the ngTemplateOutletContext input.');
expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(1);
});
it('should not update when there is a change to insertion and declaration is marked dirty', () => {
appComponent.insertion.name = 'new name';
appComponent.declaration.changeDetectorRef.markForCheck();
fixture.detectChanges(false);
expect(fixture.nativeElement.textContent)
.toEqual(
'Insertion(initial)TemplateDeclaration(initial)TemplateContext(initial)Declaration(initial)');
expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(0);
});
it('should update insertion view and template when there is a change to insertion and insertion marked dirty',
() => {
@ -460,14 +457,13 @@ describe('change detection for transplanted views', () => {
expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(0);
});
it('should only refresh template once when declaration and insertion are marked dirty', () => {
it('should refresh template when declaration and insertion are marked dirty', () => {
appComponent.declaration.changeDetectorRef.markForCheck();
appComponent.insertion.changeDetectorRef.markForCheck();
fixture.detectChanges(false);
expect(appComponent.declaration.transplantedViewRefreshCount)
.toEqual(
1,
'Expected transplanted view to only be refreshed when insertion component is refreshed');
.withContext('Should refresh once because backwards references are not rechecked')
.toEqual(1);
});
});
@ -498,7 +494,17 @@ describe('change detection for transplanted views', () => {
})
class OnPushDeclaration {
@ViewChild(OnPushInsertionHost) onPushInsertionHost?: OnPushInsertionHost;
value = 'initial';
private _value = 'initial';
throwErrorInView = false;
get value() {
if (this.throwErrorInView) {
throw new Error('error getting value in transplanted view');
}
return this._value;
}
set value(v: string) {
this._value = v;
}
constructor(readonly cdr: ChangeDetectorRef) {}
}
@ -523,6 +529,22 @@ describe('change detection for transplanted views', () => {
.createComponent(componentUnderTest);
}
it('can recover from errors thrown during change detection', () => {
const fixture = getFixture(OnPushDeclaration);
fixture.detectChanges();
fixture.componentInstance.value = 'new';
fixture.componentInstance.cdr.markForCheck();
fixture.componentInstance.throwErrorInView = true;
expect(() => {
fixture.detectChanges();
}).toThrow();
// Ensure that the transplanted view can still get refreshed if we rerun change detection
// without the error
fixture.componentInstance.throwErrorInView = false;
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toEqual('new');
});
it('refresh when transplanted view is declared in CheckAlways component', () => {
const fixture = getFixture(CheckAlwaysDeclaration);
fixture.detectChanges();
@ -593,40 +615,113 @@ describe('change detection for transplanted views', () => {
'Expected transplanted view to be refreshed even when insertion is not dirty');
});
it('should not fail when change detecting detached transplanted view', () => {
describe('ViewRef and ViewContainerRef operations', () => {
@Component({template: '<ng-template>{{incrementChecks()}}</ng-template>'})
class AppComponent {
@ViewChild(TemplateRef) templateRef!: TemplateRef<{}>;
constructor(readonly rootVref: ViewContainerRef, readonly cdr: ChangeDetectorRef) {}
constructor(
readonly rootViewContainerRef: ViewContainerRef, readonly cdr: ChangeDetectorRef) {}
checks = 0;
templateExecutions = 0;
incrementChecks() {
this.checks++;
this.templateExecutions++;
}
}
const fixture = TestBed.configureTestingModule({declarations: [AppComponent]})
.createComponent(AppComponent);
const component = fixture.componentInstance;
fixture.detectChanges();
let fixture: ComponentFixture<AppComponent>;
let component: AppComponent;
let viewRef: EmbeddedViewRef<{}>;
beforeEach(() => {
fixture = TestBed.configureTestingModule({declarations: [AppComponent]})
.createComponent(AppComponent);
component = fixture.componentInstance;
fixture.detectChanges();
viewRef = component.templateRef.createEmbeddedView({});
component.rootViewContainerRef.insert(viewRef);
});
const viewRef = component.templateRef.createEmbeddedView({});
// This `ViewContainerRef` is for the root view
component.rootVref.insert(viewRef);
// `detectChanges` on this `ChangeDetectorRef` will refresh this view and children, not the root
// view that has the transplanted `viewRef` inserted.
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.checks).toEqual(0);
it('should not fail when change detecting detached transplanted view', () => {
// This `ViewContainerRef` is for the root view
// `detectChanges` on this `ChangeDetectorRef` will refresh this view and children, not the
// root view that has the transplanted `viewRef` inserted.
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);
// Detach view, manually call `detectChanges`, and verify the template was refreshed
component.rootVref.detach();
viewRef.detectChanges();
expect(component.checks).toEqual(1);
// Detach view, manually call `detectChanges`, and verify the template was refreshed
component.rootViewContainerRef.detach();
viewRef.detectChanges();
expect(component.templateExecutions).toEqual(1);
});
it('should work when change detecting detached transplanted view already marked for refresh',
() => {
// detach the viewRef only. This just removes the LViewFlags.Attached rather than actually
// detaching the view from the container.
viewRef.detach();
// Calling detectChanges marks transplanted views for check
component.cdr.detectChanges();
expect(() => {
// Calling detectChanges on the transplanted view itself will clear the refresh flag. It
// _should not_ also attempt to update the parent counters because it's detached and
// should not affect parent counters.
viewRef.detectChanges();
}).not.toThrow();
expect(component.templateExecutions).toEqual(1);
});
it('should work when re-inserting a previously detached transplanted view marked for refresh',
() => {
// Test case for inserting a view with refresh flag
viewRef.detach();
// mark transplanted views for check but does not refresh transplanted view because it is
// detached
component.cdr.detectChanges();
// reattach view itself
viewRef.reattach();
expect(() => {
// detach and reattach view from ViewContainerRef
component.rootViewContainerRef.detach();
component.rootViewContainerRef.insert(viewRef);
// calling detectChanges will clear the refresh flag. If the above operations messed up
// the counter, this would fail when attempted to decrement.
fixture.detectChanges(false);
}).not.toThrow();
expect(component.templateExecutions).toBeGreaterThan(0);
});
it('should work when detaching an attached transplanted view with the refresh flag', () => {
viewRef.detach();
// mark transplanted views for check but does not refresh transplanted view because it is
// detached
component.cdr.detectChanges();
// reattach view with refresh flag should increment parent counters
viewRef.reattach();
expect(() => {
// detach view with refresh flag should decrement parent counters
viewRef.detach();
// detectChanges on parent should not cause infinite loop if the above counters were updated
// correctly both times.
fixture.detectChanges();
}).not.toThrow();
});
it('should work when destroying a view with the refresh flag', () => {
viewRef.detach();
// mark transplanted views for check but does not refresh transplanted view because it is
// detached
component.cdr.detectChanges();
viewRef.reattach();
expect(() => {
viewRef.destroy();
fixture.detectChanges();
}).not.toThrow();
});
});
describe('when detached', () => {
@Component({
selector: 'on-push-component',
@ -743,6 +838,96 @@ describe('change detection for transplanted views', () => {
expect(appComponent.transplantedViewRefreshCount).toEqual(3);
});
});
it('does not cause error if running change detection on detached view', () => {
@Component({
standalone: true,
selector: 'insertion',
template: `<ng-container #vc></ng-container>`,
})
class Insertion {
@ViewChild('vc', {read: ViewContainerRef, static: true}) viewContainer!: ViewContainerRef;
@Input() template!: TemplateRef<{}>;
ngOnChanges() {
return this.viewContainer.createEmbeddedView(this.template);
}
}
@Component({
standalone: true,
template: `
<ng-template #transplantedTemplate></ng-template>
<insertion [template]="transplantedTemplate"></insertion>
`,
imports: [Insertion]
})
class Root {
readonly cdr = inject(ChangeDetectorRef);
}
const fixture = TestBed.createComponent(Root);
fixture.componentInstance.cdr.detach();
fixture.componentInstance.cdr.detectChanges();
});
it('backwards reference still updated if detaching root during change detection', () => {
@Component({
standalone: true,
selector: 'insertion',
template: `<ng-container #vc></ng-container>`,
changeDetection: ChangeDetectionStrategy.OnPush
})
class Insertion {
@ViewChild('vc', {read: ViewContainerRef, static: true}) viewContainer!: ViewContainerRef;
@Input() template!: TemplateRef<{}>;
ngOnChanges() {
return this.viewContainer.createEmbeddedView(this.template);
}
}
@Component({
template: '<ng-template #template>{{value}}</ng-template>',
selector: 'declaration',
standalone: true,
})
class Declaration {
@ViewChild('template', {static: true}) transplantedTemplate!: TemplateRef<{}>;
@Input() value?: string;
}
@Component({
standalone: true,
template: `
<insertion [template]="declaration?.transplantedTemplate"></insertion>
<declaration [value]="value"></declaration>
{{incrementChecks()}}
`,
imports: [Insertion, Declaration]
})
class Root {
@ViewChild(Declaration, {static: true}) declaration!: Declaration;
readonly cdr = inject(ChangeDetectorRef);
value = 'initial';
templateExecutions = 0;
incrementChecks() {
this.templateExecutions++;
}
}
const fixture = TestBed.createComponent(Root);
fixture.detectChanges(false);
expect(fixture.nativeElement.innerText).toEqual('initial');
expect(fixture.componentInstance.templateExecutions).toEqual(1);
// Root is detached and value in transplanted view updates during CD. Because it is inserted
// backwards, this requires a rerun of the traversal at the root. This test ensures we still
// get the rerun even when the root is detached.
fixture.componentInstance.cdr.detach();
fixture.componentInstance.value = 'new';
fixture.componentInstance.cdr.detectChanges();
expect(fixture.componentInstance.templateExecutions).toEqual(2);
expect(fixture.nativeElement.innerText).toEqual('new');
});
});
});

15
packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json
View file

@ -245,6 +245,9 @@
{
"name": "GenericBrowserDomAdapter"
},
{
"name": "HAS_CHILD_VIEWS_TO_REFRESH"
},
{
"name": "HAS_TRANSPLANTED_VIEWS"
},
@ -713,9 +716,6 @@
{
"name": "cleanUpView"
},
{
"name": "clearViewRefreshFlag"
},
{
"name": "cloakAndComputeStyles"
},
@ -834,7 +834,7 @@
"name": "detectChangesInEmbeddedViews"
},
{
"name": "detectChangesInView"
"name": "detectChangesInViewIfAttached"
},
{
"name": "detectChangesInternal"
@ -1232,6 +1232,9 @@
{
"name": "map"
},
{
"name": "markAncestorsForTraversal"
},
{
"name": "markAsComponentHost"
},
@ -1491,10 +1494,10 @@
"name": "updateMicroTaskStatus"
},
{
"name": "updateViewsToRefresh"
"name": "urlParsingNode"
},
{
"name": "urlParsingNode"
"name": "viewAttachedToChangeDetector"
},
{
"name": "visitDslNode"

15
packages/core/test/bundling/animations/bundle.golden_symbols.json
View file

@ -269,6 +269,9 @@
{
"name": "GenericBrowserDomAdapter"
},
{
"name": "HAS_CHILD_VIEWS_TO_REFRESH"
},
{
"name": "HAS_TRANSPLANTED_VIEWS"
},
@ -773,9 +776,6 @@
{
"name": "cleanUpView"
},
{
"name": "clearViewRefreshFlag"
},
{
"name": "cloakAndComputeStyles"
},
@ -900,7 +900,7 @@
"name": "detectChangesInEmbeddedViews"
},
{
"name": "detectChangesInView"
"name": "detectChangesInViewIfAttached"
},
{
"name": "detectChangesInternal"
@ -1301,6 +1301,9 @@
{
"name": "map"
},
{
"name": "markAncestorsForTraversal"
},
{
"name": "markAsComponentHost"
},
@ -1566,10 +1569,10 @@
"name": "updateMicroTaskStatus"
},
{
"name": "updateViewsToRefresh"
"name": "urlParsingNode"
},
{
"name": "urlParsingNode"
"name": "viewAttachedToChangeDetector"
},
{
"name": "visitDslNode"

15
packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json
View file

@ -164,6 +164,9 @@
{
"name": "GenericBrowserDomAdapter"
},
{
"name": "HAS_CHILD_VIEWS_TO_REFRESH"
},
{
"name": "HAS_TRANSPLANTED_VIEWS"
},
@ -575,9 +578,6 @@
{
"name": "cleanUpView"
},
{
"name": "clearViewRefreshFlag"
},
{
"name": "collectNativeNodes"
},
@ -669,7 +669,7 @@
"name": "detectChangesInEmbeddedViews"
},
{
"name": "detectChangesInView"
"name": "detectChangesInViewIfAttached"
},
{
"name": "detectChangesInternal"
@ -1022,6 +1022,9 @@
{
"name": "map"
},
{
"name": "markAncestorsForTraversal"
},
{
"name": "markAsComponentHost"
},
@ -1239,10 +1242,10 @@
"name": "updateMicroTaskStatus"
},
{
"name": "updateViewsToRefresh"
"name": "urlParsingNode"
},
{
"name": "urlParsingNode"
"name": "viewAttachedToChangeDetector"
},
{
"name": "walkProviderTree"

18
packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json
View file

@ -242,6 +242,9 @@
{
"name": "GenericBrowserDomAdapter"
},
{
"name": "HAS_CHILD_VIEWS_TO_REFRESH"
},
{
"name": "HAS_TRANSPLANTED_VIEWS"
},
@ -779,9 +782,6 @@
{
"name": "cleanUpView"
},
{
"name": "clearViewRefreshFlag"
},
{
"name": "collectNativeNodes"
},
@ -912,7 +912,7 @@
"name": "detectChangesInEmbeddedViews"
},
{
"name": "detectChangesInView"
"name": "detectChangesInViewIfAttached"
},
{
"name": "detectChangesInternal"
@ -1409,6 +1409,9 @@
{
"name": "map"
},
{
"name": "markAncestorsForTraversal"
},
{
"name": "markAsComponentHost"
},
@ -1718,6 +1721,9 @@
{
"name": "unwrapRNode"
},
{
"name": "updateAncestorTraversalFlagsOnAttach"
},
{
"name": "updateControl"
},
@ -1725,10 +1731,10 @@
"name": "updateMicroTaskStatus"
},
{
"name": "updateViewsToRefresh"
"name": "urlParsingNode"
},
{
"name": "urlParsingNode"
"name": "viewAttachedToChangeDetector"
},
{
"name": "walkProviderTree"

18
packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json
View file

@ -227,6 +227,9 @@
{
"name": "GenericBrowserDomAdapter"
},
{
"name": "HAS_CHILD_VIEWS_TO_REFRESH"
},
{
"name": "HAS_TRANSPLANTED_VIEWS"
},
@ -758,9 +761,6 @@
{
"name": "cleanUpView"
},
{
"name": "clearViewRefreshFlag"
},
{
"name": "collectNativeNodes"
},
@ -882,7 +882,7 @@
"name": "detectChangesInEmbeddedViews"
},
{
"name": "detectChangesInView"
"name": "detectChangesInViewIfAttached"
},
{
"name": "detectChangesInternal"
@ -1367,6 +1367,9 @@
{
"name": "map"
},
{
"name": "markAncestorsForTraversal"
},
{
"name": "markAsComponentHost"
},
@ -1694,6 +1697,9 @@
{
"name": "unwrapRNode"
},
{
"name": "updateAncestorTraversalFlagsOnAttach"
},
{
"name": "updateControl"
},
@ -1701,10 +1707,10 @@
"name": "updateMicroTaskStatus"
},
{
"name": "updateViewsToRefresh"
"name": "urlParsingNode"
},
{
"name": "urlParsingNode"
"name": "viewAttachedToChangeDetector"
},
{
"name": "walkProviderTree"

15
packages/core/test/bundling/hello_world/bundle.golden_symbols.json
View file

@ -104,6 +104,9 @@
{
"name": "GenericBrowserDomAdapter"
},
{
"name": "HAS_CHILD_VIEWS_TO_REFRESH"
},
{
"name": "HAS_TRANSPLANTED_VIEWS"
},
@ -440,9 +443,6 @@
{
"name": "cleanUpView"
},
{
"name": "clearViewRefreshFlag"
},
{
"name": "collectNativeNodes"
},
@ -528,7 +528,7 @@
"name": "detectChangesInEmbeddedViews"
},
{
"name": "detectChangesInView"
"name": "detectChangesInViewIfAttached"
},
{
"name": "detectChangesInternal"
@ -806,6 +806,9 @@
{
"name": "map"
},
{
"name": "markAncestorsForTraversal"
},
{
"name": "markViewDirty"
},
@ -984,10 +987,10 @@
"name": "updateMicroTaskStatus"
},
{
"name": "updateViewsToRefresh"
"name": "urlParsingNode"
},
{
"name": "urlParsingNode"
"name": "viewAttachedToChangeDetector"
},
{
"name": "walkProviderTree"

15
packages/core/test/bundling/hydration/bundle.golden_symbols.json
View file

@ -185,6 +185,9 @@
{
"name": "GenericBrowserDomAdapter"
},
{
"name": "HAS_CHILD_VIEWS_TO_REFRESH"
},
{
"name": "HAS_TRANSPLANTED_VIEWS"
},
@ -635,9 +638,6 @@
{
"name": "clearElementContents"
},
{
"name": "clearViewRefreshFlag"
},
{
"name": "collectNativeNodes"
},
@ -726,7 +726,7 @@
"name": "detectChangesInEmbeddedViews"
},
{
"name": "detectChangesInView"
"name": "detectChangesInViewIfAttached"
},
{
"name": "detectChangesInternal"
@ -1094,6 +1094,9 @@
{
"name": "map"
},
{
"name": "markAncestorsForTraversal"
},
{
"name": "markViewDirty"
},
@ -1320,10 +1323,10 @@
"name": "updateMicroTaskStatus"
},
{
"name": "updateViewsToRefresh"
"name": "urlParsingNode"
},
{
"name": "urlParsingNode"
"name": "viewAttachedToChangeDetector"
},
{
"name": "walkProviderTree"

18
packages/core/test/bundling/router/bundle.golden_symbols.json
View file

@ -272,6 +272,9 @@
{
"name": "GuardsCheckStart"
},
{
"name": "HAS_CHILD_VIEWS_TO_REFRESH"
},
{
"name": "HAS_TRANSPLANTED_VIEWS"
},
@ -980,9 +983,6 @@
{
"name": "cleanUpView"
},
{
"name": "clearViewRefreshFlag"
},
{
"name": "collectNativeNodes"
},
@ -1167,7 +1167,7 @@
"name": "detectChangesInEmbeddedViews"
},
{
"name": "detectChangesInView"
"name": "detectChangesInViewIfAttached"
},
{
"name": "detectChangesInternal"
@ -1700,6 +1700,9 @@
{
"name": "map"
},
{
"name": "markAncestorsForTraversal"
},
{
"name": "markAsComponentHost"
},
@ -2066,6 +2069,9 @@
{
"name": "unwrapSafeValue"
},
{
"name": "updateAncestorTraversalFlagsOnAttach"
},
{
"name": "updateMicroTaskStatus"
},
@ -2076,10 +2082,10 @@
"name": "updateSegmentGroupChildren"
},
{
"name": "updateViewsToRefresh"
"name": "urlParsingNode"
},
{
"name": "urlParsingNode"
"name": "viewAttachedToChangeDetector"
},
{
"name": "walkProviderTree"

15
packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json
View file

@ -146,6 +146,9 @@
{
"name": "GenericBrowserDomAdapter"
},
{
"name": "HAS_CHILD_VIEWS_TO_REFRESH"
},
{
"name": "HAS_TRANSPLANTED_VIEWS"
},
@ -515,9 +518,6 @@
{
"name": "cleanUpView"
},
{
"name": "clearViewRefreshFlag"
},
{
"name": "collectNativeNodes"
},
@ -597,7 +597,7 @@
"name": "detectChangesInEmbeddedViews"
},
{
"name": "detectChangesInView"
"name": "detectChangesInViewIfAttached"
},
{
"name": "detectChangesInternal"
@ -899,6 +899,9 @@
{
"name": "map"
},
{
"name": "markAncestorsForTraversal"
},
{
"name": "markViewDirty"
},
@ -1089,10 +1092,10 @@
"name": "updateMicroTaskStatus"
},
{
"name": "updateViewsToRefresh"
"name": "urlParsingNode"
},
{
"name": "urlParsingNode"
"name": "viewAttachedToChangeDetector"
},
{
"name": "walkProviderTree"

18
packages/core/test/bundling/todo/bundle.golden_symbols.json
View file

@ -167,6 +167,9 @@
{
"name": "GenericBrowserDomAdapter"
},
{
"name": "HAS_CHILD_VIEWS_TO_REFRESH"
},
{
"name": "HAS_TRANSPLANTED_VIEWS"
},
@ -683,9 +686,6 @@
{
"name": "cleanUpView"
},
{
"name": "clearViewRefreshFlag"
},
{
"name": "collectNativeNodes"
},
@ -798,7 +798,7 @@
"name": "detectChangesInEmbeddedViews"
},
{
"name": "detectChangesInView"
"name": "detectChangesInViewIfAttached"
},
{
"name": "detectChangesInternal"
@ -1232,6 +1232,9 @@
{
"name": "map"
},
{
"name": "markAncestorsForTraversal"
},
{
"name": "markAsComponentHost"
},
@ -1484,14 +1487,17 @@
{
"name": "unwrapRNode"
},
{
"name": "updateAncestorTraversalFlagsOnAttach"
},
{
"name": "updateMicroTaskStatus"
},
{
"name": "updateViewsToRefresh"
"name": "urlParsingNode"
},
{
"name": "urlParsingNode"
"name": "viewAttachedToChangeDetector"
},
{
"name": "walkProviderTree"