phx-value-* shows inconsistent behaviour with LiveView

[nduitz]

Describe the bug

After upgrading surface from 0.5 to 0.6 phx-value-* bindings changed in how they behaved. This is not documented in the changelog's breaking changes and I cant find the commit that introduced the behavioural change.

How to reproduce it

On Surface 0.5 this matches:

<button :on-click={"some_event"} phx-value-some_value={"foo"} ></button>

def handle_event("some_event", %{"some_value" => some_value}, socket)

On Surface 0.6 this doesn't match because some_value is now some-value

The behavior you expected

LiveView itself does not change the variable name. It is ok to do so but this should be documented then in a prominent place and should be mentioned as breaking change.

Your Environment

Surface: v0.6.0
LiveView: v0.16.4
Elixir: v1.12.3

[dvic]

Is there a difference in the HTML Surface produces (0.5 vs 0.6)?

[nduitz]

Is there a difference in the HTML Surface produces (0.5 vs 0.6)?

yes 0.5 would render phx-value-some_value where 0.6 would render phx-value-some-value

[objectuser]

Same issue here. I wanted to note that if the value of phx-value-* is an expression (i.e., {...}) then the underscores are replaced with dashes. But if it's a constant, e.g., "foo", then the underscores are maintained.

[msaraiva]

I can confirm it's a regression. Working on the fix now.

[msaraiva]

This has been fixed in 5c53916.

I'll release v0.6.1 later today with this fix. I the meantime, feel free to use master and let me know if you still find any issue.

Thanks!

[hubertlepicki]

@msaraiva there's another case where this is still happening, and also on v0.6.1.

I am using <Link> component, which then under the hood passes attributes using :attrs to <a> tag.

In essence, these two lines have different output:

          <a href="#" :attrs={["phx-value-order_column": "bar"]}>ELO</a>
          <a href="#" phx-value-order_column="bar">ELO</a>

which are rendered to, respectively:

      <a phx-value-order-column="bar" href="#">ELO</a>
      <a href="#" phx-value-order_column="bar">ELO</a>

As you can see above, one changes underscore to dashes, the other does not. This is happening when using Link component but as you can also see above when using :attrs directly.

[hubertlepicki]

I was poking around the source code and I am unsure how this should be fixed, and if at all. The behavior currently is confusing and ideally I'd like to see something like this instead:

  test "literal attribute names in camel_case should not be translated to snake-case" do
    expected = ~S(<button phx-value-my_value="foo"></button>)

    html = render_surface(do: ~F[<button phx-value-my_value="foo"/>])
    assert html =~ expected

    html = render_surface(do: ~F[<button phx-value-my_value={"foo"}/>])
    assert html =~ expected

    html = render_surface(do: ~F[<button phx-value-my_value={String.downcase("FOO")}/>])
    assert html =~ expected
+
+    html = render_surface(do: ~F[<button {... "phx-value-my_value": "foo"}/>])
+    assert html =~ expected
+
+    html = render_surface(do: ~F[<button :attrs={%{"phx-value-my_value": "foo"}}/>]) 
+    assert html =~ expected
+  end

But I am unsure how to do it without special handling for the attribute that has name of :"phx-vallue-" or :"phx_value_".

@msaraiva I can try implementing the above but do you think the above behavior is desired, plus are you ok with special case for "phx-value-*" attribute?

[hubertlepicki]

I had another pass at the issue and what seems to be the easiest solution for me is to switch to using String as the key, and map instead of keyword keys when passing the "phx-value-" attribute name, and it seems to work as expected when used directly:

<button {... %{"phx-value-my_value" => "foo"}}/>

and

<button :attrs={%{"phx-value-my_value" => "foo"}}/>

both preserve underscore, because of how that's handled in here

The problem still remains when using with pre-defined components. They take opts as :keyword, so we cannot pass a map there.

which would be also acceptable solution for now.

I see now two possibilities to solve this problem:

1. Add special handling for :"phx-value-*" atoms, that would preserve whatever is there as * without replacing _ with -

2. Alter the pre-defined components, and possibly the Surface prop system, so it accepts both maps as keyword list as arguments...

Or maybe something entirely different. @msaraiva your thoughts would be appreciated.

[msaraiva]

Hi @hubertlepicki!

I agreed this is confusing and to be honest, I think the best option would be to never do any conversion, however this is the way how Phoenix/LV handles attributes so it's better to keep it consistent.

I see now two possibilities to solve this problem:

1. Add special handling for :"phx-value-*" atoms, that would preserve whatever is there as * without replacing _ with -
2. Alter the pre-defined components, and possibly the Surface prop system, so it accepts both maps as keyword list as arguments...

I guess #2 seems to be the best option we have. I mean, by default, we already accept and convert map values passed as keyword lists so we could either also accept keyword values passed as maps or define a :keyword_or_map type for those cases.

[hubertlepicki]

@msaraiva sorry for the late response, I had some sick downtime last week, I agree. I'll add a feature-request and possibly start working on it.

[Coffei]

There seems to be a regression in version 0.7, see #562.