Skip to content

Syntax for writing a dictionary of attributes onto an element #5071

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
chanan opened this issue May 2, 2018 · 33 comments
Closed

Syntax for writing a dictionary of attributes onto an element #5071

chanan opened this issue May 2, 2018 · 33 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components Components Big Rock This issue tracks a big effort which can span multiple issues Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-razor-pages Needs: Design This issue requires design work before implementating.

Comments

@chanan
Copy link

chanan commented May 2, 2018

This issue is being opened by request of @SteveSandersonMS Please see the full issue details https://github.com/aspnet/Blazor/issues/735 in the Blazor Repo.

The summary is that will be nice to be able to pass arbitrary attributes from a custom component down to an HTML element. For example:

<MyComponent id="myid" />

in the component render it as:

<button id="myid" />

without having to code every single attribute possible. Not mention allowing attributes such data-* to be passed down.

@SteveSandersonMS SteveSandersonMS changed the title Allow adding arbitrary attributes to a component Syntax for writing a dictionary of attributes onto an element May 2, 2018
@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented May 2, 2018

Edit: @rynowak hijacking this post

Summary

We want to have a feature for splatting arbitrary attributes into an element or component.

This will look like:

    <div @attributes="myAttributes" another="something">...</div>

Status/Items

Preview 6 update: The compiler support (splat operator) didn't make it for VS16.2-preview2. So it's possible to use this feature from components written in C#. We'll add the language feature in preview 7.

  • Make last attribute wins work for markup blocks and prerendering
  • Add AddMultipleAttributes to render tree builder
  • Add a new attribute for gather parameters
  • Implement parameter binding for gather parameters
  • Add an analyzer for gather parameters
    • Require dictionary-like property
    • Require a single gather parameter per type
  • Compiler support for AddMultipleAttributes
  • E2E tests
  • E2E tests with our form controls

Syntax

Other attributes would be allowed to appear before or after the splat:

    <div foo= "bar" @attributes="myAttributes" another="something">...</div>

Multiple splats are allowed:

    <div foo= "bar" @attributes="myAttributes" another="something" @attributes="myOtherAttributes">...</div>

Attributes and expressions will be evaluated from left to right (as they are today).

Semantics

We should define this feature in terms of an IEnumerable<KeyValuePair<<string, object>>. This means that types like the following would all work:

  • Dictionary<string, object>
  • List<KeyValuePair<string, object>>

If desired we can also make this work for arbitrary objects by mapping their properties into key-value pairs like we do in ASP.NET Core routing.

As in other languages we should rely on ordering to determine precedence. I propose we follow what typescript does, and allow later attributes to take precedence.

Examples:

@{
    var values = new Dictionary<string, string>()
    {
        { "a", "123" },
        { "b", "456" },
    }
}

<!-- results in <div b="456" a="ABC"></div> -->
<div @attributes=values  a="ABC"></div> 

<!-- results in <div a="123" b="456"></div> -->
<div a="ABC" @attributes=values></div> 

Implementation concerns

We have a choice of how to implement this semantic. Given the following code:

<div a="ABC" @attributes=values></div> 

This will map to something like:

builder.OpenElement(0, "div");
builder.AddAttribute(1, "a" "ABC");
builder.AddMultipleAttributes(2, values);
builder.CloseElement(0, "div");

note: I propose that we do the obvious thing with sequence numbers... We map a single splat to a single render tree builder call and a single sequence number

We have to decide how to do the de-duplication of the duplicate attribute (in this case a). This could be handled by the render tree builder (a) shows up once in the render tree or it could be handled by the renderer (a) shows up twice in the render tree, and the renderer is smart enough to do the right thing.

I'm looking for thoughts and input on which is the better approach.

@aspnet-hello aspnet-hello transferred this issue from aspnet/Razor Dec 14, 2018
@aspnet-hello aspnet-hello added this to the 3.0.0 milestone Dec 14, 2018
@aspnet-hello aspnet-hello added 1 - Ready area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-razor-pages Needs: Design This issue requires design work before implementating. labels Dec 14, 2018
@danroth27 danroth27 added the area-blazor Includes: Blazor, Razor Components label Feb 6, 2019
@danroth27 danroth27 modified the milestones: 3.0.0, 3.0.0-preview4 Feb 25, 2019
@rynowak rynowak added the Components Big Rock This issue tracks a big effort which can span multiple issues label Mar 4, 2019
@rynowak rynowak mentioned this issue Mar 4, 2019
56 tasks
@rynowak
Copy link
Member

rynowak commented Apr 25, 2019

@SteveSandersonMS - added some design notes, would like your feedback on this.

@SteveSandersonMS
Copy link
Member

This basically all looks great to me and matches what I think we've discussed before.

the only syntactic conflict is with directive attributes

One further syntactic possibility is to copy JS/TS and be explicit that we're merging contents by prefixing with ... as JS does. So the closest Razorish equivalent to the JS/TS syntax would be:

<somelem normal-attribute="something" @...myAttributes />

You'd then get a compile error if you just tried to put @myAttributes inside an element, except if that's the name of a directive attribute.

I'm not 100% determined that we should do this, but am interested in what you think.

@chucker
Copy link

chucker commented May 2, 2019

I'm not sure JS-esque @... i quite the right 'idiomatic' syntax for C#, but I do favor such explicitness, given recent issues like #9786 which arose because the Razor syntax isn't particularly explicit.

@mkArtakMSFT mkArtakMSFT removed the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label May 9, 2019
@rynowak
Copy link
Member

rynowak commented May 31, 2019

Would it make sense to also allow assignment of a proxy-object accompanying the dictionary of attributes, and have that object implement the render-behavior? The default implementation would have the duplicate order preference you discussed.

I don't think a feature is needed for this. If a component defines a CaptureUnmatchedAttributes parameter, you can assign it directly.

@rynowak
Copy link
Member

rynowak commented May 31, 2019

Also, to your examples you could do exactly what you're asking for once we have the language feature by returning an IEnumerable<KeyValuePair<string, object>>. So, I guess that's a yes 👍

@Liander
Copy link

Liander commented Jun 1, 2019

Also, to your examples you could do exactly what you're asking for once we have the language feature by returning an IEnumerable<KeyValuePair<string, object>>. So, I guess that's a yes 👍

Thanks, Yes, for some of the examples that are only providing another API in a typed manner then one would only need to handle duplicates (at most) and it resembles with this splat handling.

For the list example I was hoping that one could have it generating other presets to child list-items to set 'list-group-item' automatically, and similar for the 'form' example, it could target settings of label, placeholder etc. To override the default render behaviour would mainly be to make those settings available to children. Making a component is always always an alternative but there is a risk of getting myriads of things like MyMaterialDesignForm, MyBootstrapForm, etc.

When it is only about passing along settings to chidren it could be an alternative to have the ability of passing along defaults selectively to children (by target element ID or by type in these examples) instead of making a component for handling it. Can that be done somehow with CaptureUnmatchedAttributes? To be used by child elements selectively instead of making components?

@rynowak
Copy link
Member

rynowak commented Jun 1, 2019

When it is only about passing along settings to chidren it could be an alternative to have the ability of passing along defaults selectively to children (by target element ID or by type in these examples) instead of making a component for handling it.

For now this kind of scheme would have to be built-in the the component you're calling. We have no immediate plans (3.0) to build metaprogramming or macro-like extensbility.

@Liander
Copy link

Liander commented Jun 1, 2019

For now this kind of scheme would have to be built-in the the component you're calling. We have no immediate plans (3.0) to build metaprogramming or macro-like extensbility.

Yes, a macro might behave similarly but then the elements need to be predefined. Building on this splat feature, please note that you should already be able to achieve this by having one "splat factory" register values to a service that another "splat factory" retrieves values from in a child element giving some filter key. Manually injecting those factory calls on all child elements would be impractical and ugly though. I wanted to point out the similarities to see if those calls could be handled somehow by the framework instead. Anyway, thanks for thinking about it.

@SteveSandersonMS
Copy link
Member

