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

TagHelperAttributes are now immutable #151

Open
NTaylorMullen opened this Issue Feb 5, 2016 · 1 comment

Comments

Projects
None yet
1 participant
@NTaylorMullen
Copy link
Member

NTaylorMullen commented Feb 5, 2016

Changing attributes in place

Before:

var attribute = output.Attributes[0];
attribute.Value = "something different"

After:

var attribute = output.Attributes[0];
output.Attributes[0] = new TagHelperAttribute(attribute.Name, attribute.Value, attribute.Minimized);

We also added an IndexOfName method to coincide with locating specific attributes and changing them in place.

Setting attributes

Before:

output.Attributes["class"] = "btn"; // This was done via an implicit operator hack before
output.Attributes["checked"] = new TagHelperAttribute("checked", "true");

After:

output.Attributes.SetAttribute("class", "btn");
output.Attributes.SetAttribute(new TagHelperAttribute("checked", "true"));

The hack I hint to in the first example is that any string used to be implicitly converted to a new TagHelperAttribute with name = null and value = the string and then in the string indexer we'd do crazy error detection logic to prevent invalid scenarios and then finally set the attributes Name to the provided key. With the new immutability we were able to centralize a lot of the error logic and not have it pollute the code in every area.

IReadOnlyTagHelperAttribute

Before:

var attributes = new List<IReadOnlyTagHelperAttribute>();
...
var attributeList = new TagHelperAttributeList(attributes);

After:

var attributes = new List<TagHelperAttribute>();
...
var attributeList = new TagHelperAttributeList(attributes);

Now that TagHelperAttribute is an immutable type there's no need for the IReadOnlyTagHelperAttribute type. It's been removed. Part of this change also resulted in ReadOnlyTagHelperAttributeList<TAttribute> being changed to ReadOnlyTagHelperAttributeList since there's no more differentiation between IReadOnlyTagHelperAttribute and TagHelperAttribute.

ReadOnlyTagHelperAttributeList is abstract

There wasn't a lot of value in creating a ReadOnlyTagHelperAttributeList by itself (we didn't) so for the sake of simplifying the API decided to make it abstract.

Before:

new ReadOnlyTagHelperAttributesList<IReadOnlyTagHelperAttributeList>();
new ReadOnlyTagHelperAttributesList<TagHelperAttribute>();

After:

new ReadOnlyTagHelperAttributesList();
new ReadOnlyTagHelperAttributesList();

@NTaylorMullen NTaylorMullen added this to the 1.0.0-rc2 milestone Feb 5, 2016

@aspnet aspnet locked and limited conversation to collaborators Feb 5, 2016

@NTaylorMullen

This comment has been minimized.

Copy link
Member

NTaylorMullen commented Feb 5, 2016

Please discuss at: aspnet/Razor#687

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.