phoenixframework / phoenix_live_view

Rich, real-time user experiences with server-rendered HTML
https://hex.pm/packages/phoenix_live_view
MIT License
5.99k stars 902 forks source link

HEEx: cannot declare anonymous function components: `<% name = fn assigns -> %><em>@name</em><% end %>` #3289

Closed azizk closed 3 weeks ago

azizk commented 1 month ago

Environment

Actual behavior

The following HEEx code compiles with a warning but works as expected. However, when the code is formatted, the opening <% is changed to <%=, which breaks the functionality.

<% hruler = fn assigns -> %>
  <hr style={assigns[:color] && "background-color: #{@color}"} />
<% end %>

<%= hruler.(%{color: "red"}) %>
    warning: the contents of this expression won't be output unless the EEx block starts with "<%="
    │
  1 │ <% hruler = fn assigns -> %>
    │ ~
    │
    └─ apps/my_web/lib/my_web/live/post_index_live.html.heex:1:1

Expected behavior

It should compile without warnings, and formatting the code should not change <% to <%=, preserving the functionality.

SteffenDE commented 4 weeks ago

Related: https://github.com/elixir-lang/elixir/pull/10566.

I think you may be able to write something like

<% hruler = fn assigns ->
  ~H|<hr style={assigns[:color] && "background-color: #{@color}"} />|
end %>

instead.

I'll say though that what you are doing there is very particular and could have some inadvertent effects on proper change tracking. Is there a reason not to define a separate function component for your hruler?

azizk commented 4 weeks ago

Yes, that's a possible workaround, but it breaks as soon as you have to interpolate something with EEx tags (e.g. <%= @text %>), because the parser doesn't understand that there is a nested HEEx template literal.

It seems peculiar and I don't know if it affects change tracking, but nevertheless it would be cool if it worked so we could define ad-hoc functions like that for repetetive stuff and not be required to touch the HTML or Live module.

josevalim commented 3 weeks ago

Please do a bug report on Elixir. However, I must say that this should be discouraged, it will definitely have negative consequences when it comes to change tracking.

SteffenDE commented 3 weeks ago

@josevalim I looked into it a little bit and the formatter changing it to <%= is caused by this:

https://github.com/phoenixframework/phoenix_live_view/blob/671f6d70e136a7c1da3d551a78d3ce9a721aac7c/lib/phoenix_live_view/html_algebra.ex#L282-L291

where we always force a <%= for EEx blocks ignoring the opt. The warning is from the linked Elixir PR, but I'm not sure what Elixir should do differently?

josevalim commented 3 weeks ago

I see, so it has to be an open issue in both places.

The issue is that both Elixir and LiveView are assuming that, if there is a block, we want to output the result. And this example shows that to be false. Perhaps we can do a more robust check in Elixir (for example, only emit the warning if the block is at the root of the AST node). The fix on LiveView should be simpler, we should instead preserve the block type.

SteffenDE commented 3 weeks ago

I have a local branch to change it in LiveView, will try to push it up later today 👍

feliperenan commented 3 weeks ago

@SteffenDE I went ahead and pushed the fix over here https://github.com/phoenixframework/phoenix_live_view/pull/3299

Let me know if you thought something else and feel free to close in case you have another solution 🙌🏼. I haven't digg into the Elixir warning issue though, let me know if you are on it. I'm happy to help if you need anything

SteffenDE commented 3 weeks ago

I'll leave this open to wait for the Elixir changes as well.

josevalim commented 3 weeks ago

Fixed in https://github.com/elixir-lang/elixir/commit/a39f469a761520d89ea1b9fe0505a87efb6ce7f0.