Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected supression (or undetected) ExpressionChangedAfterItHasBeenCheckedError #44277

Open
sebastian-zarzycki-apzumi opened this issue Nov 25, 2021 · 5 comments
Labels
area: core Issues related to the framework runtime core: change detection P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Milestone

Comments

@sebastian-zarzycki-apzumi
Copy link

Which @angular/* package(s) are the source of the bug?

core

Is this a regression?

No

Description

During the development, I've bumped into some change detection case that is somewhat non-trivial and I felt like maybe Angular doesn't do enough to properly flag it. I'm not sure if this a bug or not (perhaps not), maybe it should be just an improvement. That said, I'd still like to understand what's going on, because I've wasted a lot of time looking in a completely wrong place.

In the example provided in the link, when you click the button, a new component appears. The (fabricated for this example) purpose was for it to update the two props above (by pushing a new value into an observable that resides within a shared service between child and parent and parent subscribes to it) - but it doesn't happen. When you click it again, you finally see it (next change detection triggered, obviously).

After a long and arduous process, now I do know what's wrong here and how to fix it with regards to change detection cycle, child updating parent's DOM in the same change detection macrotask, etc. What I'm not sure about is: why the error isn't thrown, when it's clearly a case of a new value being pushed in the same tick after the parent DOM update (which is clearly demonstrated by the values not updating the first time)?

You can easily influence this by pushing two objects in HelloComponent or by pushing just an empty array into observable in parent, instead of initial object. Then, you would (correctly) see the standard ExpressionChangedAfterItHasBeenCheckedError error, as expected.

It's still an overwrite with a new reference to a new array, so how the comparison works here behind the scenes? Why the expression (*ngFor in this case, in parent) is apparently evaluated to "the same value" and thus not throwing an error, even if it obviously contains a new array with another object filled with props with another values? Is it because *ngFor interpolates to a string at this stage (or uses some other simplification, like only checking array length, comparing by shallow value etc.)?

Seems like the error should be thrown here. It might be "working as intended", but that "intended" is confusing without a warning.

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/angular-jkshfv?file=src%2Fapp%2Fhello.component.ts

Please provide the exception or error you saw

It's one of these unusual cases, where I would actually like to see an error, but I don't see it :)

Please provide the environment you discovered this bug in (run ng version)

Angular CLI: 12.1.0
Node: 14.17.4
Package Manager: npm 8.1.3
OS: darwin x64

Angular: 12.1.0
... animations, cdk, cli, common, compiler, compiler-cli, core
... forms, language-service, platform-browser
... platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1201.0
@angular-devkit/build-angular   12.1.0
@angular-devkit/core            12.1.0
@angular-devkit/schematics      12.1.0
@schematics/angular             12.1.0
rxjs                            6.6.3
typescript                      4.3.4

Anything else?

I do realize that this borderlines on a support request, but, again, the intention here is to improve the framework to throw some kind of warning for these cases, as opposed to doing nothing, as it is right now.

@pkozlowski-opensource pkozlowski-opensource added area: core Issues related to the framework runtime core: change detection labels Nov 25, 2021
@ngbot ngbot bot added this to the needsTriage milestone Nov 25, 2021
@JoostK
Copy link
Member

JoostK commented Nov 26, 2021

This is an interesting case. The devModeEqual check accepts any object as equal:

export function devModeEqual(a: any, b: any): boolean {
const isListLikeIterableA = isListLikeIterable(a);
const isListLikeIterableB = isListLikeIterable(b);
if (isListLikeIterableA && isListLikeIterableB) {
return areIterablesEqual(a, b, devModeEqual);
} else {
const isAObject = a && (typeof a === 'object' || typeof a === 'function');
const isBObject = b && (typeof b === 'object' || typeof b === 'function');
if (!isListLikeIterableA && isAObject && !isListLikeIterableB && isBObject) {
return true;
} else {
return Object.is(a, b);
}
}
}

I presume that this is done in the assumption that using any of its primitive properties in some (child) template would then report an ExpressionChangedAfterItHasBeenCheckedError, so the changed object itself is accepted as is. However, the object's properties are only rendered in embedded views using the original embedded view context, so the changes to the object will not be visible until the next change detection cycle, as only then will a new embedded view be created/updated.

@sebastian-zarzycki-apzumi
Copy link
Author

Oh. I was under the impression that ngFor uses DefaultIterableDiffer?

@JoostK
Copy link
Member

JoostK commented Nov 26, 2021

It does, but that's unrelated to checkNoChanges. Angular's core binding mechanism uses Object.is to determine equality; if something changes then it will rebind the input and trigger ngOnChanges. For NgForOf this means that changing the array will rebind the collection input, which NgForOf then analyzes for differences using IterableDiffer. Any changes to the array's items (depending on trackBy) will trigger creation/deletion/updating of embedded views. This all happens in the primary change detection run.

After the primary change detection run has finished will Angular run its checkNoChanges phase. This will re-evaluate binding expressions using the devModeEqual comparator, in addition to Object.is. During this phase, the changes are never actually written to the corresponding directive inputs, hence NgForOf and its IterableDiffer will never see the changed array during this phase. It's only when a subsequent primary change detection run is triggered that NgForOf is given the new array and its items.

@sebastian-zarzycki-apzumi
Copy link
Author

sebastian-zarzycki-apzumi commented Dec 9, 2021

So what are the further steps from here? Not that I'm impatient, just curious, when the eventual triage might happen?

@alxhub alxhub added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label Nov 16, 2022
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Nov 16, 2022
@JeanMeche
Copy link
Member

JeanMeche commented Mar 29, 2024

Fwiw, the new @for block triggers NG0100 in the case described here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Issues related to the framework runtime core: change detection P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

No branches or pull requests

5 participants