Genie.jl icon indicating copy to clipboard operation
Genie.jl copied to clipboard

Support HAML.jl and/or support pluggable renderers

Open tkluck opened this issue 4 years ago • 10 comments

Following up on the discourse exchange. What's the best way to add HAML.jl support as a templating language?

It's already straightforward for a user to just return a HTTP.Response with body generated by HAML, but real integration would:

  • use Genie's native template discovery
  • compile HAML.jl's $x to Genie's @vars(:x)
  • more?

Happy to send a pull request if you can give me some guidance!

tkluck avatar Oct 15 '19 19:10 tkluck

@tkluck Thanks for this, it would be awesome to add HAML support!

This is the workflow for rendering the HTML views:

  1. the user code calls Renderer.html() to invoke HTML rendering
  2. Renderer.html() -> Renderer.tohtml()
  3. Renderer.tohtml() -> Flax.html_renderer()
  4. Flax.html_renderer() -> Flax.get_template()

In get_template is most of the logic. I suggest mirroring the implementation for Markdown rendering, it's only a few lines of code. We have:

content = if extension in MARKDOWN_FILE_EXT
    vars_injection, md = include_markdown(path, context = context)
    string_to_flax(md, partial = partial, f_name = f_name, prepend = vars_injection)
else
    html_to_flax(path, partial = partial)
end

So we need a new condition

elseif extension in HAML_FILE_EXT
    vars_injection, haml = include_haml(path, context = context)
    string_to_flax(haml, partial = partial, f_name = f_name, prepend = vars_injection)
end

The include_markdown function returns a (HTML) string resulting from parsing the Markdown file. The vars_injection part supports Markdown metadata and simply dumps the variables definitions. You can use the same strategy to inject the vars. Then the string_to_flax function converts the resulting (HTML) string to Genie's Julia-based template and stores it as a Julia file (in the build/FlaxViews/ folder). The generated file is then used, until the template file changes (the change is detected based on file timestamp). When that happens, the template is re-parsed and the whole process repeats. However, this build, invalidation, and regeneration process is automatically handled by Genie.

Re template discovery, we only need to define the HAML_FILE_EXT vector and it will just work.

I suggest to opt-in to this workflow and parse the HAML file and return the HTML string from include_haml() - potentially using a strategy similar to Markdown parsing for vars_injection.

Let me know if you have any questions or suggestions -- or if there are any issues integrating in this workflow.

** Food for thought **

  1. Should we move out Flax.html_renderer out of the Flax module into something more generic, like a new HTMLRenderer module? It might make sense if we see Flax as a type of template itself. Or we can leave it as is if we see Flax as a rendering engine that uses one of the multiple templating dialects.

  2. Can we make dependencies on external parsers (HAML, Mustache, etc) optional? (Probably not without Pkg warnings like "Genie does not have HAML in it's dependencies"...).

I hope this helps.

essenciary avatar Oct 16 '19 09:10 essenciary

Thanks for the detailed thoughts @essenciary !

To start with your

Can we make dependencies on external parsers (HAML, Mustache, etc) optional? (Probably not without Pkg warnings like "Genie does not have HAML in it's dependencies"...).

I think this is where Requires.jl is a good fit. I've had (limited but) good experiences with it.

I think

Should we move out Flax.html_renderer out of the Flax module into something more generic, like a new HTMLRenderer module?

is also a good idea. It could be achieved by something like

module HTMLRenderer

abstract type Renderer end

const known_extensions = Pair{String, Renderer}[]

function render(r::Renderer; kwds...)
    error("No renderer implemented for $r")
end

end # module

module Flax # ...or HAML, or other renderers

struct Flax <: HTMLRenderer.Renderer end

function __init__()
    push!(HTMLRenderer.known_extensions, ".flax.html" => Flax())
end

function HTMLRenderer.render(::Flax; kwds...)
    return HTTP.Response(.....)
end

end # module

What do you think? The nice thing about this scheme is that it's also "reversely" pluggable; i.e. even if Genie doesn't know about a templating language yet, some other package can push something onto the known_extensions. It's also possible to do something with method overloading like

find_renderer(::Val{Symbol("flax.jl")}) = Flax()

but I'm not sure I like it; there should be a way to encode the priority if there's files with the same name but different templating languages. The vector known_extensions provides that.

tkluck avatar Oct 16 '19 15:10 tkluck

@tkluck The proposed architecture looks great :) Should I make an attempt to implement it and clear the path for your HAML integration? Or would you like to do the honors?

I'll take a look at Requires.jl - I heard about it before but never checked it, thanks!

essenciary avatar Oct 17 '19 18:10 essenciary

