Skip to content

fix(core): update animation scheduling #64441

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
thePunderWoman wants to merge 1 commit into from
Closed

fix(core): update animation scheduling #64441

thePunderWoman wants to merge 1 commit into from

Conversation

@thePunderWoman
Copy link
Contributor

@thePunderWoman thePunderWoman commented Oct 15, 2025 •
edited
Loading

In some rare cases, the injection context that houses the afterEveryRender for the animation queue gets destroyed. This results in any subsequent animations getting queued, but never run. To prevent this from happening, this PR updates the queuing mechanism to use afterNextRender and to trigger the scheduling at the time of adding animations to that queue. This has a nice side effect of optimizing the animation queue to only run animations when needed.

fixes: #64423

@thePunderWoman thePunderWoman added 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 Oct 15, 2025
@ngbot ngbot bot modified the milestone: Backlog Oct 15, 2025
@thePunderWoman thePunderWoman force-pushed the animation-queue branch 3 times, most recently from adab1f1 to 95f5c8f Compare October 15, 2025 16:55
@thePunderWoman
Copy link
Contributor Author

thePunderWoman commented Oct 15, 2025

Caretaker note: the failing G3 tests are unrelated to this change.

JoostK
JoostK reviewed Oct 15, 2025
View reviewed changes
Copy link
Member

@JoostK JoostK left a comment

Choose a reason for hiding this comment

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

It should be possible to write a test for this, right?

@thePunderWoman
Copy link
Contributor Author

thePunderWoman commented Oct 15, 2025

@JoostK I'm honestly not sure how this would be handled in a test.

@JoostK
Copy link
Member

JoostK commented Oct 15, 2025

@JoostK I'm honestly not sure how this would be handled in a test.

I don't know the exact nature of #64423, but replicating that in a test and verifying that things get destroyed as expected sounds browser/animating timing independent so is hopefully reliably testable. Given the subtleties of the timing and this being a regression makes it even more a candidate to have a test for.

@thePunderWoman
Copy link
Contributor Author

thePunderWoman commented Oct 15, 2025

@JoostK I'm happy to add it. I'm just not sure what it would look like. That issue does not have a very minimal reproduction, which is quite challenging to pare down.

@thePunderWoman thePunderWoman force-pushed the animation-queue branch 3 times, most recently from 5ead563 to 622aa92 Compare October 15, 2025 22:31
AndrewKushnir
AndrewKushnir approved these changes Oct 15, 2025
View reviewed changes
@AndrewKushnir AndrewKushnir removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Oct 15, 2025
@thePunderWoman
fix(core): update animation scheduling
308e5db 
In some rare cases, it seems the animation queue disappears despite being afterEveryRender. This updates the animation scheduler to be afterNextRender instead and only schedules it when we need to.

fixes: angular#64423
@thePunderWoman thePunderWoman added the action: merge The PR is ready for merge by the caretaker label Oct 16, 2025
@thePunderWoman
Copy link
Contributor Author

thePunderWoman commented Oct 16, 2025

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

thePunderWoman added a commit that referenced this pull request Oct 16, 2025
@thePunderWoman
fix(core): update animation scheduling (#64441)
911d682 
In some rare cases, it seems the animation queue disappears despite being afterEveryRender. This updates the animation scheduler to be afterNextRender instead and only schedules it when we need to.

fixes: #64423

PR Close #64441
@angular-automatic-lock-bot
Copy link

angular-automatic-lock-bot bot commented Nov 16, 2025

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

Reviewers

@JoostK JoostK JoostK left review comments

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

Angular 20.3.4 using createComponent on ng-container creates not destroying itself

3 participants

@thePunderWoman @JoostK @AndrewKushnir