Open tkluck opened 4 years ago
@tkluck Thanks for this, it would be awesome to add HAML support!
This is the workflow for rendering the HTML views:
Renderer.html()
to invoke HTML rendering Renderer.html() -> Renderer.tohtml()
Renderer.tohtml() -> Flax.html_renderer()
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
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.
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.
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 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!
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?
Awesome, thanks! Yes, I agree - we shouldn't worry about Requires. That would be a larger refactoring, affecting more areas.
@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:
Haml
which provides all the logic. 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
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
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 :)
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 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:
GenieHAML
Genie
and HAML
import Genie, HAML
function Genie.Renderers.Html.html(...)
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:
```julia
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?
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 .
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:$x
to Genie's@vars(:x)
Happy to send a pull request if you can give me some guidance!