mhanberg / temple

An HTML DSL for Elixir and Phoenix
MIT License
484 stars 19 forks source link

Does it work with the latest Liveview ? #199

Closed mindreframer closed 1 year ago

mindreframer commented 1 year ago

Hey @mhanberg, I'm hitting this strange bug, what am I doing wrong here?

Describe the bug When trying to use it with the latest liveview, it fails.

# This fails
  def temple_component(assigns) do
    temple do
      div class: "border-2 border-emerald-500 rounded m-2 p-2" do
        slot @inner_block

        ul class: "list-disc pl-6" do
        end
      end
    end
  end

Backtrace:

== Compilation error in file lib/temple_liveview_web/live/comment_live/index.ex ==
** (CompileError) lib/temple_liveview_web/live/comment_live/index.ex:103: undefined variable "changed" (context Phoenix.LiveView.Engine)
    (elixir 1.14.2) expanding macro: Kernel.var!/2
    lib/temple_liveview_web/live/comment_live/index.ex:103: TempleLiveviewWeb.CommentLive.Index.temple_component/1
    (phoenix_live_view 0.18.18) expanding macro: Phoenix.Component.render_slot/2
    lib/temple_liveview_web/live/comment_live/index.ex:103: TempleLiveviewWeb.CommentLive.Index.temple_component/1
    (temple 0.11.0) expanding macro: Temple.Renderer.compile/1
    lib/temple_liveview_web/live/comment_live/index.ex:103: TempleLiveviewWeb.CommentLive.Index.temple_component/1
    (elixir 1.14.2) expanding macro: Kernel.then/2
    lib/temple_liveview_web/live/comment_live/index.ex:103: TempleLiveviewWeb.CommentLive.Index.temple_component/1

Expected behavior Should work

Desktop (please complete the following information):

mhanberg commented 1 year ago

I'll have to fool around with Phoenix 1.7 to see if there is anything in there causing any problems. I can reproduce the problem with your repo, but in my Phoenix 1.6 applications that run the latest Phoenix Live View, it seems to work as expected.

mindreframer commented 1 year ago

Yes, this would be great! And Phoenix 1.7 was a huuuge release, lots of updates there. Maybe it's something trivial, who knows. Thanks!

mindreframer commented 1 year ago

Hey @mhanberg, the issue seems to be this line:

  defmacro render_slot(slot, argument \\ nil) do
    quote do
      unquote(__MODULE__).__render_slot__(
        var!(changed, Phoenix.LiveView.Engine),
        unquote(slot),
        unquote(argument)
      )
    end
  end

And since Temple does not have the changed variable in the scope, it fails to compile. When I comment the unquote block out, compilation works. It seems to expect the changed var.

Also, I'm not so sure what var!(changed, Phoenix.LiveView.Engine), does, accessing a var within a context?

This is the code for var!

defmacro var!({name, meta, atom}, context) when is_atom(name) and is_atom(atom) do
    # Remove counter and force them to be vars
    meta = :lists.keydelete(:counter, 1, meta)
    meta = :lists.keystore(:if_undefined, 1, meta, {:if_undefined, :raise})

    case Macro.expand(context, __CALLER__) do
      context when is_atom(context) ->
        {name, meta, context}

      other ->
        raise ArgumentError,
              "expected var! context to expand to an atom, got: #{Macro.to_string(other)}"
    end
  end

Maybe you'll understand it better. And are you sure this works with Phoenix 1.6? It looks like a Liveview related issue to me here.

mindreframer commented 1 year ago

I have found this piece of code: https://github.com/elixir-lang/elixir/blob/d9967a1090df9cfe4b76b41e41244186946e3e13/lib/elixir/lib/kernel/special_forms.ex#L1033

  You cannot even access variables defined in the same module unless
  you explicitly give it a context:

      defmodule Hygiene do
        defmacro write do
          quote do
            a = 1
          end
        end

        defmacro read do
          quote do
            a
          end
        end
      end

      Hygiene.write()
      Hygiene.read()
      ** (RuntimeError) undefined variable a or undefined function a/0

  For such, you can explicitly pass the current module scope as
  argument:

      defmodule ContextHygiene do
        defmacro write do
          quote do
            var!(a, ContextHygiene) = 1
          end
        end

        defmacro read do
          quote do
            var!(a, ContextHygiene)
          end
        end
      end

      ContextHygiene.write()
      ContextHygiene.read()
      #=> 1

Hmmm... Is the variable stored within the module then? When I try the ContextHygiene.read() command first, it fails with the same message, as our failure

iex(3)> ContextHygiene.read
** (CompileError) iex:3: undefined variable "a" (context ContextHygiene)
    (elixir 1.14.2) expanding macro: Kernel.var!/2
    iex:3: (file)
    expanding macro: ContextHygiene.read/0
    iex:3: (file)

