Skip to content

fix(ChangeDetection): ngAfterViewChecked is called when ChangeDetecto…#9317

Closed
alfonso-presa wants to merge 1 commit into
angular:masterfrom
alfonso-presa:fixLifecycleEventsWhenDetached
Closed

fix(ChangeDetection): ngAfterViewChecked is called when ChangeDetecto…#9317
alfonso-presa wants to merge 1 commit into
angular:masterfrom
alfonso-presa:fixLifecycleEventsWhenDetached

Conversation

@alfonso-presa

@alfonso-presa alfonso-presa commented Jun 17, 2016

Copy link
Copy Markdown
Contributor

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x")

[X] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

ngDoCheck, ngAfterViewChecked, ngAfterContentChecked and ngOnChanges are called when ChangeDetector is detached

What is the new behavior?

ngDoCheck, ngAfterViewChecked, ngAfterContentChecked and ngOnChanges are NOT called when ChangeDetector is detached

Does this PR introduce a breaking change? (check one with "x")

[ ] Yes
[X] No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

ngDoCheck, ngAfterViewChecked, ngAfterContentChecked and ngOnChanges should not be invoked when ChangeDetector is detached.

fixes #7054

@vicb vicb added comp: core/change_detection action: review The PR is still awaiting reviews from at least one requested reviewer labels Jun 18, 2016
@vicb vicb added this to the 2.0.0-rc.3 milestone Jun 18, 2016
@timruffles

timruffles commented Jun 19, 2016

Copy link
Copy Markdown
Contributor

oh damn, I saw your previous PR was closed so spent a while making #9351 Still, I learned a lot doing it :D

Will close mine.

@alfonso-presa alfonso-presa force-pushed the fixLifecycleEventsWhenDetached branch from 1fabcc0 to 3f763a2 Compare June 20, 2016 15:57
…r is detached

ngDoCheck, ngAfterViewChecked, ngAfterContentChecked and ngOnChanges should not be invoked when ChangeDetector is detached.

fixes angular#7054
@alfonso-presa alfonso-presa force-pushed the fixLifecycleEventsWhenDetached branch from 3f763a2 to 0a9233d Compare June 20, 2016 16:02
@alfonso-presa

Copy link
Copy Markdown
Contributor Author

@timruffles I think it's my fault... because I referenced another issue in the commits fix statement. Sorry for that.

I also realised thanks to your code that it was better to call the method ifChangeDetectorIsAttached rather than ifChangeDetectionModeIsNotDetached ^_^.

@tbosch

tbosch commented Jun 20, 2016

Copy link
Copy Markdown
Contributor

I don't think this is correct. The change detector of a component refers to its template. E.g. setting the change detector to OnPush also only affects the template, but not the lifecycle hooks.

@alfonso-presa

Copy link
Copy Markdown
Contributor Author

@tbosch do you mean that the lifecycle hooks should be invoked independently of the change detector's state and configuration? That conflicts with the expected behavior explained in #7054. Should it be closed?.... or am I facing the problem the wrong way?

@tbosch

tbosch commented Jun 21, 2016

Copy link
Copy Markdown
Contributor

Just closed #7054. Closing this PR for now as well.

@tbosch tbosch closed this Jun 21, 2016
@angular-automatic-lock-bot

Copy link
Copy Markdown

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 Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: review The PR is still awaiting reviews from at least one requested reviewer cla: yes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(ChangeDetection): ngAfterViewChecked is called when ChangeDetector is detached

5 participants