Skip to content

Revert "fix(core): render hooks should not specifically run outside t…#55624

Closed
atscott wants to merge 1 commit intoangular:mainfrom
atscott:revertrenderhooksinsidezone
Closed

Revert "fix(core): render hooks should not specifically run outside t…#55624
atscott wants to merge 1 commit intoangular:mainfrom
atscott:revertrenderhooksinsidezone

Conversation

@atscott
Copy link
Contributor

@atscott atscott commented May 1, 2024

…he Angular zone (#55399)"

This reverts commit 7e89753.

Running render hooks inside the zone is specifically problematic for afterRender hooks. If the callback has async task, it would cause an infinite change detection. In addition, updating state in render hooks is generally discourages and certainly should update state in a way that notifies Angular of the change (either via signal or with markForCheck) rather than relying on ZoneJS to pick it up (which would only work if the change is done inside an async task).

…he Angular zone (angular#55399)"

This reverts commit 7e89753.

Running render hooks inside the zone is specifically problematic for
`afterRender` hooks. If the callback has async task, it would cause an
infinite change detection. In addition, updating state in render hooks
is generally discourages and certainly should update state in a way that
notifies Angular of the change (either via signal or with `markForCheck`) rather
than relying on ZoneJS to pick it up (which would only work if the
change is done inside an async task).
@atscott atscott added the target: patch This PR is targeted for the next patch release label May 1, 2024
@atscott atscott added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels May 1, 2024
@atscott
Copy link
Contributor Author

atscott commented May 2, 2024

merge assistance: g3 failures are unrelated

@AndrewKushnir
Copy link
Contributor

This PR was merged into the repository by commit 8cabb7a.

AndrewKushnir pushed a commit that referenced this pull request May 2, 2024
…he Angular zone (#55399)" (#55624)

This reverts commit 7e89753.

Running render hooks inside the zone is specifically problematic for
`afterRender` hooks. If the callback has async task, it would cause an
infinite change detection. In addition, updating state in render hooks
is generally discourages and certainly should update state in a way that
notifies Angular of the change (either via signal or with `markForCheck`) rather
than relying on ZoneJS to pick it up (which would only work if the
change is done inside an async task).

PR Close #55624
AndrewKushnir pushed a commit that referenced this pull request May 2, 2024
…he Angular zone (#55399)" (#55624)

This reverts commit 7e89753.

Running render hooks inside the zone is specifically problematic for
`afterRender` hooks. If the callback has async task, it would cause an
infinite change detection. In addition, updating state in render hooks
is generally discourages and certainly should update state in a way that
notifies Angular of the change (either via signal or with `markForCheck`) rather
than relying on ZoneJS to pick it up (which would only work if the
change is done inside an async task).

PR Close #55624
@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 Jun 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants