toddams / RazorLight

Template engine based on Microsoft's Razor parsing engine for .NET Core
Apache License 2.0
1.51k stars 260 forks source link

Add debug mode and include known template keys in TemplateNotFoundException #398

Closed maxbanas closed 3 years ago

maxbanas commented 3 years ago

Is your feature request related to a problem? Please describe. It can be tricky for new users to figure out the correct template key to use (#378, #385, etc.)

Describe the solution you'd like Create a "debug mode", that when enabled, adds known template keys to the TemplateNotFoundException.

Additional context Originally discussed here.

jzabroski commented 3 years ago

Great. Really excited for this feature. Think it will be super helpful.

As a separate topic, the other major pain point people have is troubleshooting why assemblies don't load correctly. I have a new idea on that based on MetadataLoadContext, which is the side-effect free dual to AssemblyLoadContext (it just loads the assembly metadata as data as opposed to the actual assembly as a dll).

There are a few open source projects for pre-.NET 5 that can do this as well, but they use Mono.Cecil to read the metadata. MetadataLoadContext is the newer, official API from Microsoft.

maxbanas commented 3 years ago

Here's the work in progress. A brief summary/things to consider:

If you think this is close enough to being complete, I can open a pull request and we can sort out the details there.

jzabroski commented 3 years ago
  • RazorLightProject now has a method, GetKnownKeysAsync. I figured it would be best for this method to be virtual so we don't break anyone's custom project, but what should the default implementation be?

Empty enumerable for now. Makes reasoning about control flow easier.

One review item: https://github.com/toddams/RazorLight/compare/master...maxbanas:feature/398#diff-faaf482eb73b368fd3c2aeb32459b8cf25da987a137c77fff3145e741591b520R298

I believe you have a typo in one of the propNames, perhaps due to using auto-complete.

As far as improvements, I think this is good enough to release pretty much. The only incremental improvement I could suggest is doing something where we compute the closest matching template key. Or handling the special case where an EmbeddedResource Project has.... zero embedded resources found? (In this case, it would make sense to maybe include a link to Microsoft Docs on adding cshtml files as embedded resources, noting the default auto-globbing rule for cshtml files is Content https://docs.microsoft.com/en-us/aspnet/core/razor-pages/sdk?view=aspnetcore-5.0#overview)

Side note: I personally probably put more thought into a nice EmbeddedResource Project since that is what I personally use and care about. I don't like loading stuff from external resources, whether it is file system or database, because then I have to worry about managing that external resource after I ship and deploy. I rather just do hotfixes and re-run everything I do through regression tests. View code is code, and code should be tested.

The above side note is really just to say, if you think a similar point can be made for the file system or EFCore project, then maybe implement that.

In the case of perhaps having a generic no templates found at all exception, we might want the error message to mention this can happen if people render raw strings (as there is no template cache IIRC).

Thoughts?

And, yes, after you fix typo, submit PR and we can move discussion there.

Thanks again for your help!