search

T4MVC / R4MVC

R4MVC is a Roslyn code generator for ASP.NET Core MVC apps that creates strongly typed helpers that eliminate the use of literal strings in many places
Apache License 2.0
159 stars 48 forks source link

Create equivilent of T4MVCExtensions #19

Closed wwwlicious closed 7 years ago

wwwlicious commented 9 years ago

T4MVC uses a number of interfaces and classes which do not need to be generated in the target project such as IT4MVCActionResult.

Suggest following the same idea and create coreclr compatible versions of the following folder https://github.com/T4MVC/T4MVC/tree/master/T4MVCExtensions

kevinkuszyk commented 9 years ago

I was thinking we could just take a dependency on the T4MVCExtensions nuget package, rather than re-implementing it.

I hadn't considered we'd need a coreclr version though until just now. How much work would be involved in making a coreclr version of T4MVCExtensions do you think, vs just re-implementing it?

@davidebbo - do you have any thoughts on this?

wwwlicious commented 9 years ago

a couple of thoughts on why it might be unavoidable to diverge from t4mvc.

the coreClr version of these classes as you mentioned being the first, another is that by including a reference of R4MVCCompilerModule in the target project you are already referencing the dll, this is different from t4mvc where the payload is some .tt and xml files.

The namespaces also use System.Web.Mvc, this is Microsoft.AspNet.Mvc for vNext.

ActionResult helpers should target IActionResult for vNext so a lot less subclassing.

one other is that, as I worked with roslyn, there are things that make less sense when not using reflection, such are the T4MVCAttribute, in T4MVC, this used a bool constructor argument which initialized a property about including or excluding. With Roslyn, the attribute never has to compile for it to be useful, you are just checking the syntax tree for the node and in fact you would never check any property, merely the constructor argument that was passed to it. This is why I opted for the R4MVCExcludeAttribute as it was actually much easier to check for in Rosyln.

I think there are a few other changes also in mvc which really require a re-implementation but I have only explored bits of the extension classes.

davidebbo commented 9 years ago

My take is that there are enough differences in the new world that you may indeed be better off starting clean of the the helper assembly. It's not all that much code, and it will yield something that just fits better.

kevinkuszyk commented 9 years ago

I think that settles it then - we should re-implement for R4MVC.

artiomchi commented 7 years ago

Actually, reusing T4MVCExtensions would be a pretty bad idea.

T4MVCExtensions depends on the full framework, including a reference to System.Web. The new MVC framework is a complete rewrite, which moves away from System.Web to make it leaner and faster (and fix a whole bunch of architectural issues with the old stack). It also is built against .NET Standard which makes it cross platform compatible. As such, unless you want to force a dependency against the full .NET framework - you'd have no choice other than to create a whole separate project for MVC Core

kevinkuszyk commented 7 years ago

@artiomchi agreed.

artiomchi commented 7 years ago

@kevinkuszyk I would suggest to reopen this issue, since the library still has to be created, so it would be used to track this progress...