fix(animations): cleanup DOM elements when the root view is removed#41001
fix(animations): cleanup DOM elements when the root view is removed#41001arturovt wants to merge 1 commit intoangular:masterfrom
Conversation
|
This is a replacement of this PR (#39982). @atscott cc. There were some presubmit failures, but you mentioned in that comment that you manually fixed some issues (#39982 (comment)). Could you try to re-run the presubmit? |
|
triggered global tests to see what still needs to be fixed (if any). |
|
Global tests are all green. This should be good to go now 👍 |
|
I see that you just added the If you want your PR to be merged, it has to pass all the CI checks. If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help. |
|
@atscott oh great 👍 Well, I see the Presubmit Failed 😄 |
Currently, when importing `BrowserAnimationsModule`, Angular uses `AnimationRenderer` as the renderer. When the root view is removed, the `AnimationRenderer` defers the actual work to the `TransitionAnimationEngine` to do this, and the `TransitionAnimationEngine` doesn't actually remove the DOM node, but just calls `markElementAsRemoved()`. The actual DOM node is not removed until `TransitionAnimationEngine` "flushes". Unfortunately, though, that "flush" will never happen, since the root view is being destroyed and there will be no more flushes. This commit adds `flush()` call when the root view is being destroyed. BREAKING CHANGE: DOM elements are now correctly removed when the root view is removed. It is possible that tests could be accidentally relying on the old behavior by trying to find an element that was not removed in a previous test. If this is the case, the failing tests should be updated to ensure they have proper setup code which initializes elements they rely on.
There were some unrelated failures. I just need to force it green :) |
…emoved (angular#41001)" This reverts commit a31da48.
|
After a bit more investigation, the failure was legitimate - the SSR code was configured to destroy the platform (internal link to the code) before returning the generated HTML which resulted in the Angular elements being cleaned up as well. I missed this in the global tests because I hadn't configured them to run with Ivy enabled. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
PR Checklist
PR Type