Skip to content

Commit

Permalink
Warn if using <input name="id"> (phoenixframework#3269)
Browse files Browse the repository at this point in the history
* Warn if using `<input name="id">`

References: phoenixframework#3262

* add module + function, add test

* warn client-side if debug is enabled
  • Loading branch information
SteffenDE authored May 28, 2024
1 parent c644a4a commit 4c1e4c9
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 3 deletions.
10 changes: 9 additions & 1 deletion assets/js/phoenix_live_view/dom_patch.js
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,15 @@ export default class DOMPatch {
morph.call(this, targetContainer, html)
})

if(liveSocket.isDebugEnabled()){ detectDuplicateIds() }
if(liveSocket.isDebugEnabled()){
detectDuplicateIds()
// warn if there are any inputs named "id"
Array.from(document.querySelectorAll("input[name=id]")).forEach(node => {
if(node.form){
console.error("Detected an input with name=\"id\" inside a form! This will cause problems when patching the DOM.\n", node)
}
})
}

if(appendPrependUpdates.length > 0){
liveSocket.time("post-morph append/prepend restoration", () => {
Expand Down
42 changes: 40 additions & 2 deletions lib/phoenix_live_view/tag_engine.ex
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,7 @@ defmodule Phoenix.LiveView.TagEngine do
suffix = if closing == :void, do: ">", else: "></#{name}>"
attrs = remove_phx_no_break(attrs)
validate_phx_attrs!(attrs, tag_meta, state)
validate_tag_attrs!(attrs, tag_meta, state)

case pop_special_attrs!(attrs, tag_meta, state) do
{false, tag_meta, attrs} ->
Expand All @@ -736,6 +737,7 @@ defmodule Phoenix.LiveView.TagEngine do

defp handle_token({:tag, name, attrs, tag_meta} = token, state) do
validate_phx_attrs!(attrs, tag_meta, state)
validate_tag_attrs!(attrs, tag_meta, state)
attrs = remove_phx_no_break(attrs)

case pop_special_attrs!(attrs, tag_meta, state) do
Expand Down Expand Up @@ -1215,10 +1217,46 @@ defmodule Phoenix.LiveView.TagEngine do
List.keydelete(attrs, "phx-no-format", 0)
end

defp validate_tag_attrs!(attrs, %{tag_name: "input"}, state) do
# warn if using name="id" on an input
case Enum.find(attrs, &match?({"name", {:string, "id", _}, _}, &1)) do
{_name, _value, attr_meta} ->
# TODO: Remove conditional once we require Elixir v1.14+
meta =
if Version.match?(System.version(), ">= 1.14.0") do
[
line: attr_meta.line,
column: attr_meta.column,
file: state.file,
module: state.caller.module,
function: state.caller.function
]
else
Macro.Env.stacktrace(%{state.caller | line: attr_meta.line})
end

IO.warn(
"""
Setting the "name" attribute to "id" on an input tag overrides the ID of the corresponding form element.
This leads to unexpected behavior, especially when using LiveView, and is not recommended.
You should use a different value for the "name" attribute, e.g. "_id" and remap the value in the
corresponding handle_event/3 callback or controller.
""",
meta
)

_ -> :ok
end
end

defp validate_tag_attrs!(_attrs, _meta, _state), do: :ok

# Check if `phx-update` or `phx-hook` is present in attrs and raises in case
# there is no ID attribute set.
defp validate_phx_attrs!(attrs, meta, state),
do: validate_phx_attrs!(attrs, meta, state, nil, false)
defp validate_phx_attrs!(attrs, meta, state) do
validate_phx_attrs!(attrs, meta, state, nil, false)
end

defp validate_phx_attrs!([], meta, state, attr, false)
when attr in ["phx-update", "phx-hook"] do
Expand Down
11 changes: 11 additions & 0 deletions test/phoenix_live_view/html_engine_test.exs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
defmodule Phoenix.LiveView.HTMLEngineTest do
use ExUnit.Case, async: true

import ExUnit.CaptureIO

import Phoenix.Component

alias Phoenix.LiveView.Tokenizer.ParseError
Expand Down Expand Up @@ -1763,6 +1765,15 @@ defmodule Phoenix.LiveView.HTMLEngineTest do
""")
end)
end

test "warns when input has id as name" do
assert capture_io(:stderr, fn ->
eval("""
<input name="id" value="foo">
""")
end) =~
"Setting the \"name\" attribute to \"id\" on an input tag overrides the ID of the corresponding form element"
end
end

describe "handle errors in expressions" do
Expand Down

0 comments on commit 4c1e4c9

Please sign in to comment.