Add support for ViewComponentTagHelpers.
#937
Conversation
| @@ -19,46 +21,82 @@ public DefaultTagHelperResolver(bool designTime) | |||
| public override IReadOnlyList<TagHelperDescriptor> GetTagHelpers(Compilation compilation) | |||
| { | |||
| var results = new List<TagHelperDescriptor>(); | |||
| var errors = new ErrorSink(); | |||
There was a problem hiding this comment.
We don't flow this error sink out today. It'd probably be valuable to do that to inform tooling on bits that go wrong.
There was a problem hiding this comment.
Yep. We need to do a refactor on THD or least evaluate the design. Let's plan on doing this today.
| { | ||
| try | ||
| { | ||
| var descriptor = factory.CreateDescriptor(type); |
There was a problem hiding this comment.
These bits will be replaced by MVC invocations. Didn't feel that it was worth the time to convert each exception clause into an errorSink.OnError call/flow the ErrorSink throughout the factory.
| return results; | ||
| private static void VisitViewComponents(Compilation compilation, List<TagHelperDescriptor> results, ErrorSink errors) | ||
| { | ||
| var @interface = compilation.GetTypeByMetadataName(ViewComponentTypes.ViewComponentAttribute); |
There was a problem hiding this comment.
should we no-op if this type isn't defined?
There was a problem hiding this comment.
Have you given any thought to how we'll determine if the user is on MVC >= 1.1
There was a problem hiding this comment.
should we no-op if this type isn't defined?
If there's no attribute the classes could be defined in a class library using the ViewComponent suffix couldn't they?
Have you given any thought to how we'll determine if the user is on MVC >= 1.1
Great question. Hmm, I could look for public types added in 1.1 to check for its existence. What do you think?
There was a problem hiding this comment.
Hmm, depending on the approach for detecting 1.1 MVC maybe nooping would be the right approach
| return results; | ||
| private static void VisitViewComponents(Compilation compilation, List<TagHelperDescriptor> results, ErrorSink errors) | ||
| { | ||
| var @interface = compilation.GetTypeByMetadataName(ViewComponentTypes.ViewComponentAttribute); |
There was a problem hiding this comment.
Have you given any thought to how we'll determine if the user is on MVC >= 1.1
| return false; | ||
| } | ||
|
|
||
| return symbol.Name.EndsWith(ViewComponentTypes.ViewComponentSuffix) || |
There was a problem hiding this comment.
There's also NonViewComponent attribute to consider, which makes the entire hierarchy not a VC.
There was a problem hiding this comment.
Oh shoot, good catch. I missed that bit.
| // Will invoke synchronously. Method must not return void, Task or Task<T>. | ||
| if (returnType.SpecialType == SpecialType.System_Void) | ||
| { | ||
| throw new InvalidOperationException(ViewComponentResources.FormatViewComponent_SyncMethod_ShouldReturnValue( |
There was a problem hiding this comment.
Are these the errors the VC would report at runtime when called?
- Hardcoded `ViewComponent` discovery. - Hardcoded `ViewComponentTagHelperDescriptor` creation. - Added test to validate that ViewComponents are discovered and transitioned into TagHelpers properly. - Avoided adding a reference to MVC to prevent circular references. This resulted in custom marker attributes to represent `ViewComponent`s. Also made a lot of use of `ViewComponent` conventions (ending in "ViewComponent"). #932
NTaylorMullen
force-pushed
the
nimullen/vctaghelpers.932
branch
from
0322322
to
6e64785
Compare
ViewComponentdiscovery.ViewComponentTagHelperDescriptorcreation.ViewComponents. Also made a lot of use ofViewComponentconventions (ending in "ViewComponent").#932