Allow keyword list options to use nil as key and/or value (#456)

phoenixframework · Jan 23, 2025 · 9007635 · 9007635
1 parent df2a3f6
commit 9007635
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 15 deletions.
diff --git a/lib/phoenix_html/form.ex b/lib/phoenix_html/form.ex
@@ -334,17 +334,25 @@ defmodule Phoenix.HTML.Form do
         [acc | option(option_key, option_value, [], selected_values)]
 
       options, acc when is_list(options) ->
-        {option_key, options} = Keyword.pop(options, :key)
-
-        option_key ||
-          raise ArgumentError,
-                "expected :key key when building <option> from keyword list: #{inspect(options)}"
-
-        {option_value, options} = Keyword.pop(options, :value)
-
-        option_value ||
-          raise ArgumentError,
-                "expected :value key when building <option> from keyword list: #{inspect(options)}"
+        {option_key, options} =
+          case List.keytake(options, :key, 0) do
+            nil ->
+              raise ArgumentError,
+                    "expected :key key when building <option> from keyword list: #{inspect(options)}"
+
+            {{:key, key}, options} ->
+              {key, options}
+          end
+
+        {option_value, options} =
+          case List.keytake(options, :value, 0) do
+            nil ->
+              raise ArgumentError,
+                    "expected :value key when building <option> from keyword list: #{inspect(options)}"
+
+            {{:value, value}, options} ->
+              {value, options}
+          end
 
         [acc | option(option_key, option_value, options, selected_values)]
 

diff --git a/test/phoenix_html/form_test.exs b/test/phoenix_html/form_test.exs
@@ -248,9 +248,10 @@ defmodule Phoenix.HTML.FormTest do
 
   describe "options_for_select/2" do
     test "simple" do
-      assert options_for_select(~w(value novalue), "novalue") |> safe_to_string() ==
+      assert options_for_select(["value", "novalue", nil], "novalue") |> safe_to_string() ==
                ~s(<option value="value">value</option>) <>
-                 ~s(<option selected value="novalue">novalue</option>)
+                 ~s(<option selected value="novalue">novalue</option>) <>
+                 ~s(<option value=""></option>)
 
       assert options_for_select(["value", :hr, "novalue"], "novalue") |> safe_to_string() ==
                ~s(<option value="value">value</option>) <>
@@ -261,14 +262,16 @@ defmodule Phoenix.HTML.FormTest do
                [
                  [value: "value", key: "Value", disabled: true],
                  :hr,
-                 [value: "novalue", key: "No Value"]
+                 [value: "novalue", key: "No Value"],
+                 [value: nil, key: nil]
                ],
                "novalue"
              )
              |> safe_to_string() ==
                ~s(<option disabled value="value">Value</option>) <>
                  ~s(<hr/>) <>
-                 ~s(<option selected value="novalue">No Value</option>)
+                 ~s(<option selected value="novalue">No Value</option>) <>
+                 ~s(<option value=""></option>)
 
       assert options_for_select(~w(value novalue), ["value", "novalue"]) |> safe_to_string() ==
                ~s(<option selected value="value">value</option>) <>