Add MacroComponent directives and Root Tag Attribute functionality

[green-david]

As discussed in #4114, this lays the foundation required for ColocatedCSS

Root Tag Attribute

This PR adds a new configuration parameter, :root_tag_attribute which defaults to phx-r which instructs the TagEngine to place an attribute on all "root" tags. "Root" tags in this context are tags at the outermost nesting level of the entire template or the contents provided to any component's inner block or named slots within the template. This is to lay the foundation of ColocatedCSS being able to use these root attributes as selectors to scope CSS. For example:

<div phx-r>
  <div></div>
</div>

MacroComponent Directives

This PR adds the concept of MacroComponent "directives" which can be returned from a new callback, directives/2, to instruct the TagEngine to perform manipulations that would normally be impossible outside of a MacroComponent itself during HEEx template compilation, primarily those involving other tags in the template.

The first supported directive is :root_tag_attribute which is added by this PR allows for adding additional root tag attributes. The plan is for ColocatedCSS to utilize this functionality to apply custom scopes to each component in order to scope CSS properly. For example:

<div phx-r phx-sample-one="test" phx-sample-two="test">
  <div></div>
</div>

[SteffenDE]

I think this should keep the existing root tag annotations, since an EEx nesting does not affect the tag structure. The existing root state is used to validate that live components have a single static root tag, so

<%= if ... do %>
  <div>...</div>
<% else %>
  ...
<% end %>

is not allowed, but for the macro component case, those nested tags would still need to be considered as root for scoped CSS to properly work.

Also, note that setting this to false means that right now this code crashes

<%= if true do %>
  <div>Inside do-block</div>
<% end %>

if root tag annotations are configured, as we try to Enum.map(false, ...) in line 1176.

[green-david]

I think this should keep the existing root tag annotations, since an EEx nesting does not affect the tag structure. The existing root state is used to validate that live components have a single static root tag, so

@SteffenDE Im in a little over my head here when it comes to the nesting stuff, I might need some additional clarification here.

I was attempting to pattern this off of the existing single root tracking logic as you mention, which sets a root of nil in handle_body and a root of false in handle_end. I was under the impression after reading the EEx.Engine docs and the existing set_root_on_tag/set_root_on_not_tag code that this was essentially to "skip" determining whether or not the template has a single-root when going through the nestings and instead wait until we process the entire body to do the single-root determination. Does that sound correct?

It seemed like during the nestings we wouldn't really care about applying any root tag annotations and could instead just wait until we go to handle_body and do it all in that one pass, so this was an attempt at indicating to "skip" applying root tag annotations when processing nestings so as to not apply them a second time when processing the body. But this may be a fundamental lack of understanding of how the whole EEx compilation lifecycle works how and when the tokens are processed.

Also, note that setting this to false means that right now this code crashes

<%= if true do %>
  <div>Inside do-block</div>
<% end %>

if root tag annotations are configured, as we try to Enum.map(false, ...) in line 1176.

Great find, thank you! I'll be sure to add tests specifically for nestings to cover this case and a few others to ensure these work.

[SteffenDE]

It seemed like during the nestings we wouldn't really care about applying any root tag annotations and could instead just wait until we go to handle_body and do it all in that one pass

It's been a while since I last worked on the tag engine and I'm always getting a bit confused by what is called when, but looking at this example:

defmodule Test do
  use Phoenix.Component

  def abc(assigns) do
    ~H"""
    <div>
      <p>
        <%= if @foo == :bar do %>
          <span>baz</span>
        <% end %>
      </p>
    </div>
    """
  end
end

and compiling it with a dbg call in both handle_end and handle_body

{:handle_end, state.tokens} #=> {:handle_end,
 [
   {:text, "\n    ", %{line_end: 10, column_end: 9}},
   {:close, :tag, "span",
    %{line: 9, column: 20, tag_name: "span", inner_location: {9, 20}}},
   {:text, "baz", %{line_end: 9, column_end: 20}},
   {:tag, "span", [],
    %{line: 9, column: 11, tag_name: "span", inner_location: {9, 17}}},
   {:text, "\n      ", %{line_end: 9, column_end: 11}}
 ]}

[lib/phoenix_live_view/tag_engine.ex:216: Phoenix.LiveView.TagEngine.handle_body/1]
{:handle_body, state.tokens} #=> {:handle_body,
 [
   {:text, "\n", %{line_end: 13, column_end: 5}},
   {:close, :tag, "div",
    %{line: 12, column: 5, tag_name: "div", inner_location: {12, 5}}},
   {:text, "\n", %{line_end: 12, column_end: 5}},
   {:close, :tag, "p",
    %{line: 11, column: 7, tag_name: "p", inner_location: {11, 7}}},
   {:text, "\n  ", %{line_end: 11, column_end: 7}},
   {:expr, "=",
    {:if, [line: 8, column: 13],
     [
       {:==, [line: 8, column: 21],
        [
          {:@, [line: 8, column: 16], [{:foo, [line: 8, column: 17], nil}]},
          :bar
        ]},
       [
         do: {:__block__, [live_rendered: true],
          [safe: ["\n      ", "<span", ">", "baz", "</span>", "\n    "]]}
       ]
     ]}},
   {:text, "\n    ", %{line_end: 8, column_end: 9}},
   {:tag, "p", [],
    %{line: 7, column: 7, tag_name: "p", inner_location: {7, 10}}},
   {:text, "\n  ", %{line_end: 7, column_end: 7}},
   {:tag, "div", [],
    %{line: 6, column: 5, tag_name: "div", inner_location: {6, 10}}}
 ]}

you can see (ignore that the tokens are in reverse order) that the nested expression is already compiled to a live_rendered block at the time handle_body runs. This means that we also need to apply the root annotation inside the nesting in case we're currently still at the root.

(As a tip if you want to reproduce this and explore the tag engine with logging: create a test.exs file in your LiveView repo with a module containing HEEx as shown above, then first compile LiveView with mix compile, clear your console, and then run the script with mix run test.exs to only see the logs from compiling that single module)

I was under the impression after reading the EEx.Engine docs and the existing set_root_on_tag/set_root_on_not_tag code that this was essentially to "skip" determining whether or not the template has a single-root when going through the nestings and instead wait until we process the entire body to do the single-root determination. Does that sound correct?

Now again, it's been a while, so this might not be 100% correct, but my understanding is that when we are in a nesting, we know that the template does not have a single static root tag, so we skip further root tracking (set_root_on_tag and set_root_on_not_tag become no-ops). Furthermore, the root flag is also used in the diff to allow the javascript to do some optimizations that only work for components with a single root tag. As the nesting is converted into its own rendered struct, that struct gets root: false.

We might need a better name here, since root can become a source of confusion here.

[green-david]

This helps tremendously and greatly improves my understanding of what is going on. The fact that the nesting is converted to an expression token by the time it makes it to handle_body makes a ton of sense. Thank you!! I will make changes here and add tests specifically for nestings 🙇‍♂️

[green-david]

So... @SteffenDE I spent a bit tonight working towards making these fixes and stumbled upon what appears to be a can of worms. 🙃

As you very helpfully pointed out, we need to apply the root tag annotations within the nesting logic (i.e. handle_end) as nestings are pre-compiled to live_rendered blocks by the time we make it to handle_body, so if we wait until handle_body its too late.

I got that working for the generic root attribute (phx-r) as we know that ahead of time, but for macrocomponent directives this poses what appears to be a bit of a chicken and egg problem...

We don't receive the macrocomponent directives for what additional root annotations to apply until we call transform/2, but that doesn't happen until they get processed in handle_body! so by the time we know what extra annotations to apply, we are too late to apply them. 😢

The only crackpot solution I have been able to theorize is somehow collecting and processing the macrocomponent directives ahead of time instead of at the end in handle_body - possibly by introducing some new callback to get the directives specifically? - but this would require us to "look ahead" as we are tokenizing the template (handle_text perhaps?) and somehow get the macrocomponent directives at that point so they are available by the time we start processing nestings...

