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

Progress #1

Closed wwwlicious closed 7 years ago

wwwlicious commented 9 years ago

Hi Kevin, Thought we should move our discussion here since this is what we are discussing.

I've gotten a bit further on today with generating the controller partial classes with support for inheritance and basic generics. Your changes made for testing look good but I'm not sure how to pull them into my fork as our approaches are slightly different.

Take a look if you want and see if there is anything in there that is helpful, happy to help if I can, roslyn is certainly "an adventure" :)

kevinkuszyk commented 9 years ago

Thanks, I was going to suggest the same.

Apologies for reworking the project structure - I've spend the past couple of days trying to make it build on the CI server. I thought there were some namespace issues with the R4Mvc.Test and R4Mvc.Test.Site projects, which is why I killed the R4Mvc.Test.Site and replaced it with R4MvcHostApp which is more in keeping with the T4MVC repro.

At the moment I have lots of questions and design decisions to figure out. I'll raise them all up on here as issues - I'd welcome your thoughts. Also any help with building out the solution would be much appreciated.

I'll have a look at the code in your fork later today.

kevinkuszyk commented 9 years ago

I think you're on the right lines on your fork, although it looks like you've hit one of the problems I did - which is the solution doesn't build. Once I updated all the nuget packages that fixed it. Try pulling the latest from this repro in to see if that makes it build.

I finished my brain dump earlier, and it looks like you've already made some progress on issues #4, #5, #6, and #7. Let's move the conversation onto each of the issues.

If you're able to get some of the issues working with tests and are happy with the code, I'll merge them in. In the meantime feel free to open pull requests and we can discuss the code as you build it out.

wwwlicious commented 9 years ago

I haven't had issues compiling my fork but could not compile yours, perhaps there is something in the kvm version we're running? I've had issues with VS CTP and the kvm caching compilation though preventing building even after the issues were fixed, restarting these usually resolved things.

I'll have a look at creating some tests tomorrow but I'm new to git pull requests, never done one so I apologise in advance if I make a mess of it!

kevinkuszyk commented 9 years ago

It seems that upgrading the nuget packages to beta4-* broke compatibility with VS. When I rolled everything back I was able to build my repro and yours. I've spent that past few days trying to get it to build on the build server too.

I'm new to git pull requests, never done one so I apologise in advance if I make a mess of it!

Don't worry. Thanks for the first PR - I've had a quick look on GH and it looks good. I'll try to merge in in tomorrow.

I typically use GitHub Flow, so let's use that on this project too. We need to get your master synced up with mine, so that you can work on topic branches and create future pull requests from them instead of your master branch. I'll try and figure out the git commands to do that tomorrow.

wwwlicious commented 9 years ago

Sounds good, I thought I would keep PR's small until I understand the workflow better and can get unit testing up and running to test some of the stuff I wrote.

kevinkuszyk commented 9 years ago

Ok, so I forked your repro and ran the following git commands to get it into a state where it's synced up with my repro. I'm by no means a git expert, so please make a backup of your local repro before trying this. You can see the result here. My objectives were:

  1. Get your master in sync with mine.
  2. Integrate your existing work in your master with mine, and put it in a dev branch you can use to raise a new pull request from later.
  3. Not lose any work!

Here are the steps:

Create a new branch called dev from your master branch:

git checkout -b dev master

Push your new dev branch up to GitHub:

git push origin dev

Delete your master branch:

git branch -d master

Add the offical R4MVC repro as a remote. We will used this later to pull changes into your fork:

git remote add upstream https://github.com/T4MVC/R4MVC.git

Fetch the latest changes from the upstream repro:

git fetch upstream

Make a new master branch from the upstream repro:

git checkout -b master upstream/master

Overwrite your master on GitHub with the one from the upstream repro:

git push origin master -f

I then rebased your dev branch on the updated master:

git checkout dev
git rebase master

There were quite a few conflicts to resolve, but most of them were with the project.json. I used TortoiseGit to resolve them, and for most of them I used the version from master.

Finally I pushed the updated dev branch back to GitHub:

git push origin dev

