Infer component generic types from ancestor components

[SteveSandersonMS]

Summary

Currently, component generic type inference is based exclusively on parameters passed to that component, ignoring any other context such as ancestor components. This imposes problematic limitations in more sophisticated scenarios, particularly for component vendors trying to create a really smooth consumption experience.

See customer report in #7268, but note this has also been requested independently by component vendors.

Motivation and goals

The classic example is a generic <Grid> component containing generic <Column> children. As of today you have to do something like this:

<Grid Items="@people">
    <Column TItem="Person" Name="Full name">@context.FirstName @context.LastName</Column>
    <Column TItem="Person" Name="E-mail address">@context.Email</Column>
</Grid>

...when what you actually want to do is this:

<Grid Items="@people">
    <Column Name="Full name">@context.FirstName @context.LastName</Column>
    <Column Name="E-mail address">@context.Email</Column>
</Grid>

Notice how the consumer has to re-specify TItem on each column, because it can't be inferred from the enclosing <Grid> (even though <Grid> itself can infer based on its own Items parameter).

In scope

• Inferring a generic type from an ancestor in the same .razor file
  • ... when a same-named generic type is specified explicitly on the ancestor (e.g., <Grid TItem=Person>)
  • ... when a same-named generic type is inferred on the ancestor (e.g., <Grid Items=@someEnumerableOfPerson>)
• Sensible overriding rules
  • We still prefer to use explicitly-provided generic type parameters (e.g., <Column TItem=Person>) - that overrules all inference
  • Failing that, we still prefer to infer based on a regular [Parameter] - that overrules inference based on ancestors
  • Only failing both of those do we go looking for same-named ancestor generic type parameters
• Continuing to provide a good compile error if one or more generic types can't be inferred

Open questions:

• Instead of only matching based on generic type names, should we have a way to specify in a strongly-typed way which other component(s) are valid suppliers of values for each generic type? This probably requires new syntax, e.g., a directive like @typeparam TItem from Grid. I'm not keen on inventing new syntax like that.
• Or should we wait until implementing some kind of "Restrict component hierarchy" feature, and then say we're only going to infer based on the specific ancestors that are declared as required? I'm not convinced this is a good idea, because "Restrict component hierarchy" will probably only support immediate-child/parent relationships, otherwise it runs into trouble when people want to split things over multiple .razor files, and in that case it's not really flexible enough to handle the requirements for generic type inference. For generic type inference, we want to support arbitrary depth ancestors within the same .razor file.

Out of scope

• Partial generic type inference. Due to C# compiler limitations, if you specify any generic type values explicitly, you must specify all of them explicitly. This is an existing limitation in the Razor compiler too, and the work here isn't going to change that.
• Matching against an ancestor in a different .razor file. We can't know which other .razor files are going to contain a reference to your component, hence we don't know what the ancestors will be. Even if we did, there might be multiple ones that would "supply" different generic types. Altogether it's meaningless to imagine we're matching against "ancestors" in entirely separate files.
  • Note that people can still do generic type inference across files, but only the existing form of generic type inference that requires your component to receive a parameter involving that generic type.
• Anything cleverer than matching based on generic type parameter name. For example, even if the closest candidate match is incompatible due to generic type constraints, we're not going to know about that and pick a different candidate. Likewise, if the candidate you want to match has a different name, we can't know that's your intention (even if there's only one such candidate).

Risks / unknowns

It's taking a fairly magic thing and making it more magic still. It will become fairly hard to explain the exact rules around type inference.

It makes the declared generic type parameter name more important. It's no longer just named for explanatory purposes, but also for uniqueness purposes. Some existing components might have generic type parameter names that aren't unique enough (e.g., just T) and so the inference-based-on-ancestors mechanism might match a different ancestor than you intended. This isn't massively bad because you'd know about it at compile time, plus in most cases, developers can change their generic type names to make them more unique when required. We should check that our own generic components have good type generic type names.

If we implement an "Extract Component"-type refactoring in the future, it will have to account for this. That is, it will have to work out which generic types were being inferred from an ancestor, and convert those into @typeparam declarations on the extracted component.

Examples

See the <Grid> example above as the primary scenario.

Other scenarios are anything where you have multiple child components that all take a RenderFragment<T> ChildContent parameter, each of which does something with the same data source. Example: a <Chart> component containing multiple <Series>.

