I'd propose to use null instead of undefined here:

and also make the animations field of the DirectiveMeta structure non-optional. This should be more inline with the rest of the code.

Can we avoid having any here?

The any is definitely an issue here. The static evaluation result of the animations metadata can also be DynamicValue, as we no longer require static understanding of the animations data at compile time (unlike VE) so for cases where someone does animations: buildComplexAnimation(foo, bar) this will be a DynamicValue.

Can we do additional processing of this data to arrive at just the animation trigger names? I suppose that's sufficient for autocompletion purposes and it's potentially useful during template type-checking. This could be modelled as e.g. string[] | 'dynamic' | null to distinguish statically analyzable animation triggers, dynamic animations and no animations. It does not allow for partially dynamic animations, but I would keep it simple for now (treat it as dynamic if it's not possible to extract all trigger names)

The any is definitely an issue here. The static evaluation result of the animations metadata can also be DynamicValue, as we no longer require static understanding of the animations data at compile time (unlike VE) so for cases where someone does animations: buildComplexAnimation(foo, bar) this will be a DynamicValue.

Can we do additional processing of this data to arrive at just the animation trigger names? I suppose that's sufficient for autocompletion purposes and it's potentially useful during template type-checking. This could be modelled as e.g. string[] | 'dynamic' | null to distinguish statically analyzable animation triggers, dynamic animations and no animations. It does not allow for partially dynamic animations, but I would keep it simple for now (treat it as dynamic if it's not possible to extract all trigger names)

The DecoratorHandler.register callback is designed to register analysis data from a previous build into the object graph of the current build, e.g. for publishing directives into the NgModule scope registry. As such, it should ideally not be doing "real" work such as partial evaluation; if this information has to be computed then that is done in DecoratorHandler.analyze and stored in the resulting metadata type.

The value type of string is not accurate here; animations are a complex structure of objects to describe the animation sequences. This also isn't necessarily an array of Maps, as it could be entirely dynamic as mentioned before, and animations also support nested arrays (e.g. [[trigger('trigger')]] is supported). If we have logic to extract just the animation trigger names as suggested before, this comment will also be addressed.

@JoostK I add a new animationsMate here because I don't know why the meta.animations will be replaced during the compilation step, see the comment in line 346.

That comment is outdated, it used to be associated with pipes and directives (see 27bc7dc). Feel free to remove it.

You'll need a new field for this data anyway, as the existing animations is a WrappedNodeExpr that just contains the original ts.Node from the source, without having gone through any static analysis.

This could be modelled as e.g. string[] | 'dynamic' | null to distinguish statically analyzable animation triggers, dynamic animations and no animations. It does not allow for partially dynamic animations, but I would keep it simple for now (treat it as dynamic if it's not possible to extract all trigger names)

@JoostK I think it's better to be Array<string|DynamicValue>|null. It's still useful to provide the completion if not all trigger names can be extracted.
Now I will ignore the DynamicValue if the nested array contains the dynamic animations. Or I should clear the collected name and treat it as dynamic

I was deliberately avoiding DynamicValue as it is only produced in the partial evaluator, whereas we'll also want to use 'dynamic' (or similar) in your retrieveAnimationName for objects that aren't recognized (at which point we aren't able to create a DynamicValue). An alternative could be to model it using something like Array<string | null> | null, although that's a bit less descriptive.

I suggest to name this more aptly animationTriggerNames, as meta is such a generic term.

I was deliberately avoiding DynamicValue as it is only produced in the partial evaluator, whereas we'll also want to use 'dynamic' (or similar) in your retrieveAnimationName for objects that aren't recognized (at which point we aren't able to create a DynamicValue). An alternative could be to model it using something like Array<string | null> | null, although that's a bit less descriptive.

Please make this a free function, as there's no class data needed here. As a suggestion for the name, consider using collectAnimationTriggerNames as collect is a good indication that res is filled (in favor of a return value)

We generally use for-of loops instead of forEach.
The underscored _val name typically indicates that the variable is not used, but it is here.

I would expect that we record the else case here (and above if name is not a string) also as dynamic. It's possible that _val is a DynamicValue at this level, as the partial evaluator retains arrays even if it contains a dynamic value.

That comment is outdated, it used to be associated with pipes and directives (see 27bc7dc). Feel free to remove it.

You'll need a new field for this data anyway, as the existing animations is a WrappedNodeExpr that just contains the original ts.Node from the source, without having gone through any static analysis.

Consider using a more descriptive name for this as well, ideally it's identical to the name used in the analysis object.

You'll also want tests for dynamic data, where two cases are relevant:

1. the value itself is completely dynamic:

animations: buildComplexAnimations(),

2. an individual animation is dynamic:

animations: [trigger('a'), buildComplexAnimations()],

where buildComplexAnimations can be something that will be dynamic due to containing multiple statements:

function buildComplexAnimations() {
  const name = 'complex';
  return [trigger(complex)];
}

I just add a new interface, that's more descriptive.
Maybe it's better to use isPartial instead of includeDynamicAnimations.

The interface was a great idea! I don't mind either name, so sticking with the one you have is fine (although I did suggest a minor change to the name)

Also could you move this function after the main class, as this is only auxiliary logic I feel it fits better there.

Could you update this description please

This can be removed.

The animations should be Array, but if the user's input is not Array (although the ts will report the error). This is why I add the condition here. If it's no Array, the function collectAnimationNames will be broken.

You are correct. My suggestion was to remove the res variable but I somehow goofed up the selection range. But I notice you removed the res so this one has been resolved :-)

Please avoid such non-descriptive variables names:

super nit:

The interface was a great idea! I don't mind either name, so sticking with the one you have is fine (although I did suggest a minor change to the name)

This assertion is superfluous, given the next expectation that test for the full array contents. The same is true for the two tests below.

Was this meant to be something like this?