-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[WIP] ASP.NET Core Razor engine - do not merge #2573
base: master
Are you sure you want to change the base?
Conversation
|
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 |
…al views, about half of Razor tests now pass
This PR pulls in quite a few dependencies...
This include more or less the entire MVC framework 😱 Is this really needed? I think this could be a show-stopper 😢 |
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 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. |
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.
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... |
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 |
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. |
@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 |
I don't think so. It's mostly about using the
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. |
@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 |
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". |
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 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, 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... |
…r request, all but three tests now pass
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 Nancy/src/Nancy/DynamicDictionaryValue.cs Line 356 in 704530c
Wondering what the reasoning is here? I can't think of a case where I'd want a Wondering if this would be better:
|
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. |
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 😄 |
Any chance of getting a 'working hello world' example that showcases this? :) |
@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? |
@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. |
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. |
@davidfowl @rynowak @DamianEdwards are the changes "in the work", "planned" or "we've casually talked about how cool it would be to do" ? =) |
@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 |
@thecodejunkie - this is under active development. Example: aspnet/Razor#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 👍 |
@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? |
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. |
@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... |
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. |
👍 💯 🌈 (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) |
To make this a little more concrete: https://github.com/aspnet/Razor/blob/dev/src/Microsoft.AspNetCore.Razor.Evolution/RazorEngine.cs
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. |
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? |
@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 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
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 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 |
Oh and the gist of creating a new IViewEngine implementation is
Our current engine uses it's own view base class, called NancyRazorViewBase (similar to Microsoft.Extensions.RazorViews.BaseView by Microsoft) which exposes some Nancy specific feature, to the views, such as
All in all, Nancy is a pretty thin wrapper around Razor. The reason our current Razor view engines, that's based around |
@thecodejunkie FWIW, I am absolutely still interested in working this. I eventually gave up on this PR when it became clear the new Razor Evolution (or I guess now it's Razor Language) stuff was moving forward and was going to obsolete the technique used here. Figured we'd leave the PR open though since it has so much good discussion, at least until the new bits are ready. Your comments above are on point - we need to both "light up" expected Razor functionality (which I understand is all modular and mostly opt-in in Razor Langauge), as well as inject expected Nancy functionality and objects. I think Razor Language is still baking, but I'm not totally sure. Whenever it does get released I'll circle back and we can begin work. |
@daveaglick While it might still be baking, I doubt we'll see massive breaking changes att this stage. Hopefully @rynowak can give us a guided tour of the moving parts in the new bits, so we know a bit more about the various extension points and classes. How about we start spiking and exploring? 😄 |
Hi everyone, I'm paying attention here, just a little busy with finishing up 2.0.0 right now. We still have a lot of polish to do to the API surface and some of the new features like extensible directives. I'll pop in later tonite and try to answer most of the questions that have been asked here so far. As a starting point take a look at https://github.com/aspnet/Razor/blob/dev/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/RazorExtensions.cs These are probably the most interesting jumping off points for new Razor. |
SummaryThe best infodump on the work that we've been doing are these two posts: I also want to quote myself and emphasize a few parts of the first paragraph... We're current undertaking an effort as part of the Razor Pages effort to improve the fundamentals, and extensibility of Razor. Much of the Razor codebase was ported as-is from the original implementation and hasn't had a fresh coat of paint in a long time. Our intention here is to start fresh with APIs for using Razor and build a set APIs that meet our extensibility goals and can be stable for the future. What have we doneToolingWell first of all you'll notice there's now a VS Extension project checked in to our repo. This is new and exciting because the engineers shipping the tooling are also fixing bugs and implementing features in ASP.NET as part of their day-to-day. The folks that have been responsible for Razor in VS in the past are still involved and still our partners. We think that the best way to divide the work of supporting Razor means that my direct team takes on a bigger responsibility. This also means that you can F5 the tools (seriously), and that development/bugs/features for the tools are all happening on github in a way that you can see. We have a roadmap for tooling that is really exciting and extends a few releases into the future, we'll be ready to discuss it more publicly once we do a few more rounds of planning with the team. AbstractionsYou'll see that we've started fresh with the abstractions and types in the codegen/compilation side of Razor. This is largely a rewrite of the backend of Razor, the part that transformation and does code generation. The original parser is there, and it's buried, we're unsure if we'll ever have the need to expose any of those APIs. ExtensibilityWe've thought more about providing the right kinds of extensibility and providing integration points that are simple and powerful. For instance, the parser is no longer customizable - except by defining a custom directive which we will parse for you. The kinds of extensibility MVC used to do to light up features are way simpler now. Here's the code to parse We've separated the runtime assemblies from the compiler assemblies. Runtime Assemblies There's a wall here Pure Language Razor + Roslyn Tooling Only (does not ship on NuGet.org) MVC's Extensiblity for Razor Microsoft.AspNetCore.Mvc.Razor.Extensions You'll also see that we've aligned more closely with Roslyn. Q & AFrom @thecodejunkie
This has always been the intent for Razor - I'd be curious about how we got this so wrong in the past? Was it the coupling between the compiler assembly and the runtime assembly? We're doing work in this release to free codegen from the ghosts of the past.
Notice I said that we did work to free codegen, not that we've taken advantage of that freedom 😆. There's way too much policy in the base class for a Razor page/view right now. We'd like to address that in a future release.
All the code in the Razor repo just generates code. The So I think the line you're looking for is the 'base class' for pages. I don't believe anything in the Razor codegen knows anything about the HTTP stack. That's all done by the 'base class' that MVC provides.
Here's the best answer I can give: https://github.com/aspnet/Razor/blob/dev/src/Microsoft.AspNetCore.Razor.Runtime/Microsoft.AspNetCore.Razor.Runtime.csproj That's the 'runtime' for Tag Helpers and it only pulls in HTML abstractions, which is super duper lean and doesn't pull in any of the HTTP stack. I see you raised a concern about this - your call if that's good enough for your requirements. I'd like to understand why if you still don't like it. You have a few options for Tag Helpers if you don't want to depend on Razor.Runtime. If you want to provide more feedback I can try to help.
Layout pages, sections, templates and other view-rendering ilk are entirely part of MVC's view engine. You can only get the parsing part of sections and templates for free. Tag Helpers depend on the Razor.Runtime set of assemblies (explained above).
Start with
FYI that type is things like our diagnostics page, which use Razor at build time to generate the C#. The implementations used by our view engine are much more complicated. That's a good starting point though and you should still be fairly compatible. We've actually made one simplification, which reminds me we should update this code... dotnet/extensions#236
These things are still tied to the view engine. We did add support for imports to the core of Razor. This is needed because it actually effects codegen. We also added the ability to have a global import as part of the template engine. The feature alone obviated the need for 4-5 other kinds of extensiblity because you can now express those things in-language.
I would be happy to set up a time to chat if that's what would be most helpful. From @daveaglick:
I'm not sure if you mean Razor Language the assembly or Razor Language the language.... The Razor Language is not going to change. We tightened up a few semantics here and there as part of this release because we're now thinking about 3rd parties extending the system. This release would have been our chance to make breaking changes to Razor the language, and there just wasn't enough value there to justify it - so what's there is there. I wouldn't expect Razor the language to change much ever. We will do more Tag Helper features and continue to updated it as C# updates, but we're happy with where it is. The assembly is going to have a lot more small API cleanup as we march towards 2.0.0 RTM, but it's pretty stable. Nothing is going to break dramatically between now and then, just the usual tweaks and renames. |
Is there any news on this front? |
Hey, I maintain this, https://github.com/MyUNiDAYS/MustardBlack/tree/razorcore which is a web framework similar to Nancy. In early versions we were heavily inspired by the Nancy Razor view engine, so I felt it was only proper for me to return the favour here and point you at the RazorCore implementation I've just got working. Its still a WIP, but the Many thanks to @rynowak for his pointers above. I'm working on this right now so keep your eyes open for new pushes, and it will be back in |
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:
IServiceProvider
implementation for TinyIoC that the Razor host can consume. This seemed easier, at least for now.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:
HttpContext
)*.MSBuild
projects? currently targetingnetstandard 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!