I don't know if that is even feasible or all the ramifications of that, at minimum, I think it would mean that we need to traverse over some tokens twice, once in the "first pass" to look for macrocomponent directives ahead of time, and again in the "second pass" in the existing handle_body processing.

Let me know if any of that makes sense. I'm going to ruminate on it and see if I have any epiphanies.

[SteffenDE]

stumbled upon what appears to be a can of worms

I didn't think about this problem, but you identified it correctly. Is is a can of worms.

We could look at the tokens in handle_text. In case there's no interpolation (<%= ... %>) or other EEx expression involved, we would expect the full macro component opening and closing tag + content to be part of the tokens, but I'm not sure if I like that. As mentioned, I wanted (and still want to) make macro components work with more/all of HEEx (#3846), and this makes me feel like we should look into that more. If we had some well-defined intermediate representation of all of HEEx, modifying the full template in a second pass would be much easier to do (at least my feeling says so).

I'll be thinking about this more over the weekend.

[green-david]

Thanks for the thoughts and quick reply as always Steffen!

I started on a proof-of-concept of doing the first-pass exactly as you mention by catching the MacroComponents as they stream in in handle_text, but exactly as you have foreseen, if someone tries to (incorrectly) perform operations like expressions or interpolation in their MacroComponent it goes a bit haywire as we don't get the whole MacroComponent token set at once. Other than that it seems to be working (1 test failing which is specifically testing for invalid MacroComponent usage), but definitely feels like a much more elegant solution must be lurking somewhere...

[green-david]

@SteffenDE I went ahead and pushed changes to this PR which use the handle_text pre-pass approach to address the issue of applying the directives within nestings.

There is one failing test in test/phoenix_component/macro_component_integration_test.exs for raises when there is EEx inside which is failing as build_ast expects that for a macrocomponent that is malformed by having EEx in it that all of the tokens are available but some of them may just be invalid (in which case it can return a good error message by detecting an :expr token), but in the pre-pass if the macrocomponent is malformed by having EEx in it, we just don't have all of the tokens available to us and can't give as nice of an error message other than "your macrocomponent is malformed".

I don't expect we will ship this with this pre-pass workaround, but figured id throw it up anyways for you to see what it looks like and at least fix nestings and address the other PR comments.

[SteffenDE]

There's an issue with this: having multiple HTML attributes with the same name is invalid

<div phx-r phx-r="foo" phx-r="bar" />

@josevalim had an idea to call it root_tag_attributes instead and then do something like this

# default: no root_tag_attributes configured
config :phoenix_live_view, root_tag_attributes: [some_attribute: "some-value"]

Then, the configuration for phx-r would be

config :phoenix_live_view, :colocated_css, style_attribute: "phx-r"

And we'd add our style attribute to that list:

attrs = Application.get_env(:phoenix_live_view, :root_tag_attributes, [])

if style = Keyword.get(Application.get_env(:phoenix_live_view, :colocated_css, []), :style_attribute) do
  [{style, true} | attrs]
else
  attrs
end

Application.put_env(:phoenix_live_view, :root_tag_attributes, attrs)

I'm just not sure right now where that code would live.

Then the macro component would return

style_attribute = Application.get_env(:phoenix_live_view, :colocated_css, []) |> Keyword.get(:style_attribute, "phx-r")

{:ok, ..., root_tag_attributes: [{style_attribute, hash}]}

So we expect different macro components to use different attributes. The tag engine would then merge the attributes from the directive with the attributes from the config (attributes = Map.new(config_attrs ++ directive_attr)).

[green-david]

What about something like this?

config :phoenix_live_view, root_tag_attribute: "phx-r"

config :phoenix_live_view, :colocated_css, style_attribute: "phx-css"

style_attribute = Application.get_env(:phoenix_live_view, :colocated_css, []) |> Keyword.get(:style_attribute, "phx-r")

{:ok, ..., root_tag_attributes: [{style_attribute, hash}]}

This would mean that all root elements get phx-r but only root elements within components with a ColocatedCSS MacroComponent would additionally have phx-css="SCOPE_HERE". I think this would prevent us from having to "stuff" our own attribute into the config as you mention in the below example, while also providing us enough information to scope CSS correctly?:

attrs = Application.get_env(:phoenix_live_view, :root_tag_attributes, [])

if style = Keyword.get(Application.get_env(:phoenix_live_view, :colocated_css, []), :style_attribute) do
  [{style, true} | attrs]
else
  attrs
end

Application.put_env(:phoenix_live_view, :root_tag_attributes, attrs)

ColocatedCSS could then set a lower bound as any element which has phx-r but doesn't have phx-css="SCOPE_HERE", I think.

Maybe this provides a good middle-ground. Thoughts?

[SteffenDE]

That works for me! (cc @josevalim)

[josevalim]

The discussion @SteffenDE and I had were more from the point of view of adding feature feature. I think for now we only need:

config :phoenix_live_view, :colocated_css, style_attribute: "phx-css"

And we will initialize root_tag_attributes in the engine as:

if style = Keyword.get(Application.get_env(:phoenix_live_view, :colocated_css, []), :style_attribute) do
  [{style, true}]
else
  []
end

The :root_tag_attributes configuration will be "in case we need it", no need to add it right now.

[SteffenDE]

personally, I'd like to avoid css specific code in the tag engine if possible, but José can overrule me :D

[josevalim]

@SteffenDE unfortunately you may need to be overulled. Since this is happening at compile-time, I don't think we'd have an appropriate moment to copy the configuration from CSS to root_tag_attributes :( As there is no entry point. I don't think we even require the Phoenix LiveView to be started? Unless we can do it when we start the LivView compiler or similar...

[green-david]

With my proposed approach I think you don't have to copy over anything from CSS to root_tag_attributes (but I may be missing something):

this sets what attribute you want to be applied to all roots in all templates, or disable root attribute application entirely (which would mean you couldn't use ColocatedCSS, but your templates wouldn't be polluted by the extra attribute you don't use), and is used in the TagEngine:

config :phoenix_live_view, root_tag_attribute: "phx-r"

this configures what special attribute to additional apply for ColocatedCSS separately, and is only used in the ColocatedCSS transform/2 logic

config :phoenix_live_view, :colocated_css, style_attribute: "phx-css"

then the TagEngine knows to always apply root_tag_attribute from the app config to all root elements in the template, but if it encounters a ColocatedCSS macro component at the root level of the template which returns a root_tag_attribute: {"phx-css", scope_here} directive, it knows to also apply the phx-css attribute in addition to the generic phx-r attribute.

So the config is completely separate, and we know to inject the CSS specific attributes as we compile that specific template. The root_tag_attribute config is really to just let them change the generic one to a different tag name or disable it entirely.

WDYT? @josevalim

[josevalim]

You are conceptually correct and that's what we want. The downside of this approach is that it configures it twice. It would be nice if the users could opt-in by only configuring the style_attribute. But we can iron out this detail later. So we may need to overrule @SteffenDE for user convenience needs. :) but we can do it later!

[green-david]

Ah, that makes sense.

If user convenience is the goal, perhaps we could just default the root attribute to being applied as phx-r and the ColocatedCSS style attribute to phx-css and then let them opt in to different attribute names via adding config explicitly, or opt out of root attributes entirely via something like

config :phoenix_live_view, root_tag_attribute: false

🤔 🤔 🤔

but you are right, we can iron out the details later 😎 👍

[SteffenDE]

Since this is a more advanced topic, I'm not sure if we should document it in Phoenix.Component. Probably better in the macro component docs. Those are private right now, but I think that's fine. The main way users would interact with this would be through colocated CSS for now.

[green-david]

Fair point :) I was 50/50 on it when I was writing the docs as well. I ended up throwing it in in case they could be useful for some people as CSS or JS selectors, but I think it makes total sense since it is niche functionality to not immediately throw it at people in the Phoenix.Component docs!

[SteffenDE]

@green-david I created a draft PR to implement this based on the new compiler and added you as co-author. See the changes without tests here: 790e31c

Let me know what you think!

Closing in favor of #4127.