4000 feels like a magic number here. Can we use a named a named constant and/or have a comment on why this value. (Would animations longer than 4s just get removed after ther 4s ?)

Certainly. I've added a const and associated comment about why 4 seconds.

Also to quickly answer your question here, if you're using an animation function (rather than classes) foranimate.leave, you are required to call the complete function to tell the framework when to remove the element. The feedback on the RFC was to have a timeout for safety, and so I went with the same timeout duration as cross document view transitions. So after a 4 second timeout, the element gets removed. If your animation is done before that, no issues.

What if we can have an injection token that configures it? And the default would still be 4s.

We can keep it like this for now, but we should probably output a dev-mode warning for cases when timeout is triggered (for ex. for cases when the "done" callback is never invoked). That can help to surface this situations and we can see if there are cases when the timeout needs to be adjusted.

A couple additional thoughts:

• This can be done in a followup PR
• We should consider cases when those warnings are produced for elements inside of@for loops and avoid spamming console with those warnings

Do we need this symbol in the public API, was it intentionally exposed?

This looks unexpected, should we try making it tree-shakable (maybe via using lightweight injection token in the non-tree-shakable part and providing an actual implementation once the feature is brought it via special instructions)?

I'm not sure how we would be able to do this since there's no provider function for Animations.

We could use thefeatures prop, like we did for standalone in the past (#58288 removed it)

We could use the features prop, like we did for standalone in the past (#58288 removed it)

+1, left a comment here:#62682 (comment)

This function is pretty long, so I was thinking that it might benefit from either having more docs, explaining the overall logic and/or refactor the logic and extract it into multiple functions (I suspect that some of the functions can also be reused across instructions within this file).

Yeah there was a lot of currying happening here because of needed variable values and wanting to be able to clean up the event listeners. I've switched to listeners that only fire once now, which allowed this to be broken up. So, this should hopefully work well. Otherwise I'll have to figure out a good cleanup solution.

Update: We really only need to clean up the listeners foranimate.enter and only two of them. I've found a nice clean way to deal with that. This should be decently organized now.

We have this check in a few places and I was wondering if we are checking for thegetAnimations function presence on a given element or it's a test whether it's generally supported in the current environment? If the latter - we can just do it once somewhere and reuse the result.

This is primarily becausegetAnimations does not exist outside of a browser environment. There's two cases where we check it, one is here and the other is when we set up a cancellation on enter animations. I don't know that it makes sense to use it only once in that case. The cancellation function only is used inanimateEnter cases.

Got it, thanks. My thinking was to have this check somewhere at the module level, e.g. check ifdocument.body.getAnimations is present and just use this info everywhere. The problem that in this case well be bypassingrenderer completely, which may not be great (but likely in practice it'd not be a big problem). Maybe there are some other ways to do it? WDYT?

Another idea is that we can move this check from this utility function into an instruction, so that we just exit early (similar to SSR checks).

This is likely where theElementRegistry symbol is "leaking" (becomes non-tree-shakable).

Yeah. The DOM renderer is the key place this registry is needed to delay removal of elements.

We'd probably need to update a section title above (that says "// Styling Errors").

Should we just have 1 metric that we track (e.g. something likeperformanceMarkFeature('NgLightweightAnimations')) vs having individual metrics for enter and leave?

The dev mode check should be present at this level, otherwise the function itself would be retained (the contents of the function would be tree-shaken, but not the function def itself).

Curious what'd be an example of such usage (I believe host bindings might be in that category)? If we come up with an example, we can see what'd be the best way to handle it and whether the code should be at compile-time (preferable, if possible) or at runtime.

We can keep it like this for now, but we should probably output a dev-mode warning for cases when timeout is triggered (for ex. for cases when the "done" callback is never invoked). That can help to surface this situations and we can see if there are cases when the timeout needs to be adjusted.

A couple additional thoughts:

• This can be done in a followup PR
• We should consider cases when those warnings are produced for elements inside of@for loops and avoid spamming console with those warnings

Got it, thanks. My thinking was to have this check somewhere at the module level, e.g. check ifdocument.body.getAnimations is present and just use this info everywhere. The problem that in this case well be bypassingrenderer completely, which may not be great (but likely in practice it'd not be a big problem). Maybe there are some other ways to do it? WDYT?

Another idea is that we can move this check from this utility function into an instruction, so that we just exit early (similar to SSR checks).

Should it be aWeakMap instead?

We should try to minimize the amount of non-tree-shakable code here (in favor of having more code inside of the registry itself):

Based on the current implementation, theElementRegistry would always be present (since it's provided inroot).

I like the idea that@JeanMeche proposed: we can use "features" subsystem to bring inElementRegistry in a tree-shakable way. Another benefit would be that theElementRegistry would not need to be exposed via public API, since renderers would not need to have it as an argument. We'll be able to retrieve it from the component type (seetype: RendererType2 in thecreateRenderer method below). The "feature" would basically be a function that given an injector, returns an instance of theElementRegistry (e.g. callsinjector.get(ElementRegistry, ...)).