I'll be happy to send a pull request. Hopefully I'll find time over the weekend :)

As for integrating using Requires.jl: for now it's probably a better idea if I put the glue code in HAML.jl. It's a less mature package than Genie.jl so there's more likely to be breaking changes that need a simultaneous update of the glue code. Is that okay for you?

tkluck avatar Oct 18 '19 09:10 tkluck

Awesome, thanks! Yes, I agree - we shouldn't worry about Requires. That would be a larger refactoring, affecting more areas.

essenciary avatar Oct 18 '19 10:10 essenciary

@tkluck Current master includes the refactoring of the rendering layer. Now Renderer provides an interface and helper methods for any renderers - and the actual renderers implementations add their own render methods.

It's a breaking change as the users are now expected to use the specific renderer implementation, per their needs. The advantage is that it's extensible with 3rd party renderers provided as packages/modules and unused renderers are no longer automatically eager loaded. Also, I have renamed them to Html, Json, and Js.

I'll add some docs - it is expected for a renderer to export a dedicated method and optionally extend Genie.Renderer.render providing a specialisation for their MIME type (although not required).

So for Haml we need something in the line of:

  1. new module called Haml which provides all the logic.
  2. extend Genie.Renderer.render in the line of
function Genie.Renderer.render(::Type{MIME"text/haml"}, data::String; context::Module = @__MODULE__, layout::Union{String,Nothing} = nothing, vars...) :: Genie.Renderer.WebRenderable
  1. export a html method in the line of
function html(data::String; context::Module = @__MODULE__, status::Int = 200, headers::Genie.Renderer.HTTPHeaders = Genie.Renderer.HTTPHeaders(), layout::Union{String,Nothing} = nothing, vars...) :: Genie.Renderer.HTTP.Response
  Genie.Renderer.WebRenderable(Genie.Renderer.render(MIME"text/haml", data; context = context, layout = layout, vars...), status, headers) |> Genie.Renderer.respond
end

essenciary avatar Jan 08 '20 10:01 essenciary

That's really cool @essenciary ! I can't promise at the moment when I'll have time, but I'll do this as soon as I can divert some attention to HAML.jl. Would be exciting to get these to work together :)

tkluck avatar Jan 15 '20 12:01 tkluck

I just added a compatibility method to HAML.jl here: https://github.com/tkluck/HAML.jl/blob/master/src/Genie.jl

With this approach, the user just imports

import HAML.GenieTools: html

and uses that in their controller just as they would use the "normal" html function from Genie.

It would be even cooler if Genie supported a way to register a view's extension and calls the appropriate method accordingly, but that doesn't currently seem to be possible, right?

Depending on whether you'd want to support something like that, feel free to either leave this ticket open or close it.

While working on this, I stumbled on a possible security issue and I'll open a separate ticket for that.

tkluck avatar Mar 13 '20 18:03 tkluck

@tkluck Yes, I think we need to modify Genie's renderers. Cause otherwise you need to add Genie as a dependency for HAML.jl which is overkill.

My recommendation is to have this as a Genie plugin/adapter which:

  1. is a different package, ie GenieHAML
  2. this package has 2 dependencies: Genie and HAML
  3. the package does:
import Genie, HAML

function Genie.Renderers.Html.html(...)
# logic here
end

This wouldn't work now as it would overwrite Genie's function. So we're going to need to allow spcializations, probably by passing some sort of "source" MIME type.

Ex now we have something in the line of:

function Genie.Renderer.render(::Type{MIME"text/html"}, data::String; context::Module = @__MODULE__, layout::Union{String,Nothing} = nothing, vars...)

which would also need to inform about the "source" file and invoke the correct renderer:

function Genie.Renderer.render(::Type{MIME"text/html"}, data::String, ::Type{MIME"application/octet-stream"}; context::Module = @__MODULE__, layout::Union{String,Nothing} = nothing, vars...)

Problem is, HAML does not have its own MIME type!

Hmmm, I need to think about it. What do you think?

essenciary avatar Mar 13 '20 18:03 essenciary

Cause otherwise you need to add Genie as a dependency for HAML.jl which is overkill.

I prevented that by using Requires.jl which only loads the compatibility code when Genie is also loaded. I think that's easier to understand and more maintainable than using a compatibility package.

So we're going to need to allow spcializations, probably by passing some sort of "source" MIME type.

Yes, this makes a lot of sense! We could decide to just use the file extension instead of the mime type. This is also what I was getting at in the last bit of https://github.com/GenieFramework/Genie.jl/issues/180#issuecomment-542764524 .

tkluck avatar Mar 13 '20 19:03 tkluck