Attempt to integrate razor view engine #6

ghost · 2017-02-10T23:38:56Z

I hope this closes the #1

I googled about razor engine and F# and found some interesting projects like FSRazor, Shaver and RazorEngine. But none of those is .net core 😢 Then, i found a nice C# implementation based on SO this answer so i decided to use this.

By now you can configure where your views (.cshtmls) are placed: let view = configureRazor "views" or use a default config with let view = defaultRazor (), then you call your view function like view "Person.cshtml" model

There are some pitfalls, like you must use namespaces on your models other way razor can't find the @model on .cshtml.

dustinmoris · 2017-02-11T01:36:40Z

Hi,

I wanted to let you know that I saw your pull request and I already started to look into it.

You are right, it seems to be difficult to find a Razor library for .NET Core at the moment. RazorLight looks cool, but I saw a potential issue with it which I raised with the maintainer now.

Apart from that I like what you have done. There's a few things I would suggest to improve (e.g. have IRazorLightEngine as a dependency so we don't have to instantiate it every time) but I will give more detailed feedback when I have more clarity regarding the async/await issue I raised.

Many thanks for helping out here and taking the initiative to resolve #1. I appreciate it a lot and will do my best to get this polished with you before merging.

Just FYI I am travelling tomorrow and will be back on Sunday late night, so might not be able to respond or work on this for the next 2 days, but will get back to it as soon as possible. Many thanks again!

dustinmoris · 2017-02-13T01:24:50Z

src/AspNetCore.Lambda/HttpHandlers.fs

+    razor (EngineFactory.CreatePhysical(views))
+
+let defaultRazor () = configureRazor ""


I was thinking that instead of having to create the RazorLightEngine every time you could merge all 3 functions to one like this perhaps:

let razorView (viewName : string) (model : obj) = fun (ctx : HttpHandlerContext) -> let engine = ctx.Services.GetService<IRazorLightEngine>() let view = engine.Parse(viewName, model) setHttpHeader "Content-Type" "text/html" >>= setBodyAsString view <| ctx

Here the HttpHandler tries to resolve an IRazorLightEngine from the service provider.

You could create an extension method like this:

type IServiceCollection with member this.AddRazorEngine (viewsFolderPath : string) = this.AddSingleton<IRazorLightEngine>(EngineFactory.CreatePhysical(viewsFolderPath));

... which then can be used like this:

type Startup() = member __.ConfigureServices (services : IServiceCollection) = services.AddRazorEngine("C:\\SampleApp\\views") |> ignore

I think this would be more incline with existing ASP.NET Core patterns and it would allow users to configure the engine only once as a singleton or provide a custom implementation.

What do you think?

i'm not creating the engine on each call (i think so), those functions configure and default returns a partial applied function with the engine, so, in the app you do let view = razor () and now the engine is created once and you can use it wherever view "foo" model But, however, hey! I like your approach, uses the built in DI and looks more "aspnet friendly" 😸

Oh yeah looking back at the code you're right, it was instantiated only once.

dustinmoris · 2017-02-13T01:27:56Z

tests/AspNetCore.Lambda.SampleApp/project.json

        "Program.fs"
      ]
-    }
+    },
+    "embed": [ "**/*.cshtml" ],


When using the physical RazorLight engine then the views don't need to be embedded and this line can be removed.

dustinmoris · 2017-02-13T01:28:22Z

tests/AspNetCore.Lambda.Tests/project.json

-    }
+    },
+    "preserveCompilationContext": true,
+    "embed": [ "Person.cshtml" ]


Here as well.

dustinmoris · 2017-02-13T01:29:25Z

tests/AspNetCore.Lambda.SampleApp/Person.fs

+[<CLIMutable>]
+type Person = {
+    Name : string
+}


This is only formatting, but to be consistent I'd prefer to stick with this style please:

type Person = { Name : string }

dustinmoris · 2017-02-13T01:30:55Z

src/AspNetCore.Lambda/project.json

@@ -35,7 +35,8 @@
        "Microsoft.AspNetCore.Hosting": "1.1.0",
        "Microsoft.AspNetCore.Diagnostics": "1.1.0",
        "Newtonsoft.Json": "9.0.1",
-        "DotLiquid": "2.0.64"
+        "DotLiquid": "2.0.64",
+        "RazorLight": "1.0.0-rc1",


Haven't tested it, but could imagine that the trailing , might cause a json formatting error?

dustinmoris

Please have a look at my comments and what you think of the suggestions. Also the unit tests seem to have failed in the AppVeyor build. If you fix these things I'd be happy to merge.

ghost · 2017-02-16T20:05:37Z

hi, sorry for delay, was a busy week. I've updated the implementation following all of your comments on my last commit. Let me know if there is any other issue on this! ❤️

dustinmoris · 2017-02-16T20:18:41Z

Hi, thanks! I am currently out but will have a look later tonight!

dustinmoris · 2017-02-17T00:29:10Z

src/AspNetCore.Lambda/Common.fs

+
+let combinePaths (path1 : string) (path2 : string) =
+    Path.GetFullPath(Path.Combine(path1, path2))