@rynowak Did this issue stall on us not concluding the syntax/name? Or do you consider the plan to be decided?

@rynowak
Copy link
Member

rynowak commented Jun 5, 2019

Did this issue stall on us not concluding the syntax/name? Or do you consider the plan to be decided?

Correct. I'm proposing we use @attributes

@SteveSandersonMS
Copy link
Member

Even for components, even ones that might not have anything to do with HTML rendering?

I feel a bit odd about this, but maybe we justify it by saying you're meant to think of it as a syntactical macro - you're asking the framework to pretend a bag of stuff was actually a bunch of attributes in your anglebrackets-source-code. It is strange though:

<PersonEditor Person="@data.Person" @attributes=@ExtraParams />

@functions {
    [Parameter(CaptureExtraValues = true)] public IDictionary<string, object> ExtraParams { get; set; }
}

@rynowak
Copy link
Member

rynowak commented Jun 5, 2019

Even for components, even ones that might not have anything to do with HTML rendering?

I really don't want to pick a different name for applying extra attributes to components. That sounds like something we'll explain to users eternally.

@SteveSandersonMS
Copy link
Member

Yep, having two names would be pretty sucky and I don't want that either. But the problems with @attributes are enough to make me think @splat or @combine or @values would be more satisfying in the long run.

@danroth27
Copy link
Member

I think @attributes is fine. The markup syntax is still elements and attributes, even if some of those elements are components that use attributes to specify parameter values. That's is how I describe the syntax for using components for folks that are new to Blazor: You use an HTML-like syntax where the element matches the component type name and the attributes specify parameter values.

I also like @spat because I think it sounds fun, but after asking around a bit it seems not many folks agree with me 😄.

@rynowak
Copy link
Member

rynowak commented Jun 6, 2019

Who didn't agree. Cool people or .... them?

@SteveSandersonMS
Copy link
Member

OK. Philosophically, @attributes is different to how I've thought about the meaning of what appears in .razor files. It's a statement about Razor syntax, rather than being a usage of Razor syntax. For it to make sense, you have to think of it as being a macro or another kind of metaprogramming.

In the end none of the other options are ideal either, so I'll stop being perfectionist about this. I can accept @attributes as being like a macro even though it gets resolved at runtime.

@Liander
Copy link

Liander commented Jun 6, 2019

In a mixed environment of C# and HTML I think it is a bit unfortunate to use the name attributes. Isn't something like @apply more descriptive? (Maybe it's confusing using a verb.)

@stavroskasidis
Copy link

Why not use this syntax to avoid conflicting with the directive attributes

<div a="ABC" @(values)></div>

I don't think it gets clearer than that IMO.

@rynowak
Copy link
Member

rynowak commented Jun 13, 2019

@stavroskasidis we considered this and we don't want to make a distinction in behavior between two constructs that are very similar in other contexts.

Ex:

<div>@Message</div>
<div>@(Message)</div>

<input @bind>
<input @(bind)>

In examples 1 and 2 they are semantically equivalent (evaluate the expression).

In examples 3 and 4 they are different. 3 is a directive attribute, and 4 is an expression.

When you but these examples side-by-side it starts to get vexing. We don't expect the usage of this feature to be extremely common, and using an attribute for it is already pretty concise. We have the ability to make something more concise later if it's needed.

@stavroskasidis
Copy link

Ok, that makes sense. Thank you for answering.

@abhisheksiddhu
Copy link

In preview6 @attributes is also used at the top level in component to apply attributes on components like authorize, I fear many users might get confused between the two.

@danroth27
Copy link
Member

@abhisheksiddhu Applying an attribute to a component class is actually done with @attribute (singular). Even so, I agree there is some potential for confusion. Tooling should help out here.

@rynowak rynowak added Done This issue has been fixed and removed Working labels Jun 25, 2019
@rynowak rynowak closed this as completed Jun 25, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components Components Big Rock This issue tracks a big effort which can span multiple issues Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-razor-pages Needs: Design This issue requires design work before implementating.
Projects
None yet
Development

No branches or pull requests