[WIP] ASP.NET Core Razor engine - do not merge

[daveaglick]

DO NOT MERGE

Relates to #2521. This is an initial cut at getting the Razor engine in .NET Core working on Nancy. There's still quite a bit left to do, but I wanted to get any early feedback you might have given the scale of changes and the importance of Razor parsing in Nancy.

Some notes:

• Host: I used the full ASP.NET Core MVC Razor host instead of trying to build up a new one from scratch as the current Nancy Razor view engine does. There are examples of lightweight Razor Core hosts (see https://github.com/aspnet/Entropy/tree/dev/samples/Mvc.RenderViewToString), but using the full MVC host should provide the most compatibility and allows Nancy to use many of the same abstractions that full ASP.NET MVC does.
• DI: I am creating an independent container within the Nancy Razor view engine to handle services. We could register all the Razor/MVC services with TinyIoC, but we'd have to provide an IServiceProvider implementation for TinyIoC that the Razor host can consume. This seemed easier, at least for now.
• Assembly Loading: I am currently loading all runtime assemblies into the Razor compilation context. There are so many required assemblies in the new version of Razor that attempting to enumerate them all by hand for loading was going to be tough. This can be revisited in the future if we need to.
• I removed the Nancy.ViewEngines.Razor.BuildProviders project. I'm not sure what it was for? Will we need it for the new implementation? [Update: don't think we'll need this - since we're using the full MVC host this time around, the VS Intellisense should do a good job with the views given they're derived from the same class]

Still to do:

• [ ] Antiforgery token support
• [x] Verify view location finding in a variety of cases
• [x] Error page support
• [x] Re-enable and port (if needed) the test projects
• [ ] Integrate Razor and Nancy logging
• [ ] Provide wrappers around Nancy context for ASP.NET MVC core classes (like HttpContext)
• [ ] Add Nancy context classes to the base Razor page
• [ ] XML code documentation
• [ ] Update the Nancy Wiki
• [x] Build housekeeping (what are the *.MSBuild projects? currently targeting netstandard 1.6, does it need to be multitarget?)

Anyway, I'll continue working on this and pushing to the PR branch. I'd welcome any comments, suggestions, direction if/when anyone gets a chance to look at it. Also, I've turned on edits from maintainers, feel free to push some code!

[xt0rted]

Nancy.ViewEngines.Razor.BuildProviders was to help with VS tooling, though it was still pretty lacking (and even worse with ReSharper)

[daveaglick]

What's the deal here: https://github.com/NancyFx/Nancy/blob/master/test/Nancy.Tests.Functional/Tests/TracingSmokeTests.cs

Noticed it while getting all the other tests in this project to pass. It passes now, but as far as I can tell it's functionally identical to the test in ViewBagTests.

[khellang]

This PR pulls in quite a few dependencies...

Microsoft.AspNetCore.Antiforgery
Microsoft.AspNetCore.Authorization
Microsoft.AspNetCore.Cryptography.Internal
Microsoft.AspNetCore.DataProtection
Microsoft.AspNetCore.DataProtection.Abstractions
Microsoft.AspNetCore.Diagnostics.Abstractions
Microsoft.AspNetCore.Hosting.Abstractions
Microsoft.AspNetCore.Hosting.Server.Abstractions
Microsoft.AspNetCore.Html.Abstractions
Microsoft.AspNetCore.Http
Microsoft.AspNetCore.Http.Abstractions
Microsoft.AspNetCore.Http.Extensions
Microsoft.AspNetCore.Http.Features
Microsoft.AspNetCore.JsonPatch
Microsoft.AspNetCore.Mvc.Abstractions
Microsoft.AspNetCore.Mvc.Core
Microsoft.AspNetCore.Mvc.DataAnnotations
Microsoft.AspNetCore.Mvc.Formatters.Json
Microsoft.AspNetCore.Mvc.Razor
Microsoft.AspNetCore.Mvc.Razor.Host
Microsoft.AspNetCore.Mvc.ViewFeatures
Microsoft.AspNetCore.Razor
Microsoft.AspNetCore.Razor.Runtime
Microsoft.AspNetCore.Routing
Microsoft.AspNetCore.Routing.Abstractions
Microsoft.AspNetCore.WebUtilities
Microsoft.CodeAnalysis.Analyzers
Microsoft.CodeAnalysis.Common
Microsoft.CodeAnalysis.CSharp
Microsoft.CSharp
Microsoft.DotNet.InternalAbstractions
Microsoft.Extensions.Caching.Abstractions
Microsoft.Extensions.Caching.Memory
Microsoft.Extensions.Configuration
Microsoft.Extensions.Configuration.Abstractions
Microsoft.Extensions.Configuration.Binder
Microsoft.Extensions.Configuration.EnvironmentVariables
Microsoft.Extensions.Configuration.FileExtensions
Microsoft.Extensions.Configuration.Json
Microsoft.Extensions.DependencyInjection
Microsoft.Extensions.DependencyInjection.Abstractions
Microsoft.Extensions.DependencyModel
Microsoft.Extensions.FileProviders.Abstractions
Microsoft.Extensions.FileProviders.Composite
Microsoft.Extensions.FileProviders.Physical
Microsoft.Extensions.FileSystemGlobbing
Microsoft.Extensions.Localization
Microsoft.Extensions.Localization.Abstractions
Microsoft.Extensions.Logging.Abstractions
Microsoft.Extensions.ObjectPool
Microsoft.Extensions.Options
Microsoft.Extensions.Options.ConfigurationExtensions
Microsoft.Extensions.PlatformAbstractions
Microsoft.Extensions.Primitives
Microsoft.Extensions.WebEncoders
Microsoft.Net.Http.Headers
Newtonsoft.Json
System.Buffers
System.Collections
System.Collections.Concurrent
System.Collections.Immutable
System.ComponentModel
System.ComponentModel.Primitives
System.ComponentModel.TypeConverter
System.Diagnostics.Contracts
System.Diagnostics.Debug
System.Diagnostics.DiagnosticSource
System.Globalization
System.IO
System.Linq
System.Linq.Expressions
System.Reflection
System.Reflection.Extensions
System.Reflection.Metadata
System.Resources.ResourceManager
System.Runtime
System.Runtime.Extensions
System.Runtime.InteropServices
System.Runtime.Serialization.Primitives
System.Text.Encoding
System.Text.Encoding.Extensions
System.Text.Encodings.Web
System.Threading
System.Threading.Tasks

This include more or less the entire MVC framework 😱 Is this really needed? I think this could be a show-stopper 😢

[daveaglick]

Yeah, it's unfortunate. That's a result of my strategy to sit on top of the MVC host instead of rolling a custom one. The MVC Razor bits are tightly coupled to the rest of the MVC framework. Most of that is from the dependency chain - if you look at the project.json you'll see that the direct dependencies are pretty minimal.

It's a tough one...

Doing it this way does bring in a bunch of dependencies (some of which are quite small, as is the trend on that team). On the other hand, it gives us a good head-start on expected functionality like sections, layouts, partials, view starts, view components, tag helpers, etc. A lot of that would have to be reimplemented in whole or in part if building a new Razor host from scratch, and without the benefit of the exhaustive testing MVC gets. It also gives us a bit more compatibility with the way VS treats Razor views as well as with third-party components built for the stock MVC Razor host (since we're just inheriting from those classes).

I do understand the concern though. To me, it seems like the trade off mostly comes down to package size (primarily in dependencies) and flexibility of the host vs. compatibility with MVC and implementation time-to-market.

[khellang]

The MVC Razor bits are tightly coupled to the rest of the MVC framework.

Yeah. This is the main problem. I remember @thecodejunkie talked with the team (@NTaylorMullen?) quite a while ago about this. I don't know what the conclusion, if any, of those conversations were, but it looks like the coupling is still pretty tight. I also remember someone (@shanselman?) saying that MS were committed to making Razor as easy as possible for other web frameworks to use, including within Visual Studio. Not sure if this is the final result of that, or if that goal was dropped/forgotten along the way. Either way, we might have to pick up the conversation again and see if there's anything that can be done for a 2.0 version or something.

Doing it this way does bring in a bunch of dependencies (some of which are quite small, as is the trend on that team). On the other hand, it gives us a good head-start on expected functionality like sections, layouts, partials, view starts, view components, tag helpers, etc. A lot of that would have to be reimplemented in whole or in part if building a new Razor host from scratch, and without the benefit of the exhaustive testing MVC gets. It also gives us a bit more compatibility with the way VS treats Razor views as well as with third-party components built for the stock MVC Razor host (since we're just inheriting from those classes).

I bet the ship has sailed long ago, but it would be nice if there was a way to factor stuff, so that you wouldn't need most of the Microsoft.AspNetCore.Mvc.* and Microsoft.Extensons.* packages, but still be able to leverage most of the nice Razor functionality. I realize that we could drop a lot of these dependencies today, but that would also mean we have to drop down to a lower level, lose a lot of the "expected" Razor functionality and end up with Yet Another Razor Dialect™.

What irks me the most about this, is having two sets of HTTP abstractions (and full web frameworks, including routing) being used alongside each other, in the same application, without necessarily needing it. It's not hard to envision how that could be super confusing for people when interacting with one or the other in different parts of the same application. Not to mention the runtime overhead.

Not really sure where to go from here. Let's have @NancyFx/most-valued-minions, @thecodejunkie and @grumpydev weigh in...

[thecodejunkie]

I need to look at the changes a bit more carefully before I can say something definitive. However my personal stand on the whole Razor thing is that the best thing, for Nancy, would be for us to not maintain Yet Another Razor Dialect™ as it is quite tedious work with means undocumented code and behavior. It would be awesome to be on par with the expected (as seen in the ASP.NET stack) Razor features, that's for sure.

That said I think we need to have a look and a chat with people like @NTaylorMullen and @shanselman to see if there is anything we can do to avoid bringing in the entire kitchen sink to make it a reality. I agree that it feels a bit icky if we have to bring in an entire parallel framework just to use the Razor bits.. maybe add @davidfowl as well regarding the HTTP abstractions

[davidfowl]

This requires a bunch of work on our side to move that infrastructure into something shared. We also want tooling to work well. Are there any nancy specific razor features? What contexts end up in the views themselves etc.

/cc @rynowak @DamianEdwards

[thecodejunkie]

@davidfowl there's no Razor features per say.. we have our own view location convention stuff etc (which is outside Razor) and our own RazorView base class which adds some stuff like dynamic text resources etc

[khellang]

Are there any nancy specific razor features?

I don't think so. It's mostly about using the NancyContext instead of HttpContext etc. RazorPage<TContext>? :smile:

This requires a bunch of work on our side to move that infrastructure into something shared.

Is this something you think is worth the time/work? I don't think this is about Nancy only. There's a lot of projects, like Wyam etc., that would benefit from having a lot of the MVC (or web)-specific functionality separated out.

[thecodejunkie]

@davidfowl the text stuff is things like this https://github.com/NancyFx/Nancy/blob/master/samples/Nancy.Demo.Razor.Localization/Views/Index.cshtml#L8 which lets you use dynamics and then behind the scenes you can plug in stuff that resolves strings (localized) from various resources

[grumpydev]

We need a way to expose some nancy types inside the razor views (context etc.), need to be able to inject assemblies into the razor context, and we need to be able to hook into "virtual view location" to search for views (i.e. "find me ~/blah.cshtml" not "find me c:\inetpub\wwwroot\blah.cshtml", as we don't always store views on the filesystem) - other than that, off the top of my head I can't think of a great deal else we need, the rest is just removal of "baggage".

[daveaglick]

I thought about this quite a bit last night, and TBH I'm having a hard time getting too worked up.

Yes, the number of dependencies is unfortunate. I took a look at the overall size of the packages. In aggregate we're talking about 220 MB, but the vast majority of that is coming from the System.* packages which aren't really due directly to MVC and are included by the main Nancy library anyway. Roslyn takes up a decent chunk too and we'd need to include that regardless. So size-wise, the MVC dependencies aren't contributing much. Number-wise, lots of small packages seems to be the way everything is moving in Core, so I'm not sure that's a big deal either.

The benefit is that we do have the ability to build on top of the MVC Razor host today. Because of the way it's so heavily service based, it's easy enough to swap out functionality with derived or fully-custom implementations for most things. The biggest disconnect appears to be around view location finding and providing "file" content, but that can be (and has been) worked around.

Long-term I'm all for continuing to push for greater abstraction. It seems like what's needed is something between the full MVC host and a bare-bones Razor host like what's in Entropy. When a user hears "Razor" I'd venture almost all of them associate that with partials, layouts, HtmlHelper, and all the other stuff that the MVC host provides.

Near-term my own preference would be to ship a Razor host for Nancy built on top of the MVC Core one for now. That gets Razor support for Nancy under Core today. If the alternative is waiting for an abstraction to become available, I'm not sure that's a great story for Nancy: "you can use this shiny new version of Nancy on Core, but one of the most popular view engines isn't available". Either way, I'll finish up the work - I need it for something else I'm working on that sits on Core. If it's decided not to pull this in (no hard feelings!) I'll probably release it as an "unofficial" view engine myself until an official Razor view engine for Nancy 2.x is available.

As for the more general talk of building out a middle-ground Razor abstraction in ASP.NET - is there an existing issue where that should be taken? Within this PR probably isn't the best place for that discussion...

[daveaglick]

I'm working on the last few failing tests right now, then I'll start focusing on polish. I'll probably have a couple questions about the inner-workings of Nancy to account for differences between the old Razor engine and this implementation.

For example, in the base Razor view in NancyRazorViewBase.cs the old engine has an implementation for WriteAttribute that uses custom logic to treat DynamicDictionaryValue objects as a special case and output an empty string if they don't contain a value. By contrast (and without this special check) the DynamicDictionaryValue class actually returns it's own type name if there's no value (https://github.com/NancyFx/Nancy/blob/704530c7d8a205234738262da5f6f08e360d7c7c/src/Nancy/DynamicDictionaryValue.cs#L356):

public override string ToString()
{
    return this.value == null ? base.ToString() : Convert.ToString(this.value);
}

Wondering what the reasoning is here? I can't think of a case where I'd want a .ToString() call on a non-existing property of a dynamic object to return "Nancy.DynamicDictionaryValue". Wondering if this implementation should be changed to operate similar to the logic in the old Razor engine, or does something actually rely on it returning a type name here?

Wondering if this would be better:

public override string ToString()
{
    return this.value == null ? string.Empty : Convert.ToString(this.value);
}

[rynowak]

FYI we will be doing something much better with the host for 1.2.0 - I know that's a long way out from now, but you're hitting problems we know about and are working to address.

[daveaglick]

All existing tests now pass locally and on the AppVeyor build. It looks like TeamCity is having trouble restoring packages and Travis is stuck on a test in unrelated code.

This basically completes the Core-compatible Razor engine. I still have some code style cleanup, code commenting, and a few other things (see checklist above) but for the most part this should be good for review. It also satisfies my own need for Razor support in a Core project.

You'll obviously need to decide if you want to pull this in, re-implement using a more lightweight approach (but not MVC Core compatible), or wait until the improvements in the host that @rynowak mentioned are available. No hard feelings in any of these cases :smile:

[mylemans]

Any chance of getting a 'working hello world' example that showcases this? :)

[thecodejunkie]

@daveaglick did you get anywhere with this? Still very much interested in this and maybe we can create a "task force" for it on our Slack to enable a quicker feedback cycle?

[daveaglick]

@thecodejunkie Yeah, got pretty far. It passes all tests and works well, AFAIK. Not quite production ready since there's some cleanup to do with regard to exposing Nancy objects in the base page class, doc comments, logging, etc. I wouldn't anticipate any of that taking too long.

I mainly pulled back to give you guys some time to decide conceptually if this is something you want to bring in. Didn't want to take a bunch of time on polish if the idea was DOA due to the issues discussed above.

[daveaglick]

FYI - I haven't abandoned this PR. Partly waiting for guidance, but also been off on a big spike for another project. I fully intend to come back, rebase, and finish all the polishing one way or the other.

[thecodejunkie]

@davidfowl @rynowak @DamianEdwards are the changes "in the work", "planned" or "we've casually talked about how cool it would be to do" ? =)

[davidfowl]

@thecodejunkie they are inflight right now but won't be done in 2016. It's a pretty large refactoring https://github.com/aspnet/Razor/issues?q=is%3Aopen+is%3Aissue+label%3AEvolution

[rynowak]

@thecodejunkie - this is under active development. Example: https://github.com/aspnet/Razor/pull/869

We are probably 2-3 weeks away from a working E2E for something usable. We'd be very interested in your feedback when we get there though 👍

[daveaglick]

@rynowak @davidfowl That timeline is considerably sooner than I was expecting. Given a bunch of changes are in the pipeline, do you think it's worth finishing this out as-is or waiting for a more extensible Razor host in MVC and starting again from there?

[rynowak]

Just to clarify 2-3 weeks is the timeline for something that works 😆 - not the timeline for us to ship it. There won't be any immediate tooling support and there will be bugs and gaps.

I would advise you to think about how much code fits into each bucket.

The code that calls Razor won't really change substantially. Sure there will be different APIs but fundamentally files in and C# source code out. The code that extends Razor will change a lot and hopefully should be much simpler.

[daveaglick]

@rynowak Interesting, thanks (and your point about shipping vs. compiling is well taken).

Since this PR is almost entirely about calling Razor, it'll be interesting to see how much it does change once some of these enhancements are in place.

The main problem right now is that hosting Razor without re-implementing support for things like partial views requires bringing in a ton of dependencies because you have to essentially use the MVC Razor host. Perhaps with these changes, the alternate approach of not using the MVC Razor host but instead using a custom Razor host and adding modular support for partial views, tag helpers, etc. becomes doable. Does that get the gist of it? Still trying to understand how what you folks are working on relates to this type of third-party Razor use/hosting...

[rynowak]

Yep, the MVC RazorHost has an awful lot of policy in it. If you guys are interested in having this working the shortest path will be to use MVC for now. It's not great and that's why we're addressing it.

This design of the new system is modular. If you end up building your own RazorHost you'll see a lot of it melt away.

[daveaglick]

If you end up building your own RazorHost you'll see a lot of it melt away.

:thumbsup: :100: :rainbow:

(this isn't the only project I work on that uses this approach for bringing in the MVC Razor host, so that's very welcome news - looking forward to replacing all of them with a lighter-weight host down the road)

[rynowak]

To make this a little more concrete: https://github.com/aspnet/Razor/blob/dev/src/Microsoft.AspNetCore.Razor.Evolution/RazorEngine.cs

RazorEngine runs as series of phases that either create a new representation of the data (source -> syntax tree -> IR -> CSharp) or run a transformation on one of these representations.

We're also making the set of directives extensible https://github.com/aspnet/Razor/blob/dev/src/Microsoft.AspNetCore.Razor.Evolution/DirectiveDescriptor.cs . You'll implement a directive by creating a descriptor that describes it and then implementing an IR pass to turn it into code.

Nothing that we're doing here should require you to replace the host/engine. Everything in this system can be composed. For instance we ran into the issue when building RazorPages that you can either have a host for MVC pages or a host for MVC views. We're going to solve this as well.

[xt0rted]

I've been holding off on v2 until it's final so I've been exploring back porting some of the razor updates to v1.4 such as using Roslyn. I've also been trying to get view precompilation working with a modified version of StackExchange/StackExchange.Precompilation. I'd really like to put my efforts into v2 at this point so I can start moving the rest of my project over instead of into dead end code. Most of the links to the aspnet/Razor repo no longer work, and the labels no longer exist. What's the current status on all of this?

[thecodejunkie]

@xt0rted funny you should ask. I browsed the https://github.com/aspnet/Razor repo a bit last night, asking myself the same question. I think they've now created a stand along version of the language and parser with Microsoft.AspNetCore.Razor.Language. They have a sample generator at https://github.com/aspnet/Razor/blob/dev/src/RazorPageGenerator/Program.cs that seem to suggest on how you use it stand alone.

I've yet to dig deep enough to understand where the boundary between the razor parser and the ASP.NET MVC Razor engine starts, i.e the point where it becomes ASP.NET specific and start to make use of their HTTP abstractions. We'd need to get in just below that, as we have our own HTTP abstractions and would like to avoid creating some sort of mapping layer.

We need to understand what features we are missing out on when we step in on that layer though? Are things like TagHelpers ASP.NET specific or are they tied to the engine and could be reused by our engine? Documentation is a bit sparse on the code base, so I am hoping that @rynowak , whom work on the engine for Microsoft, wouldn't mind sharing his insights and provide a bit of guidance on getting started.

In essens what we want/need is

1. Use the cross-platform implementation of the razor parser to create our own engine without taking any dependencies on Microsoft.AspNetCore.Html.Abstractions
2. Understand what functionality that will be available to us, and what will be lost, if we step in at the level below the Microsoft.AspNetCore.Html.Abstractions (i.e will things like master pages, taghelpers and so on, be supported out of the box)
3. Create a general understanding of the moving parts of the new razor parser, i.e what interfaces are important to understand and what are their purpose?

So if @rynowak (or anyone else on the team) would be willing to give us a boot camp on these things (people like @davidfowl has earlier expressed interest in, this thread, to see that happen) then we could start looking at creating a new IViewEngine implementation, for Nancy, that's based on the latest Razor bits.

I'm not sure if @daveaglick still would be interested in working with us to make this happen, but I'd be more than happy to hack on this with him (and anyone else that want's to help). It's an important milestone for a 2.0 release

Also ping to any @NancyFx/most-valued-minions that is interested in this