Components should be based on usings not on @addTagHelper

[SteveSandersonMS]

Some rough experience notes:

Component binding will be based on using and C# namespaces instead of directives:

• Components can bind with the short name when in scope (C# name binding rules)
• Components can bind with a fully-qualified name
• Components can bind with an alias (using Foo = Bar)
• Binding considers using in the same document and imports
• Binding considers the current namespace of the document
• Usings that bring in components aren't grey-ed out
• no support for global:: qualification
• no support for partially-qualified names (@using MyCompany + <MyLibary.FooBar> => MyCompany.MyLibrary.FooBar
• Components respect @namespace directive (blocked by Add support for @namespace directive for components #8007)

Directive attributes:

• directive attributes like bind, onclick, ref are always bound in a component document

Avoiding crosstalk:

• ITagHelper and VCTH tag helpers are never bound in a component document
• Components are never bound in a view document (will be supported in the future)
• addTagHelper/removeTagHelper/tagHelperPrefix are errors in a component document or _Imports.razor
• directive attributes like bind, onclick, ref are never bound in a view document

Tooling:

• Completion supports all of the above points

If it's not specified here then we're probably OK with the way it currently works for Blazor 0.7 - please feel free to ask and add more notes

[rynowak]

This can also be removed once we have this feature: https://github.com/aspnet/AspNetCore-Tooling/blob/master/src/Razor/src/Microsoft.AspNetCore.Razor.Language/Components/ComponentImportProjectFeature.cs#L50

Currently we generate an addTagHelper for our best guess at the project's root namespace. I'm doing the work to separately flow the root namespace, but I can't clean up this part as part of that work. Ultimately we don't want to dynamically generate imports at all, it's a hack.

[ajaybhargavb]

Ultimately we don't want to dynamically generate imports at all, it's a hack.

We still want to dynamically generate @using statements right?
Especially @using Microsoft.AspNetCore.Components

[rynowak]

that's not dynamic though.

My point is that if you're inside MyApp.Shared you don't need a using for MyApp.Shared nor do you for MyApp, those things are already in scope. So everything that works today with components will just work without any dynamic import generation.

[ajaybhargavb]

Current State:
All the components are already implemented as tag helpers and is associated with a TagHelperDescriptor.
DefaultRazorTagHelperBinderPhase filters these TagHelperDescriptors by visiting all the @addTagHelper and @removeTagHelper in the document and imports and binds accordingly.

Proposed implementation:

• Make DefaultRazorTagHelperBinderPhase visit @using directives. And use TagHelperDescriptor.Kind to differentiate components from regular tag helpers and use the appropriate directive.
• Make TagHelperBinder ignore tag helper prefix for components
• The descriptors created in ComponentTagHelperDescriptorProvider will include another rule to match full qualified tag names as well aka both <MyApp.Counter /> and <Counter />
• No tooling change is necessary. We should still get correct intellisense based on the presence of @usings

Note:

• We may need to add some errors for cases when components and tag helpers clash but that should probably be a separate issue.
• When I add a using it is always grayed out with "Using directive is unnecessary". We may want to eventually fix that by modifying the code gen.

Thoughts? Am I missing any requirements that can't be accomplished by this implementation?

[rynowak]

What about the special things that components have:

• bind
• ref
• onclick

Also there will be a change in the completion provider. The completion provider interacts with tag helpers.

Also consider aliases using Foo = Bar.Baz
Also consider the ambient namespace of the current document.

This seems like a good start.

[rynowak]

/cc @NTaylorMullen

[rynowak]

/cc @SteveSandersonMS @danroth27 any other comments or concerns?

[ajaybhargavb]

What about the special things that components have:
bind
ref
onclick

They all have catch-all rules which means we don't need to include any special fully qualified name rules. They will continue to work the same way without any changes,

Also there will be a change in the completion provider.

I assume you mean the TagHelperCompletionService and TagHelperFactsService. They still call into the same TagHelperBinder. That should still continue to work as long as we restrict weird tag helper/component clashes (We have some complex AllowedChildren/AllowedParent logic). I'll investigate this more to see what exact cases might be affected. @NTaylorMullen, can you think of anything of the top of your mind?

[NTaylorMullen]

@NTaylorMullen, can you think of anything of the top of your mind?

Given my current context of how components work, nope. As long as the same criteria for associating a Components html element => TagHelperDescriptor is the same or a subset of the existing system, we can ensure the descriptors we create for Components fulfill Components tag targeting requirements. At that point we can the filter which TagHelperDescriptors are available (by @using) for the document before attempting to translate HtmlElementSyntaxNodes into TagHelpers.

Basically, just re-iterating all you said in your comment (I think you're on the right track).

Also there will be a change in the completion provider.

Ya I think I also need some more context on this one, not quite sure what @rynowak is referring to.

[rynowak]

https://github.com/aspnet/AspNetCore-Tooling/blob/master/src/Razor/src/Microsoft.VisualStudio.Editor.Razor/DefaultTagHelperFactsService.cs#L111 (get on my level)

I only know about this because I already started to do this work: https://github.com/aspnet/AspNetCore-Tooling/tree/rynowak/lets-blow-it-up

---

So I think we largely agree on the implementation plan so there's no reason to delay starting on this.

I still think we need some more level of detail on the overall experience. Saying they will continue to work the same way isn't a super high level of precision 😆

[rynowak]

I'm going to start filling up the top post with experience notes. Just because I wrote it there shouldnt' be taken as gospel, this is just a starting point.

[rynowak]

@danroth27 @SteveSandersonMS please have a look at the top post

[SteveSandersonMS]

My main question is about this:

Components can bind with the short name

Is the idea that you can always bind with the short name, even if you haven't done the necessary @using directives to bring it into scope? If so that seems pretty odd; I'd hope to be able to disambiguate between same-named components using namespaces, like you do with C# classes. Without disambiguation, component libraries are going to be hazardous to combine.

I know it would be a breaking change to only bind to things that are in scope, but it would be a good change IMHO.

[rynowak]

Components can bind with the short name

No. The intention is that this is based on the C# name binding rules (usings, current namespace, and using aliases). I think the checklist format is making it unclear.

[ajaybhargavb]

Saying they will continue to work the same way isn't a super high level of precision

Fair enough 😄 . I'll do some more investigation and add more details here.