Good morning!
I thought about it a bit further, I think to_string/1 as in the PR is indeed a good contract (but see Option 1 vs Option 2 below).
When we write a JS command inside a HEEx template, we’re intuitively serializing it as a string in an HTML attribute (with proper escaping taken care of for us).
The new use case we’re unlocking, push_event with a JS command in the payload, requires a JSON-serializable payload. A string is JSON-serializable, and so is a map with a string value.
On the client side, LiveSocket.execJS expects an “encoded JS command” as argument. “Encoded” in this context means encoded as a string (which happens to be a JSON serialization of a list, but that is not part of the public contract, and could be anything).
If not implementing String.Chars / to_string, the alternative would be some explicit function, like the to_json mentioned earlier. The downside of JS.to_json is that it prescribes the serialization format. JS.encode/1 (taking a %JS{} as input, returning binary) would keep the underlying serialization format opaque, and is inline with the “encoded JS” terminology.
I understand implementing String.Chars can be convenient, examples:
iex(1)> alias Phoenix.LiveView.JS
Phoenix.LiveView.JS
iex(2)> JS.transition({"ease-out duration-200", "opacity-0", "opacity-100"}) |> IO.puts()
[["transition",{"transition":[["ease-out","duration-200"],["opacity-0"],["opacity-100"]]}]]
:ok
iex(3)> "#{JS.transition({"ease-out duration-200", "opacity-0", "opacity-100"})}"
"[[\"transition\",{\"transition\":[[\"ease-out\",\"duration-200\"],[\"opacity-0\"],[\"opacity-100\"]]}]]"
A potential downside is having the protocol implementation trigger unexpected behavior, e.g. serializing the JS command in unexpected contexts instead of throwing or failing to type-check at compile time. In this case, the explicit function would be, well, explicit, though less convenient.
So I think it comes down to deciding whether implementing String.Chars has any unintended consequences, and how we’d document the push_event use case.
Option 1: String.Chars
socket
|> push_event("exec", %{
"selector" => "#urls-#{url.id}",
"js" => "#{JS.transition({"ease-out duration-200", "opacity-0 scale-95", "opacity-100 scale-100"})}"
})
Option 2: JS.encode/1 (or similar)
socket
|> push_event("exec", %{
"selector" => "#urls-#{url.id}",
"js" =>
JS.transition({"ease-out duration-200", "opacity-0 scale-95", "opacity-100 scale-100"})
|> JS.encode()
})
- Would be documented at the function level.
- Accidentally dropping a
%JS{} inside a string (i.e. not HEEx) would still produce an error as is the current behavior.
In my current thinking, I prefer Option 2 because it preserves existing behavior, is more discoverable, doesn’t impede us from implementing String.Chars in the future and is inline with what @steffend said:
@steffend I think user code shouldn’t need to implement Jason.Encoder for JS commands. It is easier to reason about passing an opaque string (“encoded JS”) to push_event.
