Repository navigation
Calling ChangeDetectorRef.detectChanges in child view causes error in parent NgForOf directive #45577
Description
Which @angular/* package(s) are the source of the bug?
core, forms
Is this a regression?
No
Description
TL;DR: When a child view calls ChangeDetectorRef.detectChanges at an appropriate moment in its creation lifecycle, it can cause ngForOf to throw an error when deleting that child in response to input binding changes. The nature of the connection between these two events is mysterious to me.
Longer explanation:
I have an ngForOf directive that operates over a list of FormControl elements, and produces a <list-item> view for every one. ListItem implements the ControlValueAccessor interface, and every ListItem is hooked up to its associated FormControl element. I have button that, when clicked, deletes one of the FormControl elements, and thus causes ngForOf to update, removing that control's associated view. It looks like this:
@Component({
selector: 'my-app',
template: `
<ul>
<list-item
*ngFor="let control of controls"
[formControl]="control"
(registerOnChangeCalled)="changeDetectorRef.detectChanges()"
></list-item>
</ul>
<button (click)="controls.splice(0, 1)">remove first list item</button>`,
})
export class AppComponent {
controls = [new FormControl(), new FormControl()];
constructor(private readonly changeDetectorRef: ChangeDetectorRef) {}
}
Note that a registerOnChangeCalled event fired from a <list-Item> causes changeDetectorRef.detectChanges() to be called.
My ListItem component is very simple:
@Component({
selector: 'list-item',
template: `<li>I am list item</li>`,
providers: [
{
provide: NG_VALUE_ACCESSOR,
useExisting: forwardRef(() => ListItem),
multi: true,
},
],
})
export class ListItem implements ControlValueAccessor {
constructor(private readonly changeDetectorRef: ChangeDetectorRef) {}
@Output() registerOnChangeCalled = new EventEmitter();
registerOnChange(updateModelValue) {
this.registerOnChangeCalled.emit(null);
}
writeValue(modelValue) { /* unimplemented */ }
registerOnTouched() { /* unimplemented */ }
}
In this minimal repro example, registerOnChange only emits an event to registerOnChangeCalled, although in a real app it would do more. registerOnChange is a ControlValueAccessor method which is called once during the FormControl directive initialization, after ListItem's ngOnInit and before its ngAfterContentInit.
With this setup, clicking the AppComponent button causes the following error to be thrown:
ERROR TypeError: Cannot read properties of null (reading 'destroyed')
at ViewContainerRef_.move (refs.ts:218:17)
at eval (ng_for_of.ts:244:33)
at DefaultIterableDiffer.forEachOperation (default_iterable_differ.ts:112:9)
at NgForOf._applyChanges (ng_for_of.ts:227:13)
at NgForOf.ngDoCheck (ng_for_of.ts:221:25)
at checkAndUpdateDirectiveInline (provider.ts:220:15)
at checkAndUpdateNodeInline (view.ts:442:14)
at checkAndUpdateNode (view.ts:402:12)
at debugCheckAndUpdateNode (services.ts:435:44)
at debugCheckDirectivesFn (services.ts:396:7)
If I do not call changeDetectorRef.detectChanges(), the error will not be thrown. If I call changeDetectorRef.detectChanges() in ListItem.ngOnInit or ngAfterContentInit, the error will not be thrown.
This is effectively a repro of #45417, which was closed 12 days ago due to a failure to repro.
Please provide a link to a minimal reproduction of the bug
https://stackblitz.com/edit/angular-yjdjfe?file=src/app/app.component.ts
Please provide the exception or error you saw
ERROR TypeError: Cannot read properties of null (reading 'destroyed')
at ViewContainerRef_.move (refs.ts:218:17)
at eval (ng_for_of.ts:244:33)
at DefaultIterableDiffer.forEachOperation (default_iterable_differ.ts:112:9)
at NgForOf._applyChanges (ng_for_of.ts:227:13)
at NgForOf.ngDoCheck (ng_for_of.ts:221:25)
at checkAndUpdateDirectiveInline (provider.ts:220:15)
at checkAndUpdateNodeInline (view.ts:442:14)
at checkAndUpdateNode (view.ts:402:12)
at debugCheckAndUpdateNode (services.ts:435:44)
at debugCheckDirectivesFn (services.ts:396:7)
Please provide the environment you discovered this bug in (run ng version)
"dependencies": {
"@angular/animations": "^12.1.0",
"@angular/common": "^12.1.0",
"@angular/compiler": "^12.1.0",
"@angular/core": "^12.1.0",
"@angular/forms": "^12.1.0",
"@angular/platform-browser": "^12.1.0",
"@angular/platform-browser-dynamic": "^12.1.0",
"@angular/router": "^12.1.0",
"core-js": "^3.15.1",
"rxjs": "^7.1.0",
"tslib": "^2.3.0",
"zone.js": "^0.11.4"
},
"devDependencies": {
"@angular-devkit/build-angular": "~0.1100.4",
"@angular/cli": "~11.0.4",
"@angular/compiler-cli": "~11.0.4",
"@types/jasmine": "~3.6.0",
"@types/node": "^12.11.1",
"codelyzer": "^6.0.0",
"ts-node": "~8.3.0",
"tslint": "~6.1.0",
"typescript": "~4.0.2"
}
Anything else?
No response
Activity
JoostK commentedon Apr 9, 2022
The
registerOnChangeCalledevent is invoked during a change detection cycle, then triggering another nested change detection cycle. This results in a re-entrant invocation ofNgForOf.ngDoCheck:(the blue stack frame is the same method as the top stack frame)
This is a problem, as
IterableDiffer.diffin the re-entrant call will clobber the internal state, causing the outer invocation that is still processing changes usingIterableChanges.forEachOperation—whereIterableChangesis really just theIterableDifferinstance which has since been clobbered—to get into an inconsistent state.It is generally an anti-pattern to emit events to parents during a change detection cycle, as it goes against unidirectional dataflow. The framework should arguably not crash in the way it does today, but I'm not sure what the behaviour here should be.
If you change
registerOnChangeCalledto use an asynchronousEventEmitter(by passingtrueto its constructor) then the issue does not occur, as there won't be a re-entrant change detection cycle.anthonyrgreen commentedon Apr 12, 2022
Thank you very much for the prompt response.
The current behavior is quite surprising, no? I don't know of any documentation that could have led me to expect this. I know that emitting events during a change-detection cycle is not encouraged but the typical reasons for that -- as I understand them -- have to do with preventing infinite loops where component A modifies component B which modifies component A and so on. In this case, I think there are quite good reasons why I need non-uni-directional data flow (they have to do with particularities of CVAs and their validation functions -- can explain more if you are interested): shouldn't that be possible without crashing *ngFor?
At the very least, I think *ngFor should be throwing a more informative error -- perhaps keeping track of when it is reentered, and throwing an error on the basis of that? The error I got is not very helpful in diagnosing the problem unless you have pretty deep framework knowledge.
otodockal commentedon Apr 12, 2022
@anthonyrgreen don't get me wrong, but if it's only about forms, I would write this way https://stackblitz.com/edit/angular-v8g6kp?file=src%2Fapp%2Fapp.component.ts
If it's a general question, looks like this, I would stick with unidirectional flow, not a very helpful answer, I know. :-)
anthonyrgreen commentedon Apr 12, 2022
@otodockal :) well, yes, if you remove the change detection cycle, it fixes the issue. But I need the change detection cycle! That's how I ran into this bug! My toy reproduction doesn't make the necessity for change detection obvious, but that's because I'm trying to make the repro as simple as possible.
The actual fix I'm using in my app is to put the change detection cycle inside ngAfterViewInit. This makes the bug go away and accomplishes my other aims!
arobinson commentedon May 23, 2022
Not sure if this is the same issue, but I am seeing the same crash. I still need to debug more to try to find the cause. But adding here in case this helps:
pkozlowski-opensource commentedon Jun 7, 2022
@anthonyrgreen I would be very much interested in understanding more. Especially that you've mentioned "typical reasons" and "preventing infinite loops where component A modifies component B which modifies component A and so on.".
Could we try to distill this use-case in a stackblitz to understand if Angular fails to cover a "typical" use-case?
arturovt commentedon Sep 29, 2026
@JeanMeche this can be closed. registerOnChange runs during the form directive's setup, so calling detectChanges() there re-enters change detection while the parent's ngForOf differ is mid-operation, which is how the differ ends up with a stale view record. Reentrant change detection from inside another view's update is not supported, which is why the same call is fine from ngOnInit.
Guarding against it would mean detecting reentrancy in the core change detection loop, which every application runs, for a P4 with no upvotes and no duplicates since 2022.
registerOnChange is not a place for view work: the accessor should store the callback and nothing more.