-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for Phoenix 1.6 & HEEx #92
Comments
Not a code owner, or really even a contributor, but I do have...
about this. First off, the # /lib/slime/compiler.ex:98..108
defp render_attribute_code(name, _content, quoted, _) when is_list(quoted) do
quoted |> Enum.map_join(" ", &Kernel.to_string/1) |> (& ~s[ #{name}="#{&1}"]).()
end
defp render_attribute_code(name, _content, quoted, :eex) when is_binary(quoted), do: ~s[ #{name}="#{quoted}"]
defp render_attribute_code(name, _content, quoted, _) when is_binary(quoted), do: ~s[ #{name}="<%= {:safe, "#{quoted}"} %>"]
# NOTE: string with interpolation or strings concatination
defp render_attribute_code(name, content, {op, _, _}, safe) when op in [:<<>>, :<>] do
value = if safe == :eex, do: content, else: "{:safe, #{content}}"
~s[ #{name}="<%= #{value} %>"]
end I may be woefully uninformed on this, but the change to accommodate HEEx's variable syntax seems like it might just be as simple as replacing the The component name problem is a trickier one, as it will need a clever solution, and clever is the enemy of clean, but I do think the only way of working through it is going to be to add another symbol (or delimiter) to Slime's PEG. In fact, the symbol may be redundant, as templates would still need to incorporate delimiters in order to know when ... Maybe... in a tongue-in-cheek reversal... Slime could delimit component names like... main.container
article
{MyApp.Component.function}.mycomponent.element.class-names[and=:then come=~w[the attributes]] ? Maybe...? Or does the curly braces' similarity to the standard The other idea I had would be to adopt something akin to the struct declaration (which would be somewhat apropos, given the overall similarity between a component with properties and a struct with fields), and denote a delimited component name with a main.container
%{MyApp.Component.function}.mycomponent.class-names[and=:then come=~w[the attributes]]
section.foo.bar
/ or maybe even literally
main.container
%MyApp.Component.function{and: :then, come: ~w[the attributes]}.mycomponent.class-names
section.foo.bar |
From a surface level, that appears to be a good take. However some deep recess of my mind warns me that there is HTML escaping magic going on around I really like the literal struct-like implementation. It feels elixiry, and I'm having a hard time mentally poking holes in it. Shouldn't be too difficult to get the PEG to support either. Amusingly, Slim's precursor, HAML, used %html
%head
%title Foo Slim's greatest idea was tossing that useless marker: html
head
title Foo |
Total noob here but saw this issue on reddit and thought it was interesting. I had an idea for how to approach the dot syntax collision. Essentially you would have two paths when parsing an element preceded by a dot. First you need a list of all modules in the project, which can be generated with |
@stevensonmt Sounds reasonable to me. Not sure if that would have implications with compilation vs runtime, since i don't know the details of the internals very well, but that sounds like a reasonable approach. Another thought I had based on what I just read on the
If this is true, could we completely forgo supporting the Ref: https://github.com/phoenixframework/phoenix_live_view/blob/master/CHANGELOG.md#enhancements |
@stevensonmt while thats entirely possible, it does open the door to more complexities, namely, the need to keep (and update on HMR) a list of all the modules in the codebase, and a scan when compiling slim files into render functions. It also has no clean way of avoiding collisions. What if you have a Making slime respect |
I'm all for keeping it simple. I'd love to get something going that I could start to work with, test, and use, even if it was pretty verbose (as in using @paradox460 Any thoughts about copying 0.17's convention of using a |
@tensiondriven it does not seem very slim-like, but it does get stuff working faster, so there's the tradeoff. IMO the |
I dug into this pretty deeply - To implement HEEx name/attribute awareness, the changes to the library itself aren't that significant, its more how the engines are wired up. Here's what I found so far (posting for my own learning and recollection later, and possibly the benefit of others) In Phoenix app,
(From the docs, The new This doesn't address how to support the dot-syntax for |
I discovered something interesting and problematic. This function:
is responsible for taking a name/value pair and producing valid
I'm not sure why this has to be represented as inline EEx here; perhaps because the function with represents the template is generated at compile time, and that inline EEx needs to be evaluated at runtime? That's all I can think of so far. |
I've got a working branch! It requires changes to both I needed to branch the logic in the PS the new file extension for a slime - heex file is |
I have a branch running that successfully generates HEEx which is then parsed by to produce HTML. I would love some feedback on the approach. The work is done in the |
trés chic |
This issue seems to be open for some time now so the question would be how does the LiveView support for Phoenix 1.6+ looks as of now? |
It's working great, I'm using it in production and haven't run into any issues. Currently it doesn't support a sigil, which as far as I know is the only parity difference between the existing dep and this. At this point, I think we're waiting for @doomspork to merge and cut a release. The sigil will be addressed later. As far as I know, there hasn't been much testing apart from me. I'd love some testing help/feedback, if you'd like to give it a go:
|
@tensiondriven - thank you for your response! So basically with your branch one adds |
Hi @tensiondriven I'm crossposting tensiondriven/phoenix_slime_test#1 (comment) here as it may be more relevant to this thread. Tldr; does the fix not support phx.gen.html.slime for templates yet? |
Hi @tensiondriven, did you have any luck contacting @thepeoplesbourgeois to reboot this PR? |
What's the story? Anyone looking at this anymore? |
Phoenix 0.16 (and LiveView 0.16) introduce HEEx, which is a huge step forward, enabling semantic HTML parsing/validation.
I depend on phoenix_slime and am hopeful that the library can be updated to support HEEx. The main issues I see are:
{ }
syntax for attributes.form
)Since HAML/Slim/Slime reserve the dot for classes, it seems that this new convention in HEEx may be directly incompatible with Slim/Slime as we've known it. Hopefully there's some way to support it.
For anyone close to the code in this library, do you have a sense of what it would take to upgrade phoenix_slime to support the latest incarnations of Phoenix/LiveView/HEEx?
The text was updated successfully, but these errors were encountered: