Skip to content

Commit 7ad896e

Browse files
authored
Raise if JS.dispatch detail is not a map (#4062)
Today I discovered a bug in a LiveView application where a string was passed as the detail option to JS.dispatch instead of a map (as documented). It turns out it accidentally worked with the default `esbuild` config. The bug remained unnoticed for a long time because by default `app.js` is not compiled in JavaScript strict mode. When the config was changed to output ESM modules (which are always in strict mode), the error surfaced. The exact line of code in Phoenix LiveView client code where the error is raised: https://github.com/phoenixframework/phoenix_live_view/blob/98088bc7458d1821d9d9bb38caa3145a0b93ee9c/assets/js/phoenix_live_view/js.js#L77 Note that line has been there for many years. This is the explanation of the error in MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Cant_assign_to_property LiveView code is correct and follows the documented API. Since we already do validations and raise on other invalid inputs, we can prevent future mistakes by ensuring that the detail option is a map.
1 parent 7654124 commit 7ad896e

File tree

2 files changed

+12
-1
lines changed

2 files changed

+12
-1
lines changed

lib/phoenix_live_view/js.ex

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,9 +366,14 @@ defmodule Phoenix.LiveView.JS do
366366
})
367367
"""
368368

369-
{_, {:ok, detail}} ->
369+
{_, {:ok, detail}} when is_map(detail) ->
370370
Keyword.put(args, :detail, detail)
371371

372+
{_, {:ok, detail}} ->
373+
raise ArgumentError, """
374+
the detail option to JS.dispatch must be a map, got: #{inspect(detail)}
375+
"""
376+
372377
{_, :error} ->
373378
args
374379
end

test/phoenix_live_view/js_test.exs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,12 @@ defmodule Phoenix.LiveView.JSTest do
483483
JS.dispatch("foo", detail: %{done: true}, blocking: true)
484484
end
485485
end
486+
487+
test "raises when detail is not a map" do
488+
assert_raise ArgumentError, ~r/the detail option to JS.dispatch must be a map/, fn ->
489+
JS.dispatch("foo", detail: "not-a-map")
490+
end
491+
end
486492
end
487493

488494
describe "toggle" do

0 commit comments

Comments
 (0)