Even if literally the only scenario was "grids", I think that would still be important enough to warrant this work.

Also bear in mind cases where the inference needs to flow through multiple levels in the component hierarchy. You don't always infer based on the immediate parent. For example,

<Grid Items="@people">
    <CascadingValue Value="@someUnrelatedThing">
        <Column Name="Full name">@context.FirstName @context.LastName</Column>
        <Column Name="E-mail address">@context.Email</Column>
    </CascadingValue>
    <Column Name="Actions"><button ... /></Column>
</Grid>

[ebekker]

Based on the out-of-scope "partial inference" comment, would it still be possible to support implicit inference from both immediate parameter and ancestor types? In other words as long as there are no explicit type parameters provided, all other inference types would be allowed?

[ebekker]

Just thinking out loud... instead of inferring the generic type from ancestor components, would it be possible to infer it from Cascading Parameters? This would avoid the unique name dependency, and potentially could even allow inference across files. It would also provide a form of a more strongly-typed generic inference.

[SteveSandersonMS]

I know we've already established that this feature is desirable, so I'm moving on to some candidate implementation suggestions.

Implementation proposals

Before we start, first understand how the existing generic type inference works. As of 5.0, Razor's generic type inference works by converting "the code that emits component frames" into "a method call that emits those component frames". As a simplified example, without generic inference we might have this, where Items is a parameter of type IEnumerable<TItem>:

// <Column TItem=Person ColumnName="@someColumnName" Items=@people />
void BuildRenderTree(RenderTreeBuilder builder)
{
    builder.OpenComponent<Column<Person>>(0);
    builder.AddAttribute(1, "ColumnName", someColumnName);
    builder.AddAttribute(2, "Items", people);
    builder.CloseComponent();
}

Whereas with generic type inference we have this:

// <Column ColumnName="@someColumnName" Items=@people />
void BuildRenderTree(RenderTreeBuilder builder)
{
    EmitColumnComponent(builder, arg0: someColumnName, arg1: people);
}

// This is generated by the Razor compiler
static void EmitColumnComponent<TItem>(RenderTreeBuilder builder, string arg0, IEnumerable<TItem> arg1)
{
    builder.OpenComponent<Column<TItem>>(0);
    builder.AddAttribute(1, "ColumnName", arg0);
    builder.AddAttribute(2, "Items", arg1);
    builder.CloseComponent();
}

The C# compiler does the fancy work of inferring the TItem type at the call site inside BuildRenderTree based on the arg1 expression type.

Notice how EmitColumnComponent has to have parameters for all the parameters of Column<TItem>, even those not involved in generic type inference. We can't just inline the expressions for their values into the code inside EmitColumnComponent, because those expressions might reference other variables that only exist in scope at the original call site inside BuildRenderTree. For example, someColumnName might be a local variable that only exists at this point during rendering (e.g., a loop variable), so we can't reference it directly from EmitColumnComponent.

It's actually slightly more complicated still, but that's the core idea (by @rynowak originally, mentioning him in case he wants to come and share any further wisdom!) and is all we need to know to understand the following.

1. Have the C# compiler get the generic type name from lexical scope

Spoiler: this is not my preferred option - see later

What if, instead of EmitColumnComponent taking parameters for all the parameters of Column<TItem>, it only took ones for those involved in generic type inference? The other parameter values could be inlined into the generated code. For example, if you're passing a ChildContent fragment (which is compiled as a lambda), then the source for that lambda would go inside EmitColumnComponent rather than at the call site.

Then to make this safe so you can still reference variables that are only in scope at the call site, we could change the whole thing to be a local function:

// <Column ColumnName="@someColumnName" Items=@people />
void BuildRenderTree(RenderTreeBuilder builder)
{
    EmitColumnComponent(builder, arg1: people);

    // This is generated by the Razor compiler
    static void EmitColumnComponent<TItem>(RenderTreeBuilder builder, IEnumerable<TItem> arg1)
    {
        builder.OpenComponent<Column<TItem>>(0);
        builder.AddAttribute(1, "ColumnName", someColumnName);
        builder.AddAttribute(2, "Items", arg1);
        builder.CloseComponent();
    }
}

Now, if we have multiple nested levels of type inference, they will get compiled as multiple nested levels of local functions, hence they are within each other's scopes, e.g.:

