Child view not updated when observable emits new value (onPush)

[bazter]

🐞 bug report

I might be just overlooking something, but I consider myself an advanced ng developer and can't thing of any reason why this should not work. I expect a "you're doing this wrong" answer but I will happily accept it as I could not google a satisfying answer anywhere. Till that I consider this a bug.

Description

Under specific circumstances component view doesn't reflect latest state of an observable value being passed into child component using async pipe operator. AFAIK when using onPush change detection strategy, any observable used in template should trigger the change detection, which is not happening for me.

My update cycle goes like this: Parent component initializes an observable -> observable is passed into child component's input using async pipe -> in child component's OnInit method an output event is instantly emitted -> this output event is caught in parent component -> parent component emits new observable value -> updated value is not reflected in child component (child's input is not updated at all)

🔬 Minimal Reproduction

https://stackblitz.com/edit/angular-ivy-89exth

I expect this code to print number 2.

🌍 Your Environment

Angular Version:

Angular CLI: 9.1.1
Node: 10.15.3
OS: darwin x64

Angular: 9.1.2
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... platform-server, router
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.901.1
@angular-devkit/build-angular     0.901.1
@angular-devkit/build-optimizer   0.901.1
@angular-devkit/build-webpack     0.901.1
@angular-devkit/core              9.1.1
@angular-devkit/schematics        9.1.1
@angular/cdk                      9.2.2
@angular/cli                      9.1.1
@angular/localize                 9.1.4
@angular/material                 9.2.2
@ngtools/webpack                  9.1.1
@nguniversal/builders             9.1.0
@nguniversal/common               9.1.0
@nguniversal/express-engine       9.1.0
@schematics/angular               9.1.1
@schematics/update                0.901.1
rxjs                              6.5.5
typescript                        3.8.3
webpack                           4.42.0

Although latest version on stackblitz behaves the same.

[pshurygin]

The problem here is that you emit a new value inside the OnInit hook. By the time this hook is executed the parent component has already been checked for changes, so your changed value will not be rendered until the next change detection cycle. And this next CD cycle never occurs in your example because nothing is triggering it. Async pipe does not actually trigger change detection, it only marks it's component to be checked next time change detection will be run. Emitting value via subject also does not trigger change detection on it's own.

If you add detectChanges() call into yout inc() function in AppComponent your example will start to work as expected. Also there wont be a problem if you emit your new value inside some event handler instead of OnInit hook, because then this event would trigger change detection via zone.js.

This is just my understanding of the issue so please correct me if i'm wrong here.

[pkozlowski-opensource]

@pshurygin explanations are spot on here - it "works as designed" today but I can see how it can get confusing. As such I would qualify this issue as "confirmed confession".

In short, the issue here is that a value gets emitted after change detection run on the component "interested" in the value.

It is easier to see what is going on when one simplifies repro a bit and removes OnPush from the AppComponent: https://stackblitz.com/edit/angular-ivy-p1cgca?file=src%2Fapp%2Fapp.component.ts

When we do so there is a very clear error message indicating the described situation:

preview-6f2d8d3d3fde463d30fbb.js:1 ERROR Error: ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: 'undefined'. Current value: '2'. It seems like the view has been created after its parent and its children have been dirty checked. Has it been created in a change detection hook?
    at throwErrorIfNoChangesMode (core.umd.js? [sm]:5776)
    at bindingUpdated (core.umd.js? [sm]:13730)
    at Object.ɵɵproperty (core.umd.js? [sm]:14624)
    at AppComponent_Template (app.component.ts? [sm]:28)
   ...

Now, this error is not reported on the OnPush component and this is the root cause of the confusion here. If I recall properly, we've got another bug report that tracks the situation where OnPush components don't properly execute "check no changes" verifications. I will try to find this issue and link it here.

[bazter]

Thank you @pshurygin and @pkozlowski-opensource for explanation. I must have been very lucky in the past, when I relied on the fact that simply emitting an observable through the template would be enough to trigger the change detection.. and it worked every time probably because of other events flying around.

Now, because I'd like to get better and learn a proper way of doing things, what would you recommend here? After all those years I somehow consider calling cdr.detectChanges a cheat which can be always replaced by better app design. Am I wrong here or is there a better way?

[pshurygin]

This highly depends on your exact use case. You should think if you really need this emit inside OnInit hook of the child component. Maybe you can just call inc() inside parent component's OnInit instead. Or maybe you can emit new value in an event handler or after promise being resolved.

If not, than you either have to use detectChanges or simply do not rely on the immediate binding update in this case. As it is this child component which triggered the state change, than it is already aware of the change anyway, so it can handle it at the same time as emitting new value, without relying on the binding update(for ex. using local component state).

[atscott]

Closing as working as intended (see https://github.com/angular/angular/issues/37137#issuecomment-630217516). As Pawel mentioned, we are already tracking the issue with OnPush components not throwing the error correctly. We are also aware of the general confusion with the error even when it is thrown and are actively working to improve our error messages across the board.

[angular-automatic-lock-bot[bot]]

This issue has been automatically locked due to inactivity. Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}