Skip to content

Commit

Permalink
fixup! fix(core): Ensure backwards-referenced transplanted views are …
Browse files Browse the repository at this point in the history
…refreshed
  • Loading branch information
atscott committed Sep 12, 2023
1 parent 8095e39 commit df1f8cc
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 16 deletions.
35 changes: 20 additions & 15 deletions packages/core/src/render3/instructions/change_detection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,16 @@ export function detectChangesInternal<T>(
// descendants views that need to be refreshed due to re-dirtying during the change detection
// run, detect changes on the view again. We run change detection in `Targeted` mode to only
// refresh views with the `RefreshView` flag.
while ((lView[FLAGS] & LViewFlags.RefreshView || lView[HAS_CHILD_VIEWS_TO_REFRESH]) &&
retries < 100) {
while (lView[FLAGS] & LViewFlags.RefreshView || lView[HAS_CHILD_VIEWS_TO_REFRESH]) {
if (retries === 100) {
throw new RuntimeError(
RuntimeErrorCode.INFINITE_CHANGE_DETECTION, ngDevMode && 'Infinite change detection.');
}
retries++;
// Even if this view is detached, we still detect changes in targeted mode because this was
// the root of the change detection run.
detectChangesInView(lView, ChangeDetectionMode.Targeted);
}
if (retries === 100) {
throw new RuntimeError(
RuntimeErrorCode.INFINITE_CHANGE_DETECTION, ngDevMode && 'Infinite change detection.');
}
} catch (error) {
if (notifyErrorHandler) {
handleError(lView, error);
Expand Down Expand Up @@ -263,7 +264,7 @@ function detectChangesInEmbeddedViews(lView: LView, mode: ChangeDetectionMode) {
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);
}
}
}
Expand Down Expand Up @@ -299,27 +300,31 @@ 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)) {
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 HAS_CHILD_VIEWS_TO_REFRESH flag is set.
*/
function detectChangesInView(lView: LView, mode: ChangeDetectionMode) {
if (!viewAttachedToChangeDetector(lView)) {
return;
}

const tView = lView[TVIEW];
const flags = lView[FLAGS];
if ((flags & (LViewFlags.CheckAlways | LViewFlags.Dirty) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import {CommonModule} from '@angular/common';
import {ChangeDetectionStrategy, ChangeDetectorRef, Component, DoCheck, Input, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/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';

Expand Down Expand Up @@ -832,6 +832,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');
});
});
});

Expand Down

0 comments on commit df1f8cc

Please sign in to comment.