8000 OnPush change detection doesn't work for outputs emitted from lifecycle callbacks · Issue #24965 · angular/angular · GitHub
[go: up one dir, main page]
Skip to content

OnPush change detection doesn't work for outputs emitted from lifecycle callbacks #24965

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.

Sign up for GitHub

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

Jump to bottom
Closed
sebek64 opened this issue Jul 18, 2018 · 12 comments
Closed

OnPush change detection doesn't work for outputs emitted from lifecycle callbacks #24965

sebek64 opened this issue Jul 18, 2018 · 12 comments
Labels
area: core Issues related to the framework runtime core: change detection freq1: low P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: confirmed type: bug/fix
Milestone
Backlog

Comments

@sebek64
Copy link
sebek64 commented Jul 18, 2018

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[X] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:

Current behavior

This issue is related to the closed issue #24690 for which I managed to create a simple repro case. Complete description follows.

When a component with OnPush change detection subscribes an output of a subcomponent, it works pretty well, unless this subcomponent emits values from the initialization lifecycle callbacks (ngOnInit, ngAfterViewInit). Then, no change detection is performed, and even calling changeDetectorRef.markForCheck() doesn't help. Only calling detectChanges() manually works.

Quite surprisingly, when the value is emitted from ngOnInit, it is shown in the interpolation strings, but not passed to subcomponents. On the other hand, ngAfterViewInit value is not used at all.

Expected behavior

It should work out of the box, without the need to work with change detector manually. If this is not supported for some architectural reason, the documentation should contain a big warning about it.

Minimal reproduction of the problem with instructions

I've prepared a git repo here: https://github.com/sebek64/angular-issue-repro

The initial commit is a plain angular-cli project. The next commit adds the repro code. Run via "yarn start", open the browser, and you should see output like this:

outer component value: "emitted value from ngOnInit"
inside subcomponent: ""
Button

When the button is clicked, the output changes to

outer component value: "Emitted value from button"
inside subcomponent: "Emitted value from button"
Button

What is the motivation / use case for changing the behavior?

Environment


Angular version: 6.0.9


Browser:
- [X] Chrome (desktop) version 67.0.3396.99
- [X] Firefox version 52.9.0
- Platform:  Linux
@mlc-mlapis
Copy link
Contributor

@sebek64 ... do you have any problems with creating an online reproduction demo using https://stackblitz.com because it is much easier for everybody to look at it than to clone a repository, run CLI, ... and so on ... ?

@sebek64
Copy link
Author
sebek64 commented Jul 18, 2018

@mlc-mlapis Well, actually no, just laziness :). So I'll prepare it. Initially, a few weeks ago, I started by creating an obvious demo online and failed, because it always worked as expected. So this time, I used more convenient tools for me.

So I try to copy everything into an online version and post a link. I suspect some race condition according to the behavior in the real-world project, so it may well be irreproducible in web-based tool.

@sebek64
Copy link
Author
sebek64 commented Jul 18, 2018

Here is a web-based repro: https://stackblitz.com/edit/angular-gitter-egxquq

@manklu
Copy link
manklu commented Jul 18, 2018

@mlc-mlapis With stackblitz you can load the code direktly from GitHub.

https://stackblitz.com/github/sebek64/angular-issue-repro

@mlc-mlapis
Copy link
Contributor

@manklu ... yep, thanks, I forgot about this feature. 👍

@manklu
Copy link
manklu commented Jul 18, 2018

Yes, there is a problem with events during change detection. As workaround you can use something like this:

Promise.resolve().then(() => this.valueChange.emit("emitted value from ngOnInit"));

Better than calling detectChanges() directly.

@trotyl
Copy link
Contributor
trotyl commented Jul 19, 2018

This should raise an ExpressionChangedAfterItHasBeenCheckedError error, not producing any error message is indeed a BUG.

@sebek64
Copy link
Author
sebek64 commented Jul 19, 2018

So if I understand it correctly, the described scenario is not going to work, because the change detection mechanism is not designed to support it.

Based on the @manklu comment, I found another possible workaround - use "async = true" for EventEmitter, i.e., new EventEmitter<...>(true).

@manklu
Copy link
manklu commented Jul 19, 2018

If most events are emitted during change detection, this is the best solution.

@vicb vicb added the area: core Issues related to the framework runtime label Jul 20, 2018
@ngbot ngbot bot added this to the needsTriage milestone Jul 20, 2018
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Jul 31, 2018
@atscott
Copy link
Contributor
atscott commented Sep 11, 2020

As @trotyl mentioned, this should result in ExpressionChangedAfterItHasBeenCheckedError but doesn't because non-dirty OnPush components do not get checked during the checkNoChanges pass which would throw the error. #34443 would fix this, but is a pretty big breaking change (because of all the new errors that would be thrown in applications) so we don't currently have a plan for landing it.

@jelbourn jelbourn added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent and removed severity3: broken labels Oct 1, 2020
@atscott
Copy link
Contributor
atscott commented Apr 15, 2022

Closing as duplicate of #45612

@atscott atscott closed this as completed Apr 15, 2022
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators May 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area: core Issues related to the framework runtime core: change detection freq1: low P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: confirmed type: bug/fix
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
9 participants
@mhevery @vicb @atscott @jelbourn @pkozlowski-opensource @sebek64 @mlc-mlapis @trotyl @manklu
0