Once this is done you should be able to pull the latest changes in from my repro into you master branch at any time. You can then rebase your topic branches to keep them up to date:

git checkout {topic-branch}
git rebase master

If you work in topic branches and raise pull requests when work is ready to integrate the GitHub flow should work. The trick is to keep your master branch for keeping in sync with the upstream repro (it took me a while to figure that out!).

I will also work in topic branches in the main repro so you can keep track of what I'm working on. Feel free to open pull requests early for code review or to discuss options - I know I will be! Once a pull request is open you can continue pushing to the branch to update it. More info on pull requests here.

wwwlicious commented 9 years ago

thank you for taking the time for this. I followed the steps with one or two bumps but it really helped

artiomchi commented 7 years ago

Just realised that I was just spamming the "New maintainers wanted" issue with project discussion, so thought to add an update here, unless there's a better place for this.

So far I've managed to get MsBuild compilation working for AspNetCore projects, which means I can leverage a lot of the @wwwlicious's code and build on top of it. I've still been experimenting in artiomchi/T4MVCCoreLite, and I've finally got a working proof of concept.

The POC is based around the dotnet cli / powershell helper that will regenerate the classes. I've also got a class library that has the base IActionResult interface and a tag helper to render links, which is referenced from the Host MVC project. The POC can now successfully generate a link using the following syntax:

<a mvc-action="MVC.Home.Index()">Home</a>

With this stage achieved, I'm planning to start applying this to the R4MVC codebase. I'm considering either waiting for #43 to be merged, or committing on top of it (which will require project renaming (R4MVC -> R4MVC.Tools, and creating a new R4MVC project for the IActionResult interface etc)

kevinkuszyk commented 7 years ago

@artiomchi thanks. Your PR #43 is now merged and we have a green build again.

Regarding next steps, if you can work in a feature branch in your fork and open PRs from there that will work best (see this guide to the GitHub flow). You may need to delete your local master branch, fetch the latest from here, and force push it to your fork to sync it up with our upstream repro now.

Getting the dotnet cli wired up next sounds good.

Regarding the tag helpers, do we need to introduce a new <a mvc-action="..."> one? Can MVC.Home.Index() still resolve to ~/home/index like it used to in T4MVC?

artiomchi commented 7 years ago

GitHub Flow sounds like a simplified, less strict version of Git Flow which I'm usually using (except for this pull request, actually =/), so that sounds good.

Regarding the tag helpers, I'm not sure how much experience you have with ASP.NET Core so I'll have to make some assumptions here for the sake of the readers that haven't written any before.

While ASP.NET Core has support for the old/classic html helpers from previous versions of MVC, it also introduced tag helpers, that can do the same thing, but in a more html like fashion.

For example, both links below will generate <a href="/Controller/Action">link text</a>

@Html.ActionLink("link text", "Action", "Controller")
<a asp-action="Action" asp-controller="Controller">link text</a>

The benefit of the latter is that you have full control over the output html, and you can add any new attributes that you want to the anchor. Again, both examples will generate the same output

