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.4k stars 10k forks source link

@addTagHelper should give better user feedback for failures #11780

Open NickCraver opened 5 years ago

NickCraver commented 5 years ago

I've lost a lot of time to tag helpers after reading the docs. While they have been improved by clarifying the include is the assembly and not the namespace, there's still more that should be relayed to the user.

For example, I have a tag helper that works fine in 2.x, but breaks in 3.0. I have no idea why it's not registering, but it's not. My includes are:

@addTagHelper *, Microsoft.AspNetCore.Mvc.TagHelpers
@addTagHelper *, MiniProfiler.AspNetCore.Mvc

Importantly, that looks like this: image

I have no idea if those names are valid names or what will happen at runtime. Can the tooling assist here? Note that I've added a complete garbage entry to illustrate the point. The app runs without issue with no warnings, complaints, or log entries...and those tag helpers just silently fail.

Compare this to any other experience in our world: if you have a using with a namespace that doesn't exist: boom. Type that's not there: boom. Argument: boom. Property: boom. We love booms! They save prod!

If I have a tag helper with an assembly it's unable to load, why isn't there an error?

The net result in my current case is I have a tag like this:

<mini-profiler position="@RenderPosition.Right" max-traces="5" />

And it renders as:

<mini-profiler position="Right" max-traces="5" />

...so, it silently fails. That should be a <script /> tag.

Can we please not be silent here? Can we make this go boom? Can we provide any information to the user at all about why their tag helpers aren't loading? If I explicitly tell Razor to load something and it can't, I'd like to know about it, because something is wrong. Or I should remove that call. The same as I would need to remove an @using if the namespace went away to successfully compile.

/cc @NTaylorMullen @rynowak

vincentparrett commented 5 years ago

I had issues with taghelpers in v3 #9766 with smidge, and it had to do with the references... they moved the cheese in v3 (moving stuff to sdk rather than packages) and referencing the v2 assemblies did not work. Smidge issue - the only way I could get past this was to create a netcoreapp3.0 package, netstandard 2.0 taghelpers don't just work with preview 4 or later.

Totally agree with the comment about the silent failures BTW, I have wasted soooo much time on this alone while migrating a large app from 4.7.2 mvc 5x to netcore (using 3.0 as our release timeframe is around the same time).

NickCraver commented 5 years ago

@vincentparrett Thanks a ton for that. Indeed it was my issue! I think this will trip a lot of people up in the upgrade (after all, we were advised to target netstandard2.0...). I'm in the same boat of being at a complete loss as to why this was failing.

poke commented 5 years ago

after all, we were advised to target netstandard2.0...

Except that pretty much nothing about ASP.NET Core 3 is available on netstandard2.0 (the few bits that are on standard are on 2.1).

And if you happen to use any MVC-related stuff in your library, you will have to move to netcoreapp3.0. That is the effect of ASP.NET Core moving to .NET Core only.

But I agree that this should be communicated more clearly, especially to library authors. There should probably be some docs specifically on authoring libraries, ideally also covering multi-targeting for different versions since that can be pretty complicated as well.

mkArtakMSFT commented 5 years ago

Thanks for contacting us, @NickCraver. What we can do here is to warn users when the addTagHelper doesn't discover any tag helpers.

NickCraver commented 5 years ago

That sounds great - any warning of that something is wrong would be a great first step over today. If we could long-term warn "hey, we aren't loading due to netstandard2.0" or such later that'd be an awesome next step, if possible. I think out-of-date builds for tag helpers will be an ongoing issue as long as versions have to be bumped by all MVC dependencies to move forward, effectively.

ghhv commented 5 years ago

I concur this is an issue. I copied a Core 2.2 project to test with Core 3.0 preview and the project name was changed in the process and of course tag helpers stopped working because the assembly name was now different to what was set in the ViewImports. Why the heck is it using an assembly/DLL name instead of a namespace like normal convention.

Changing a project name should not be breaking taghelpers.

Your suggestion @mkArtakMSFT doesn't really seem to fix this.

The convention for the built in tag helpers, @addTagHelper *, Microsoft.AspNetCore.Mvc.TagHelpers does look like a name space reference so why is it different for custom tag helpers?

poke commented 5 years ago

@ghhv

The convention for the built in tag helpers, @addTagHelper *, Microsoft.AspNetCore.Mvc.TagHelpers does look like a name space reference so why is it different for custom tag helpers?

Microsoft.AspNetCore.Mvc.TagHelpers happens to be an assembly that is part of MVC. So the built-in tag helpers aren't special here.

mkArtakMSFT commented 5 years ago

@ghhv, to add to what @poke said, we won't be changing this as that'll be a massive breaking change.

ghhv commented 5 years ago

Hmm, OK. It doesn't have to be a breaking change does it.. can it be an additional option? Where a namespace is used, it could reference the assembly name on compiling the way everything else does?

poke commented 5 years ago

@ghhv One issue I would see with a namespace-based alternative syntax would be that the current directives actually allow you to opt in and out of individual tag helpers. The syntax is @addTagHelper FQN, Assembly where the * is just a wildcard to add them all (which is a useful shortcut).

If there was a different syntax that just used a namespace, you wouldn't have control about individual tag helpers anymore.

I think the issue here is more that there is no perfect tooling to see when a tag helper is used vs. when it is not. The only signal for this is syntax highlighting which is very tool-specific and sometimes even fails.

Since tag helpers don't have some new unique and easy-discoverable syntax but instead integrate with normal HTML and may even disguise themselves as custom elements (which for all we know could run client-side!), I don't think that there is a good way to detect that someone wants to use a tag helper when that one won't be activated right now.

So outside of some diagnostics utility, e.g. something that just lists all active tag helper usages within Razor views, I don't think there is much we can do to improve this.

ghhv commented 5 years ago

OK, thanks Patrick. Tricky. I guess then it will be limited to a validation that the assembly exists as per Artak's suggestion - which will be a big step forward.

ghost commented 2 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.