Does this mean, some code part should set the changed value on (inside?) the Phoenix.LiveView.Engine module prior to compilation of the Temple component / module using temple macro + using slots ?

I think it's a little confusing. Never knew, that one can store variables inside the module like this. It makes a module a stateful entity, that can change it's runtime behaviour based on data inside. Crazy.

Not sure if this helps, but maybe it does.

mindreframer commented 1 year ago

OK, when I set the changed value like this prior to calling the __render_slot__ function, it compiles.

  defmacro render_slot(slot, argument \\ nil) do
    quote do
      var!(changed, Phoenix.LiveView.Engine) = %{}
      # IO.inspect(unquote(slot), label: "render_slot:SLOT")
      # IO.inspect(unquote(argument), label: "render_slot:argument")
      unquote(__MODULE__).__render_slot__(
        var!(changed, Phoenix.LiveView.Engine),
        unquote(slot),
        unquote(argument)
      )
    end
  end

It means maybe, just maybe Temple is responsible to set this variable during template compilation, so that LiveView knows which assigns to track for changes?

mindreframer commented 1 year ago

But I do not see any other code setting this var like this, here is the search result from the deps folder:

$ ag -Q "var!"
phoenix/lib/phoenix/controller/pipeline.ex
93:      def action(var!(conn_before), opts) do
95:          var!(conn_after) = super(var!(conn_before), opts)
100:              var!(conn_before),
103:              var!(conn_before).private.phoenix_action,
109:      defp phoenix_controller_pipeline(unquote(conn), var!(action)) do
110:        var!(conn) = unquote(conn)
111:        var!(controller) = __MODULE__
112:        _ = var!(conn)
113:        _ = var!(controller)
114:        _ = var!(action)
122:    quote do: var!(conn_after)
127:      case var!(conn_after) do
129:        val -> plug.call(var!(conn_before), plug.init(val))
136:      case var!(conn_after) do
138:        val -> unquote(plug)(var!(conn_before), val)

phoenix/lib/phoenix/router.ex
362:      var!(add_resources, Phoenix.Router) = fn resource ->
610:              fn var!(conn, :conn), %{path_params: var!(path_params, :conn)} -> unquote(prepare) end,
977:      var!(add_resources, Phoenix.Router).(resource)

phoenix/lib/phoenix/endpoint.ex
408:      var!(config) = Phoenix.Endpoint.Supervisor.config(@otp_app, __MODULE__)
409:      var!(code_reloading?) = var!(config)[:code_reloader]
412:      _ = var!(code_reloading?)
471:      if force_ssl = Phoenix.Endpoint.__force_ssl__(__MODULE__, var!(config)) do
475:      if var!(config)[:debug_errors] do

phoenix/lib/phoenix/router/route.ex
125:        %{unquote_splicing(match_all)} = var!(conn, :conn)
126:        %{var!(conn, :conn) | unquote_splicing(merge_all)}
130:        var!(conn, :conn)

ecto/lib/ecto/query/builder/preload.ex
81:    idx = Builder.find_var!(var, vars)
88:    idx = Builder.find_var!(var, vars)

ecto/lib/ecto/query/builder/join.ex
78:    var   = Builder.find_var!(var, vars)

ecto/lib/ecto/query/builder.ex
492:    {escape_var!(var, vars), params_acc}
627:    var   = escape_var!(var, vars)
756:    do: {find_var!(var, vars), field}
759:    do: {find_var!(var, vars), field}
798:  @spec escape_var!(atom, Keyword.t) :: Macro.t
799:  def escape_var!(var, vars) do
800:    {:{}, [], [:&, [], [find_var!(var, vars)]]}
962:  def find_var!(var, vars) do
1115:    do: {find_var!(var, vars), field}
1119:    do: {find_var!(var, vars), field}
1127:    do: {find_var!(var, vars), code}

ecto/lib/ecto/query/builder/select.ex
109:    expr = Builder.escape_var!(var, vars)
110:    acc = add_take(acc, Builder.find_var!(var, vars), {tag, taken})

phoenix_template/lib/phoenix/template.ex
389:        def unquote(String.to_atom(name))(var!(assigns)) do
390:          _ = var!(assigns)

phoenix_html/lib/phoenix_html/engine.ex
170:      Phoenix.HTML.Engine.fetch_assign!(var!(assigns), unquote(name))

gettext/lib/gettext/compiler.ex
653:                var!(bindings)
680:          var!(bindings) = Map.put(bindings, :count, n)

plug/lib/plug/router.ex
470:          var!(conn),
471:          var!(glob),
584:      fn var!(conn), var!(opts) ->
585:        _ = var!(opts)

temple/lib/temple/component.ex
71:            _ = var!(assigns)
80:            _ = var!(assigns)

