Skip to content

fix(core): prevent animations renderer from impacting animate.leave#63921

Closed
thePunderWoman wants to merge 1 commit intoangular:mainfrom
thePunderWoman:bypass-animations-package
Closed

fix(core): prevent animations renderer from impacting animate.leave#63921
thePunderWoman wants to merge 1 commit intoangular:mainfrom
thePunderWoman:bypass-animations-package

Conversation

@thePunderWoman
Copy link
Copy Markdown
Contributor

@thePunderWoman thePunderWoman commented Sep 18, 2025

This bypasses the animation renderer when removing nodes that have animate.leave animations. This is accomplished by exposing the root renderer via a new Renderer2 function called getBaseRenderer(). For most renderers, this will just be the current renderer, but for the animations renderer, it will always get the root renderer.

fixes: #63893

@thePunderWoman thePunderWoman added area: animations legacy animations package only. Otherwise use area: core. action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release labels Sep 18, 2025
@ngbot ngbot bot added this to the Backlog milestone Sep 18, 2025
@thePunderWoman thePunderWoman force-pushed the bypass-animations-package branch 2 times, most recently from 65dd7d2 to b049593 Compare September 18, 2025 16:52
@pullapprove pullapprove bot requested review from crisbeto and kirjs September 18, 2025 16:52
@thePunderWoman thePunderWoman force-pushed the bypass-animations-package branch 6 times, most recently from f007b83 to a6163eb Compare September 18, 2025 19:08
@thePunderWoman thePunderWoman force-pushed the bypass-animations-package branch 4 times, most recently from 3fbabd1 to 8f2161a Compare September 18, 2025 20:30
@thePunderWoman thePunderWoman force-pushed the bypass-animations-package branch 4 times, most recently from 4fa16a8 to 86c2b7d Compare September 18, 2025 23:37
Copy link
Copy Markdown
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Can we also put together a test to reproduce the original issue to avoid regressions in the future?

@thePunderWoman thePunderWoman force-pushed the bypass-animations-package branch from 86c2b7d to 99812cb Compare September 19, 2025 20:10
@thePunderWoman thePunderWoman removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Sep 19, 2025
@thePunderWoman thePunderWoman added the action: merge The PR is ready for merge by the caretaker label Sep 19, 2025
@pullapprove pullapprove bot requested review from crisbeto and kirjs September 19, 2025 20:10
Copy link
Copy Markdown
Member

@JeanMeche JeanMeche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed-for: public-api

This adds an optional flag to the renderer on `removeChild` called `requireSynchronousElementRemoval`, which can tell any downstream renderer that elements need to be removed synchronously. This gets passed down to the legacy animation renderer to ensure that any elements that set this flag aren't impacted by that renderers changes to timing.

fixes: angular#63893
@thePunderWoman thePunderWoman force-pushed the bypass-animations-package branch from 99812cb to b3d1286 Compare September 19, 2025 20:39
thePunderWoman added a commit that referenced this pull request Sep 19, 2025
…#63921)

This adds an optional flag to the renderer on `removeChild` called `requireSynchronousElementRemoval`, which can tell any downstream renderer that elements need to be removed synchronously. This gets passed down to the legacy animation renderer to ensure that any elements that set this flag aren't impacted by that renderers changes to timing.

fixes: #63893

PR Close #63921
@thePunderWoman
Copy link
Copy Markdown
Contributor Author

This PR was merged into the repository. The changes were merged into the following branches:

@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 Oct 20, 2025
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 area: animations legacy animations package only. Otherwise use area: core. area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New Animations API timing is affected when the animations package is also present

3 participants