This method is probably not needed anymore.

dustinmoris · 2017-02-17T00:32:56Z

src/AspNetCore.Lambda/HttpHandlers.fs

+        setHttpHeader "Content-Type" "text/html"
+        >>= setBodyAsString view
+        <| ctx


Let's add some XML documentation to the handler. Example:

/// Parses and compiles a Razor view with the associated model and then writes its content to the body of the response. /// It also sets the HTTP header Content-Type to text/html.

dustinmoris

Looks good. Only two minor things to change please and if you could please update the README with this new handler as well?

ghost · 2017-02-23T16:31:17Z

Hi again, man, thanks for your guidance and patience on this PR! I've added the comments and updated the readme as you suggested. Let me know if there is any other issue on this!

dustinmoris · 2017-02-24T10:24:42Z

Hey no worries. There's been many changes happening since the last merge again, so there are more conflicts again which prohibit me from merging your PR. This weekend I will take everything you've done and update a branch with it and then probably merge it back into dev/master from there if that is okay with you?

ghost · 2017-02-24T12:58:29Z

sure!

ghost · 2017-02-24T18:41:12Z

Hi! i think i get done the merge with develop branch! 🎉 those was a lot of changes, yay fsproj! I had some troubles, though:

I've changed the <TargetFramework> to netstandard1.6 'cause 1.3 was generating the error The target "CreateManifestResourceNames" does not exist
I've added the line <EnableDefaultCompileItems>false</EnableDefaultCompileItems> 'cause i was getting the error Duplicate 'Compile' items were included.
i've changed the ItemGroup Condition to <ItemGroup Condition=" '$(TargetFramework)' == '*netstandard1.6*' "> 'cause RazorLight doen't support 1.3. isn't there a way to use >= or something like that? à la paket...?

Even though my tests, both of them, main tests and sample app's tests, are broken... can't find the View! i've added a todo line on the sampleapp in order to get some advice... buuut the sample app is working yay!

dustinmoris · 2017-02-25T12:49:34Z

Thanks for the effort!!!

Attempt to integrate razor view engine

e424915

dustinmoris reviewed Feb 13, 2017

View reviewed changes

dustinmoris requested changes Feb 13, 2017

View reviewed changes

dustinmoris and others added 2 commits February 15, 2017 22:18

Merge branch 'master' into 1-razor-support

d5a74ca

improve razor support based on comments

c4e3bf7

dustinmoris reviewed Feb 17, 2017

View reviewed changes

xml comments and readme with razorView info

ae9d3f8

Nicolás Herrera added 3 commits February 24, 2017 10:23

merge develop

4b1d733

merge develop razor-views now working lol

9cda10b

merge develop razor-views - fuck, include all changes plz

5779cbd

dustinmoris changed the base branch from master to develop February 24, 2017 21:20

dustinmoris merged commit c8245b9 into giraffe-fsharp:develop Feb 25, 2017

This was referenced Feb 25, 2017

Target lower .NET Standard #21

Closed

Add Razor support #1

Closed

ghost deleted the 1-razor-support branch April 17, 2017 00:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Attempt to integrate razor view engine #6

Attempt to integrate razor view engine #6

ghost commented Feb 10, 2017

dustinmoris commented Feb 11, 2017

dustinmoris Feb 13, 2017 •

edited

Loading

ghost Feb 13, 2017

dustinmoris Feb 13, 2017

dustinmoris Feb 13, 2017

dustinmoris Feb 13, 2017

dustinmoris Feb 13, 2017

dustinmoris Feb 13, 2017

dustinmoris left a comment

ghost commented Feb 16, 2017 •

edited by ghost

Loading

dustinmoris commented Feb 16, 2017

dustinmoris Feb 17, 2017

dustinmoris Feb 17, 2017

dustinmoris left a comment

ghost commented Feb 23, 2017

dustinmoris commented Feb 24, 2017

ghost commented Feb 24, 2017

ghost commented Feb 24, 2017 •

edited by ghost

Loading

dustinmoris commented Feb 25, 2017

		razor (EngineFactory.CreatePhysical(views))

		let defaultRazor () = configureRazor ""


		let combinePaths (path1 : string) (path2 : string) =
		Path.GetFullPath(Path.Combine(path1, path2))

Attempt to integrate razor view engine #6

Attempt to integrate razor view engine #6

Conversation

ghost commented Feb 10, 2017

dustinmoris commented Feb 11, 2017

dustinmoris Feb 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dustinmoris left a comment

Choose a reason for hiding this comment

ghost commented Feb 16, 2017 • edited by ghost Loading

dustinmoris commented Feb 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dustinmoris left a comment

Choose a reason for hiding this comment

ghost commented Feb 23, 2017

dustinmoris commented Feb 24, 2017

ghost commented Feb 24, 2017

ghost commented Feb 24, 2017 • edited by ghost Loading

dustinmoris commented Feb 25, 2017

dustinmoris Feb 13, 2017 •

edited

Loading

ghost commented Feb 16, 2017 •

edited by ghost

Loading

ghost commented Feb 24, 2017 •

edited by ghost

Loading