toddams / RazorLight

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

Improve documentation #397

Open knuxbbs opened 3 years ago

knuxbbs commented 3 years ago

Is your feature request related to a problem? Please describe. As pointed in #396 , there is very little guidance to an user who wants to use your Razor views and pages as templates to another purposes.

Describe the solution you'd like First of all, seems like this library doesn't work with pre-compiled views (#35), which is the default output for ASP.NET Core applications. This should be highlighted in the repository's README, as well as the need to set your views as EmbeddedResources.

Second, the guidance about using RazorLightEngine as a injected service should be in main page, not in an outdated wiki page. This, in fact, should be the standard way to use the library, and should be further encouraged.

jzabroski commented 3 years ago

@knuxbbs Thanks for your constructive feedback. After I took over this project from toddams, I did not know all the legacy stuff hanging around.

First of all, seems like this library doesn't work with pre-compiled views (#35), which is the default output for ASP.NET Core applications.

Technically, there is a Razorlight.Precompile.csproj. I just don't pack it because I don't want to support it, because I don't have the time to add test coverage (yet). And, with precompilation resulting in a standalone assembly, if people are not shipping the dll as a nuget package with all the transitive <dependency id="whatever" /> values, you lose context when you upgrade from .NET Core 3 to .NET 5 that the assembly was originally targeting .NET Core 3. This particular problem is what worries me from a support perspective, and I have not had time to research how to "get it right". I never saw the benefit for my clients to use this feature, but my current clients don't manage massively busy websites like my clients 6 years ago. So it's really a cost-benefit thing for me.

In my opinion, RazorLight.Precompile should:

  1. Add the following:
    <PackAsTool>true</PackAsTool>
    <ToolCommandName>precompileviews</ToolCommandName>
  2. Add test coverage to guarantee resulting dll can be linked and loaded.
  3. (Maybe) add test coverage that validates the nuget package. I don't think there are automated tools for testing nuget package generation, and most maintainers do a-b before/after tests using Nuget Package Explorer to verify they didn't bonk something.

However, without step 2, step 1 is kind of pointless because it will create a lot of frustrated users wondering why something doesn't work. If you are interested in digging into this, that would be super helpful.

as well as the need to set your views as EmbeddedResources.

That is not a requirement, but the README.md does explicitly steer people towards using AsEmbeddedResourceProject() via the Quickstart section. I guess a complete sample is necessary for some people, especially if you're less familiar with the new auto-globbing rules in .NET SDK csproj file format.

Can you give me some wording you have in mind to clarify things?

Second, the guidance about using RazorLightEngine as a injected service should be in main page

Well, this is not strictly necessary. RazorLightEngine just wraps the callback for injected services. But yes, if you don't implement the callback options yourself using some alternative to RazorLightEngine, injected services won't work.

, not in an outdated wiki page.

Agree, page is out of date, should be marked as obsolete and only useful for RazorLight 1.x.

jzabroski commented 3 years ago

@maxbanas I wonder if we can detect when the stack trace can't decode @inject and point to a Wiki page with info? The most common reason this occurs is because people bypass RazorLightEngine and/or RazorLightEngineBuilder (which registers the inject callback handler). This usually occurs when people render strings as templates, as opposed to embedded resource .cshtml files or file system .cshtml files.

maxbanas commented 3 years ago

@jzabroski I've only compiled/rendered templates using the "normal" API (RazorLightEngine via the builder), so I'm not very familiar with the other services/components in a standalone context. Do you know what entry point people would use when bypassing these?

jzabroski commented 3 years ago

@maxbanas People use IEngineHandler directly, since its a thin wrapper around the Razor compiler. The RazorLightEngine is the project system, and value added callbacks like @inject, dynamic templates, etc. that EngineHandler itself doesnt know or care about.

jzabroski commented 3 years ago

See #317 where I just closed a bug which really only occurred because I was trying to handle the scenario where people call IEngineHandler directly and wonder why AddRazorLight doesnt work for them.

knuxbbs commented 3 years ago

A documentation page like this would be very good: https://antaris.github.io/RazorEngine/AboutRazor.html

jzabroski commented 3 years ago

I'm not clear. I'm not looking to rewrite Microsoft documents on what Razor is. What do you find very good about this page?

maxbanas commented 3 years ago

@jzabroski

I wonder if we can detect when the stack trace can't decode @inject and point to a Wiki page with info? The most common reason this occurs is because people bypass RazorLightEngine and/or RazorLightEngineBuilder (which registers the inject callback handler). This usually occurs when people render strings as templates, as opposed to embedded resource .cshtml files or file system .cshtml files.

When I create the engine handler in a way that doesn't add the inject pre-render callback, I don't see any exception being thrown until I try to use the object that was supposed to be injected. In that case I just get a NullReferenceException. It looks to me like @inject compiles regardless of if a callback is registered. But if the callback isn't registered, that property never gets set. Does that sound right?

If that's the case, and people are having issues, we could grab all the runtime properties with inject attributes (the same way PropertyInjector does) and check to make sure they're not null before calling page.ExecuteAsync() or right after ExecutePageCallbacks().

If the inject callback was it's own property (i.e. not in IList<Action<ITemplatePage>>), it might be easier/faster to just require that the callback is set if the template has any injected properties, but, that's not the case.

jbaumbach commented 3 years ago

@jzabroski thank you so much for your work on this. I'm a little confused about how to install this component, perhaps you could give some tips and I could help update your docs.

Project: .Net Framework 4.7.1 (eventually will be .Net Core 5, but we're updating all our libs first)

  1. When using Nuget to install RazorLight - there are a lot of variations for "RazorLight" by toddams in the list - I presume the main one (with 2.5M downloads) is the "right" one? But it's version 1.1.0, from 2017, so that doesn't seem right.
  2. The docs page says to install with command Install-Package RazorLight -Version 2.0.0-rc.3, but I get a "Package not found" error.
  3. Trying one version back, Install-Package RazorLight -Version 2.0.0-rc.2, DOES seem like it worked, but after the installation there is no DLL or reference to anything called "RazorLight" - its almost like something is missing from the package?

Any guidance would be hugely appreciated.

jzabroski commented 3 years ago

I need to push out rc3. Really I need to setup Github actions. It's just sitting on my desktop and I have not uploaded the nuget packages. I'll do it tomorrow. (I run a company and another FLOSs project and I've gotten a lot of PRs to review, so I'm a bit behind. )

jbaumbach commented 3 years ago

Great thanks for the response @jzabroski ! Let me know if there's anything I can do to help. Our current lib is .Net framework only. Razor templates are a critical part of our update to .Net core - without a working solution the whole upgrade project is a no-go. :(

jzabroski commented 3 years ago

@jbaumbach If you run into issues with using it in .NET Framework, just provide me with a repro. If you're not familiar with Git, just download Github Desktop and fork toddams/RazorLight from within GitHub Desktop. In the latest version of GitHub Desktop, forking it from within GitHub desktop correctly sets up the remote origin and upstreams for you, but be careful not to dismiss the dialogs.

I agree, I have plenty of customers still on .NET Framework and so I would like to keep RazorLight a .NET Framework project as long as possible. Happy to compare notes on how to plan an upgrade via email.

jzabroski commented 3 years ago

@knuxbbs I fixed strange xmldocs issues so that should be some progress towards this task of "Improve documentation".