phoenix_live_view/lib/phoenix_live_view/renderer.ex
31:          def render(var!(assigns)) when is_map(var!(assigns)) do

phoenix_live_view/lib/phoenix_component.ex
937:      var!(changed, Phoenix.LiveView.Engine) = %{}
941:        var!(changed, Phoenix.LiveView.Engine),

phoenix_live_view/lib/phoenix_live_view/helpers.ex
220:        var!(changed, Phoenix.LiveView.Engine),

phoenix_live_view/lib/phoenix_live_view/tag_engine.ex
111:        var!(assigns) =
112:          unquote(__MODULE__).__assigns__(var!(assigns), unquote(key), parent_changed)
114:        _ = var!(assigns)
123:        var!(assigns) =
124:          unquote(__MODULE__).__assigns__(var!(assigns), unquote(key), parent_changed)
126:        _ = var!(assigns)

I only see my line (937) doing any changes on changed. I will give up for today.

mhanberg commented 1 year ago

I was getting some insane deja vu of this issue and then i did some reconnaissance #5 🤣

mindreframer commented 1 year ago

Yes, I was the first guy pushing you towards Liveview compatibility. 😆 This changed thing really irritates me somehow. I dont understand, how it's happening. Very strange...

mhanberg commented 1 year ago

I am currently on vacation but I'll get some time next week probably to look into it.

The error is hard to explain without explaining how live view and eex compilers work. But I do have an article explaining some of it if you'd like to read https://www.mitchellhanberg.com/how-eex-turns-your-template-into-html/

mindreframer commented 1 year ago

@mhanberg Sorry to bother on you your vacation! Enjoy it properly! I'm might look into it deeper tomorrow and thanks for the article, I're read it couple days ago 😄

Cheers and see you later!

mhanberg commented 1 year ago

@mindreframer turns out you just didn't configure temple to use the live view engine.

I opened a pr on your repro repo with the fix and a demonstration of it working.

https://github.com/mindreframer/temple_liveview/pull/1

mindreframer commented 1 year ago

@mhanberg That's awesome! Thank you! I've tried it and it works!

I have still one question related to dev experience with temple:

in VSC I have dialyzer warnings on every temple function.

Screenshot 2023-04-03 at 09 45 14 Screenshot 2023-04-03 at 09 45 52

Is there anything I have to configure for them to disappear or is this expected behaviour?

Thanks for looking into it and have a good Monday!

mindreframer commented 1 year ago

FYI: If I change the temple macro to look like this, the warning seems to be fixed:

  defmacro temple(block) do
    quote do
      require Temple.Renderer

      Temple.Renderer.compile(unquote(block))
      |> then(fn res ->
        case res do
          {:safe, template} ->
            template

          template ->
            template
        end
      end)
    end
  end
mindreframer commented 1 year ago

Nah, posted too early, the code above does not change a thing. But warnings supression works:

@dialyzer :no_match

Maybe this will help somebody.

mhanberg commented 1 year ago

I suspect you have to delete the .elixir_ls dir and restart VSCode.

The dialyzer warning is showing the return type of the normal phx emgine you had configured before.

mindreframer commented 1 year ago

@mhanberg nope, deleting .elixir_ls and restarting VSC did not improve the situation, the warnings appear again as soon the Elixir LS compilation finishes.

When I remove the condition for {:save, template} they also disappear.

  defmacro temple(block) do
    quote do
      require Temple.Renderer

      Temple.Renderer.compile(unquote(block))
      |> then(fn res ->
        case res do
         # {:safe, template} ->
         #  template

          template ->
            template
        end
      end)
    end
  end

And it makes sense, because the unquote(block) always returns a Phoenix.LiveView.Rendered struct, and that will never match the {:safe, template} condition.

Interestingly, the same issue was happening with the PageView module:

The default PageView module (which includes web/page/index.html.eex) will emit the warning: index.html.eex:1: The pattern {'safe', _@2} can never match the type binary()

Add the attribute below to suppress this.

web/views/page_view.ex

defmodule Myapp.PageView do
  use Chat.Web, :view
  @dialyzer :no_match
end

So yeah, this seems the way to go for now :)

mhanberg commented 1 year ago

Weird, I'll have to run dialyzer on a temple project to be sure.

Happy hacking!

mindreframer commented 1 year ago

Another, a bit cleaner approach, is to disable warnings in the temple macro:

defmacro temple(block) do
    Module.put_attribute(__CALLER__.module, :dialyzer, :no_match) # <---

    quote do
      require Temple.Renderer

      Temple.Renderer.compile(unquote(block))
      |> then(fn res ->
        case res do
          {:safe, template} ->
            template

          template ->
            template
        end
      end)
    end
  end

This removes the warnings consistently. Commenting this line out makes them appear again.