void BuildRenderTree(...)
{
   ...

   EmitGridComponent<TItem>(...)
   {
       ...

       EmitColumnComponent(...)
       {
           ...
       }
   }
}

So at this point, all the Razor compiler has to do is not put any generic type declarations on the nested Emit... methods if their names exactly match one of the generic type declarations on an ancestor, and by lexical scope, the ancestor's value will get used.

2. Extend the type inference method to take extra synthetic parameters used for inference only

As an alternative, instead of changing where the type inference methods go, we could just add some more parameters to them. As of 5.0, they take one parameter for each [Parameter] on the component you're rendering. But what if we extend this to include not only the [Parameter]s from the component you're rendering, but also include selected [Parameter]s from ancestor components in the same .razor markup?

For example, given this markup (where both Grid and Column declare @typeparam TItem):

<Grid Items="@people">
    <Column IsCoolColumn="true" />
</Grid>

... we generated the following:

void BuildRenderTree(RenderTreeBuilder builder)
{
    EmitGridComponent(builder, arg0: people, arg1: (RenderFragment)(builder =>
    {
        EmitColumnComponent(builder, syntheticArg0: people, arg0: true);
    });
}

static void EmitGridComponent<TItem>(RenderTreeBuilder builder, IEnumerable<TItem> arg0, RenderFragment arg1)
{
    builder.OpenComponent<Grid<TItem>>(0);
    builder.AddAttribute(1, "Items", arg0);
    builder.AddAttribute(2, "ChildContent", arg1);
    builder.CloseComponent();
}

static void EmitColumnComponent<TItem>(RenderTreeBuilder builder, IEnumerable<TItem> syntheticArg0, bool arg0)
{
    builder.OpenComponent<Column<TItem>>(0);
    builder.AddAttribute(1, "IsCoolColumn", arg0);
    builder.CloseComponent();
}

That is, when rendering Column, we notice that its TItem isn't "covered" by either an explicit declaration (TItem=...), nor by a [Parameter] on the component itself. So we fall back on scanning up the ancestor hierarchy to look for the closest thing that has a generic parameter called TItem and was supplied with any [Parameter] value that could "cover" it (or an explicit declaration).

We discover Grid is the closest candidate, and that its type inference made use of a parameter called Items of type IEnumerable<TItem>. So for EmitColumnComponent, we add a synthetic extra argument whose type is IEnumerable<TItem>, and we pass the people value which is known to be in scope because descendant rendering is always within a lambda within the call site for the ancestor.

Even though EmitColumnComponent's logic doesn't use syntheticArg0 in any way, simply passing it makes generic type inference work.

3. Use some extension of <CascadingValue>

We've had a couple of community suggestions (here, here) that <CascadingValue> should be extended to supply generic types as well as actual parameter values.

I don't personally think this works in any way, because generic type inference has to be resolved at compile time in order for generic types to do all the things people want (e.g., produce correct intellisense when operating on instances of those generic types, and give compile-time errors if you reference non-existent members on them).

CascadingValue is a runtime feature, since the actual set of component ancestors can't be known in general until runtime (you could decide it dynamically based on user actions, for example). So it's not meaningful to have it impact compiler output.

About inheriting types across independent files

More generally, I don't think the problem of inheriting types across .razor files can have any solution. Consider sources like this:

@* Index.razor *@

<Grid Items=@people>
    <MyColumns />
</Grid>

@*MyColumns.razor*@

<Column Name="First name">@context.FirstName</Column>
<Column Name="E-mail address">@context.Email</Column>

That can't possibly work at compile time because MyColumns.razor has no way to know it's only ever going to be used from within a specific place inside Index.razor. Even if at runtime that happens to be true, the compiler can't know it. So it can't possibly specify any particular TItems value for the <Column> component.

What you'd actually need to do is add a type parameter onto MyColumns.razor, e.g.,

@*MyColumns.razor*@

@typeparam TItem
<Column TItem=TItem Name="First name">@context.FirstName</Column>
<Column TItem=TItem Name="E-mail address">@context.Email</Column>

Now this can work with design 2 above, because Index.razor infers the type to pass to <MyColumns> based on the match local to that file, and MyColumns.razor explicitly passes that on to <Column> (or maybe we even make the inference match @typeparam on the component itself and pass default(TItem) to the inference function, so you can omit the TItem=TItem parts).

So in summary, I don't think that <CascadingValue> is relevant for solving this problem, and isn't needed because design 2 already produces the best possible result anyway.

Matching rules

The designs above assumed that, when we're looking for a value for TItem on a child component, we'd match that by name against any same-file ancestor that also has a type parameter called TItem. That is, matching on the name as a string. Clearly this has the drawback of not being fully strongly typed.

It would be nice if there was some way to define the exact set of types you were willing to match against. For example:

@typeparam TItem from Grid<>

or, without needing new syntax:

@typeparam TItem
@attribute [InferTypeParameter(nameof(TItem), FromComponent: typeof(Grid<>))]

Notice that we can't refer to Grid<TItem> in the attribute, because attributes can't reference type parameters from their own host class (because the attribute is on the open generic type, not any particular closed one), so we're limited to saying Grid<>, so it doesn't lead to any obvious way to say "I can only be inside a Grid<TItem> with the same type arg". Also notice that if Grid had multiple type parameters, we still have no way to say which of them we mean other than stating a name string, so there's still a level of string-based name matching.

However there's a bigger problem still with this, which is that the Razor compiler doesn't know about type compatibility. If somebody subclasses Grid<TItem> to make SuperGrid<TItem>, the Razor compiler doesn't naturally know about this relationship. Likewise with inferface implementations. Unless we actually walk the chain of all base types and interface implementations and recreate all the "is assignable from" rules inside the Razor compiler. In short, we're not going to get anywhere near doing that.

I tried to think of some way to express the type inference method such that it would end up picking a type arg based on type assignment compatibility, but it's absolutely mind-bending and I think at the very least would involve passing a separate parameter for every ancestor (and to do that, involves an extra layer of lambda-capturing for each level of ancestry). It seems like a truly bad direction to go, but by all means correct me if you see a clean way to do it!

A simple compromise

Since we can't truly have strongly-typed type parameter matching, are we at risk of accidental matches and general confusion when unrelated components both have @typeparam T?

A solution that @javiercn and I came up with is simply to make the inference matching opt-in. That is, type parameters don't flow by default. The developer explicitly enables the flow on particular type parameters, and when they do so, we interpret that as meaning "I promise the name is unique enough for my purposes".

We could either do it on the receiving side:

@* Column.razor *@
@typeparam TItem
@attribute [ReceiveTypeParamFromAncestors(nameof(TItem))]

... or on the sending side:

@* Grid.razor *@
@typeparam TItem
@attribute [CascadeTypeParam(nameof(TItem))]

Perhaps this is where @ivanchev and @ebekker's intuition about an explicit "cascade" gesture does line up with what we can do.

Preferred option

I prefer option 2 above because:

• Unlike option 1, it minimizes the amount we have to change the existing type inference logic. For user code that doesn't need to infer generic types from ancestors, the Razor compiler output would be exactly unchanged, so there's no risk of unanticipated breaking changes.
• Unlike option 1, it doesn't involve inlining parameter values into the type inference function, which could have implications for perf since we'd be closing over more locals and hoisting them. It might even affect behavior when executed within loops.
• Unlike option 1, it means the decisions about matching generic parameters would be implemented in procedural code inside the Razor compiler, so we can use whatever explicit matching rules we want. Whereas with option 1, the matching would happen implicitly inside the C# compiler based on lexical scope, so it would be way harder to put in any special cases. Also it might be much easier to produce good error messages from the Razor compiler if we control the logic more directly.

All this said, it's still a pretty complicated thing to implement, so if there are suggestions of better approaches from @NTaylorMullen, @rynowak, etc., I'd be very interested!

[ebekker]

Just to harp on the cascading param idea a bit more... perhaps I'm missing a subtle point, but while I agree that populating a cascading parameter happens at runtime, isn't the type specification fully declared and known at compile time? You have to define a property on a component class and annotate it to declare a cascading parameter, so it's type should be fully resolvable at compile time. For example:

public class MyComponent<TItem> : ComponentBase
{
    [CascadingParameter]
    public SomeType<TItem> SomeValue { get; set; }
}

It wouldn't matter if SomeValue was ever populated or not, the type is still known at compile time.

Though, I'm sure I'm missing some subtle point here...

[SteveSandersonMS]

@ebekker Sorry but I don't see how that fits in with how generic type inference works. You would need to provide a code sample showing exactly what the compiler would generate, for example in the case I was using above (<Grid Items=@people><Column /></Grid>). Additionally we don't want to give developers an extra job of converting their parameters into cascading parameters when regular parameters would suffice, as in the designs 1 and 2 above.

[Liander]

If you pass the synthetic arguments before the rest of the values to the emit-function I guess that something like <Column ValueSelector = @(x => x.Age) /> can resolve to the type Column<Person, int>, where Person is resolved from the ancestor parameter and int is resolved by the lambda declaration. To get intellisens on lambda. Is that correct and is that the plan? If yes, then I really like it.

[SteveSandersonMS]

@Liander As far as I can tell, yes it would be able to infer the type in this case. However I don't see any reason why it matters what order we put the arguments on the emit function. I think it would work exactly the same if the synthetic arguments go last.

[Liander]

It matters for the intellisens, at least if it works like ordinary factory methods.

[SteveSandersonMS]

It doesn't seem to make a difference when I tried it. If you could post a Gist showing a minimal example of a case where this matters I'd be interested.

[ebekker]

I started to look at the generated code of a sample supporting the cascading parameters and I see the errors of my way -- the surrounding page doesn't have any way to infer the type being passed from parent to child via a cascading parameter between the two -- I see your point now, thanks for humoring me!

I was originally only thinking of the developer/user ergonomics but I can see the implementation is not feasible.

[Liander]

@SteveSandersonMS I am confused. A gist will not show intellisens. I referring to the general c# behavior when having:
public void EmitColumn<T, TValue>(Func<T, TValue> ValueSelector, IEnumerable<T> people) {}
and start typing a call to EmitColumn(x => x. You won't get intellisens after the dot since T is yet to be resolved. But it would be able to infer if you change the order, agree?
Will it work differently here?

[SteveSandersonMS]

@Liander Ah yes, I see what you mean! Thanks for clarifying. I thought you were talking about the order of the parameters declared on the emit function, but now I see you mean the order of the arguments on the invocation of the emit function (which can be a different order since we can use named parameters at the call site).

I'll update the example code above to show the synthetic args going first in both cases to make this clearer and simpler.

[mrpmorris]

@SteveSandersonMS The goal of this is ultimately to allow for strongly typed RenderFragments.

public class Grid<T> : ComponentBase
{
  [ViewParameter]
  public IEnumerable<Column<T>> Columns { get; set; }
}

Nobody wants to allow any old content in their section. We want to allow only the specified types.

This would solve the inferred type requirement explicitly rather than via complex same-name rules.

[SteveSandersonMS]

@mrpmorris I appreciate the appeal of that, but it sounds like a completely different kind of feature proposal. It sounds like the “restrict view hierarchy” suggestion from before, and I’m not sure how it would lead to any type inference based on how the C# compiler works.

Rather than risking confusing people by discussing it in this issue, would you be able to file a separate issue describing your suggestion in more detail? Please include:

• How developers would use the feature, both on the supply and consumption sides
• How it would compose with other Razor syntax. For example people expect to emit a collection of <Column> in a foreach loop, or use if/else to decide what columns should appear. So it does tend to look like arbitrary markup.
• What code we would generate to make any type inference work

If there’s a good and practical way to do this we’d definitely be interested, but let’s do separate feature designs in separate issues. Thanks!

[mrpmorris]

@SteveSandersonMS I very strongly suspect (99.9%) that this is what the original report by @ivanchev is trying to achieve.

If I open a new ticket it is basically going to be the same as #7268 which you have closed in favour of this one.

I really don't think people want magic(ish) propagation of generic type parameters, what they actually want is strongly typed child components. The grid columns in a grid in the original ticket is a perfect example. The original poster doesn't want the consumer to be able to do something like this

<Grid TData="Person">
  <Columns>
    <h1>Hello</h1>
    <p>This isn't a column</p>
    <Column Field=@(x => x.GivenName)/>
  </Columns>
</Grid>

and then have to only render components - they want to be able to prevent anything other than Column in there, and are compromising by asking for cascaded generic type parameters because we are more likely to get that than to get exactly what we want.

Please correct me if I am wrong @ivanchev

[SteveSandersonMS]

Please file a separate issue with the details requested.

[mrpmorris]

Done, thanks!

#29379

[ivanchev]

@mrpmorris - these are two separate issues indeed. What you are referring to is to restrict the children of a certain RenderFragment Parameter(Columns), or component. While this is also useful, it's not the feature I requested.

What I requested is exactly what is being proposed here by @SteveSandersonMS - a way to infer the type of child components. This will allow strongly typed child fragments, without specifying a type on each column.

[SteveSandersonMS]

Thanks @ivanchev for clarifying. Since this proposal is not about restricting the type hierarchy, it allows for certain scenarios that are more general than would be allowed if we did restrict the type hierarchy. For example, having a layer of <CascadingValue> in between the <Grid> and a specific collection of <Column>. I've added this example into the proposal above. This helps to make clear how it's a different objective than #29379 (which is also a valid and useful proposal, but addresses a different need).

[SteveSandersonMS]

Also, yesterday @javiercn and I discussed these proposals at length and came up with a very simple improvement to the name-based matching to avoid most cases of accidental matches. See the new section about Matching rules above.

[Liander]

Maybe it is not an intentional detail, but I do not think you can have the Emit-functions in proposal 2 as static because you should be able to access component instance members, correct?

(That said, I actually do want to have the option of generating them static, to control what instance state to access and have change-detection of that explicit dependency if any exist, but that is another story...)

Oh.. looks like it is only the BuildRenderTree that nests the Emit-functions... I must have recalled wrong... Ignore my note.

[SteveSandersonMS]

They can be static, and in fact can be in an entirely different class, because they don't directly access anything from scope or other component members. They receive all the parameters as arguments (for example, child content is a RenderFragment delegate). This is true for the existing type inference and AFAIK will continue to work the same with proposal 2.

[craigajohnson]

@SteveSandersonMS this actually opens up a whole set of possibilities and I wonder if it syncs up a bit with the concept of a child component knowing as much as possible about its parent. We're having to do some tricks now to build out the component hierarchy. I wonder if having the TParent type param along with a corresponding TParent Parent parameter available for all components would solve this? We're effectively doing this now but only by subclassing ComponentBase and monkeying more than we'd like.

[rynowak]

I have been summoned...

Option 2 seems pretty well thought through.

This isn't that similar to how I'd imagined we'd solve this when I was on the team but seeing it all laid out with an example makes it really clear. I'm pleased that this solution avoids evaluation-order issues, which was one of my primary concerns implementing both this and the original type inference feature.

For those that are driving by and don't get what I mean, it's pretty important that the generated code evaluates the component parameters in the order that you pass them from left to right (since that's what C# does).

<Grid Items="@GetPeople()" AnotherThing="@DoSomething()">
    <Column IsCoolColumn="true" />
</Grid>

If mutate state in GetPeople() and DoSomething() you really don't want these calls to be reordered when you do something subtle like add type inference for a body.

Synthesizing a 'thunk' and calling it avoids that problem since you're now working with a variable that already holds a value rather than copy-pasting code. The temptation for folks that are new to Razor and compilers in general is to assume that we can just copy-paste your code into a new context, and examples like the above demonstrate why you can't.

---

The other path that I had in mind for this issue was related to discovery and nested classes. I'm a nested-class-a-holic, if you look at any codebase I've worked on seriously you will seem them everywhere! I wanted the world to share my joy.

The insight is that a container like Grid and the Column that goes inside it are tightly coupled. We've encountered this scenario as well for tag helpers - where you want to build a Grid but you want to only allow it contain Columns. The nested class solution to this is nice for the reason that it's something static - we can detect at discovery time the relationship between these, and Column can't appear outside of a Grid (decide on your own if that's good or not 😆).

Nested classes can solve both of these problems at once - but only work really well if you always want to solve them both together. For instance if Column is a nested class of Grid then it's proper name is actually Grid<T>.Column and Column itself has no type parameter of its own. I hadn't thought through this to the codegen level so a solution like Option 2 above might also be needed to make the nested class thing work.

---

The one part of this that I don't love is @attribute [ReceiveTypeParamFromAncestors(nameof(TItem))]. It feels a little arbitrary and clumsy. I'd want control to say which ancestors.

---

Anyways much ❤️ to the community and team. I'm off on a new adventure now, and I'm really have to see the community so engaged and the team cranking out awesome work.

[SteveSandersonMS]

Done in #29767