dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.43k stars 10.01k forks source link

Add ability to pass @typeparam down to child components #7268

Closed ivanchev closed 3 years ago

ivanchev commented 5 years ago

Is your feature request related to a problem? Please describe.

If you have a templated component, that's Generic and uses @typeparam, if you want to add other child components, you also have to specify the type when you declare them. For instance in a Grid:

<Grid Data=List<Item>>
  <Columns>
    <Column TItem=Item>
      <Template />
    </Column>
  </Columns>
</Grid>

In this setup, in order to have a strongly typed context passed to the Column templates, you have to specify explicitly TItem to each Column and cannot be inferred by the List.

Describe the solution you'd like

With the current setup we can pass simple parameters down to child components with:

<CascadingValue Value=this>
    @Columns
</CascadingValue>

I would like to have the ability to aslo pass down @typeparam values. Perhaps something like:

<CascadingType Value=TItem>
    @Columns
</CascadingType>

And if the child component declares it's own TItem, it will override the Cascading value.

SteveSandersonMS commented 5 years ago

I agree with the needs-design label!

It's not clear to me what the right design for this would be, or even if it's possible at all. If the outer component obtained its generic type through inference, then the generated code doesn't know about it at all prior to compilation. So I'm not clear that it's possible to pass it to nested components.

@rynowak Am I missing any obvious possible solution? Do you have any ideas about how something like this could be achieved? Or if not, any thoughts on an alternative feature we might be able to offer to achieve a similar goal of determining generic types based on tag hierarchy? Better still would be inferring generic types based on cascading parameters, but I don't see how that would be possible given that we don't know about cascading parameters until runtime.

ivanchev commented 5 years ago

If inference is the main problem, having to specify the type on the main component explicitly is still a viable option, as long as it's only specified in just one place.

EdCharbeneau commented 5 years ago

I'm glad to see a milestone for this. We (Telerik) have been getting requests for this feature from customers. The description is generally "Please add HTML helper like model binding intellisense". This is definitely something developers want.

Here is one such example: Check out @robertmclaws’s Tweet: https://twitter.com/robertmclaws/status/1106267471965798411?s=09

los93sol commented 5 years ago

Yep, this is a big one. Fall thru logic seems good here. If it’s not declared on the element itself but is declared on a parent with the same condition couldn’t that type param be inferred?

Liander commented 5 years ago

Regarding type inference into child contents: An observation is that when you have a context parameter assigned to an attribute you will get type inference. In other words, you would also get type inference if the context is passed to the inference method in cases when it is not used also. Then there is the problem that you don't always have a context parameter and the problem knowing which components you want to use inference on, but it indicates that there should be a construct one can use for the inference.

I did an experiment where I divided the CreateGrid inference-method into OpenGrid, AddContent and Close, where the open-grid does the inference from the (non-content) attributes and returns a TypeInference<T> singleton, which is handed over to methods building contents just for inference.

      protected void BuildRenderTree(RenderTreeBuilder builder)
      {
         builder.OpenElement(10, "div");
         TypeInference
            .OpenGrid_0(builder, 12, 13, "Data", data)
            .AddContentAttribute(builder, 14, "Columns", (typeInference_0) => (builder2) =>
               {
                  TypeInference
                     .OpenColumn_1(builder2, 16, typeInference_0, 18, "ValueSelector", person => person.Name)
                     .AddContentFunction(builder2, 19, "CellContent", (typeMap) => typeMap.T2Arg, (typeInference_1) => (value) => (builder3) =>
                       {
                          builder3.OpenElement(20, "div");
                          builder3.AddContent(21, value);  // value is here <string>
                          builder3.CloseElement();
                       })
                     .CloseComponent(builder2);
                  TypeInference
                     .OpenColumn_2(builder2, 22, typeInference_0, 23, "ValueSelector", person => person.Age)
                     .AddContentFunction(builder2, 24, "CellContent", (typeMap) => typeMap.T2Arg, (typeInference_2) => (value) => (builder3) =>
                     {
                        builder3.OpenElement(25, "div");
                        builder3.AddContent(26, value);  // value is here <int>
                        builder3.CloseElement();
                     })
                     .CloseComponent(builder2);
               })
            .CloseComponent(builder);
         builder.AddMarkupContent(27, "\r\n");
         builder.CloseElement();
      }
internal static class TypeInference
   {
      public static TypeInference<TItem> OpenGrid_0<TItem>(RenderTreeBuilder builder, int seq, 
         int __seq0, string __name0, IReadOnlyList<TItem> __arg0)
      {
         builder.OpenComponent<Grid<TItem>>(seq);
         builder.AddAttribute(__seq0, __name0, __arg0);
         return TypeInference<TItem>.Instance;
      }
      public static TypeInference<TContext, TValue> OpenColumn_1<TContext, TValue>(RenderTreeBuilder builder, int seq, TypeInference<TContext> typeInference, 
         int __seq0, string __name0, Func<TContext, TValue> __arg0)
      {
         builder.OpenComponent<Column<TContext, TValue>>(seq);
         builder.AddAttribute(__seq0, __name0, __arg0);
         return TypeInference<TContext, TValue>.Instance;
      }

The helper classes of TypeInference<…> are kept out for brevity. As one can see from the above code the inference parameters are available in sub-nodes so there is a possibility to define rules for follow-throw of defining types, however one must consider possibilities of mappings to different names.

An alternative which I think is both clearer and a great feature is to also introduce the ability to constrain contents and only accept direct child-nodes of a specific contract type instead, like having Parameter(ContentType="IColumn<TItem>"). Matching is then by type instead of name.

Hopefully this can be of some sort of inspiration.

mrpmorris commented 4 years ago

I think this would be better solved by this approach -> https://github.com/aspnet/AspNetCore/issues/17194

Ultimately it is a request to

  1. Be able to infer a generic type based on the parent.
  2. Allow the developer to force the consumer to only be able to enter child content of a specific type (array of Column<T>)

The only way I can think of to strongly tie the generic type of a child component to the generic type of the parent component is to define it in C#.

public class Grid<T> : ComponentBase
{
  [Parameter]
  public IEnumerable<T> Data { get; set; }

  [ComponentArray]
  public Column<T>[] Columns { get; set; }
}

<!-- Valid -->
<Grid Data=People>
  <Columns>
    <Column GetValue=@(x => x.Title)/>
    <Column GetValue=@(x => x.FullName)/>
  </Columns>
</Grid>

<!-- Won't compile -->
<Grid Data=People>
  <Columns>
    <h1>Hello world</h1>
  </Columns>
</Grid>

Ivan - what are your thoughts?

joelbyrd commented 4 years ago

If inference is the main problem, having to specify the type on the main component explicitly is still a viable option, as long as it's only specified in just one place.

@ivanchev Are you saying this is possible now, or just that it would be viable to make possible?

ivanchev commented 4 years ago

It's not possible now, but it would be an acceptable solution.

ghost commented 4 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

ghost commented 4 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

SteveSandersonMS commented 3 years ago

Clearing the assignment just because it was put there nearly a year ago, and there's no reason to assume who's going to be doing this in the current sprint. If Javier wants to that's totally fine, just don't want to assume that.

SteveSandersonMS commented 3 years ago

This issue is Needs: Design. I'm writing up a design in a separate issue #29349, so closing this one in favour of that one.