@Html.ActionLink("link text", "Action", "Controller", new { id = 12 }, new { @class="button-red" data_bind = "text: titleText })
<a asp-action="Action" asp-controller="Controller" asp-route-id="12" class="button-red" data-bind="text: titleText">link text</a>

For the ASP.NET Core version of T4MVC I've currently added a mvc-action TagHelper that leverages that flexibility and the power of T4MVC. I used mvc to avoid the clash with the default asp prefix, and to specify it's a MVC route, and action verb since the value of that attribute will usually be a pointer to a controller action.

The example above will now look like this:

<a mvc-action="MVC.Controller.Action(12)" class="button-red" data-bind="text: titleText">link text</a>

Of course, classic Html helpers will be supported as well to provide backwards compatibility for people that haven't started using the tag helpers.

My point in all of this is that the new library will have the same functionality as the classic T4MVC, so users will be able to use Html.ActionLink or Url.ActionLink helpers as they did before, but they'll also have the ability to use the newly introduced tag helpers as well.

The generated action overrides are close to identical to their counterparts in T4MVC (e.g. here) and you can see the TagHelper code here to see what I mean

artiomchi commented 7 years ago

P.S. Any preference for the indent character? I usually use the VS default on most of my projects (space), but I've seen some of the classes in the R4MVC source use the tab character.

Ideally I'd like to make sure that it's consistent across the whole solution, but I'll defer to your choice since it's your project

kevinkuszyk commented 7 years ago

Regarding the tag helpers, I'm not sure how much experience you have with ASP.NET Core so I'll have to make some assumptions here for the sake of the readers that haven't written any before.

Yes, not a lot. I'm aware of them, but I don't have any production projects using MVC Core yet.

For the ASP.NET Core version of T4MVC I've currently added a mvc-action TagHelper that leverages that flexibility and the power of T4MVC. I used mvc to avoid the clash with the default asp prefix, and to specify it's a MVC route, and action verb since the value of that attribute will usually be a pointer to a controller action.

That sounds like a good approach.

Of course, classic Html helpers will be supported as well to provide backwards compatibility for people that haven't started using the tag helpers.

My point in all of this is that the new library will have the same functionality as the classic T4MVC, so users will be able to use Html.ActionLink or Url.ActionLink helpers as they did before, but they'll also have the ability to use the newly introduced tag helpers as well.

Great. I missed that point before.

Any preference for the indent character? I usually use the VS default on most of my projects (space), but I've seen some of the classes in the R4MVC source use the tab character.

I don't have a preference. If you prefer spaces, then lets go with that.

artiomchi commented 7 years ago

So far the progress is going pretty good. I've got a working POC with most of the core functionality in place and working for basic scenarios.

Things that are working so far:

While there's still a lot of work to do (the code needs a lot of refactoring to make it more readable, missing functionality, handle more advanced scenarios, support areas, support view components, etc), it's a first working build in some time, which is pretty exciting.

Once I add area support, I'll be dogfooding the project in my own production environment, but additional feedback and testing would be appreciated.

The binaries are being auto deployed to a MyGet feed (didn't want it to be on NuGet yet). You'd need to install both R4Mvc and R4Mvc.Tools packages to a ASP.NET Core project, then open the Package Manager Console in VS, select the web project there and run

Generate-R4Mvc

dotnet cli support will also be added later. I started with the Powershell variant since it was trivial to implement

P.S. Visual Studio doesn't properly display IntelliSense and highlighting for tag helpers at the moment, so you might want to install this package to enable it: Razor Language Services (see more)

artiomchi commented 7 years ago

One other important note. While the R4Mvc project is multi targeted against .NET 4.5.1 and .NET Standard 1.6 (minimal targets for Microsoft.AspNetCore.Mvc package), and the package is cross platform (the website using it can easily be hosted on Mac / Linux / Nano Server), the R4Mvc.Tools package that will be used in development will HAVE to run on a Windows machine.

This is currently a limitation of the MSBuild process which is not cross platform at the time (v15.2). There have been talks about making it cross platform, which might happen in v15.3, but that's up in the air right now. Until then, the tools package is targeting .NET 4.6.2

kevinkuszyk commented 7 years ago

@artiomchi thanks, it sounds like you are making good progress.

Once you have the areas support working, do you think we have enough for a public alpha?

Setting up a CI feed from this project has been on my todo list too, so I'll get that sorted too.

artiomchi commented 7 years ago

Absolutely! I've got two last things I want to do before going to a public alpha - Html helpers (to go alongside the tag helpers) and View locators. Both are quite trivial, actually. The first is going to be mostly a port of the same extenstion class from the main library, with slight changes due to the framework classes/assemblies. And the second will initially follow the default folder conventions. Given some free time, I'll have a pull request within the next couple of days.

The generated code is simple and should be stable (it's very similar to the code generated by T4MVC). I'm not too happy with the tool code - I mostly just started the refactoring, so there's a lot of work to be done still to make it clean, and let's not forget code testing. But that's all on the tooling side.

kevinkuszyk commented 7 years ago

@artiomchi @wwwlicious ok if I close this now in favor of #50?

wwwlicious commented 7 years ago

@kevinkuszyk fine by me 👍

artiomchi commented 7 years ago

@kevinkuszyk fine by me as well!