Remove duplicate stringify_keys in text_field_tag and number_field_tag

[claudiob]

All the methods that invoke text_field_tag (such as hidden_field_tag) do not need to call stringify_keys on their options parameter since the text_field_tag method is already doing it internally:

def text_field_tag(name, value = nil, options = {})
  tag :input, { "type" => "text", "name" => name, "id" => sanitize_to_id(name), "value" => value }.update(options.stringify_keys)
end

[Note 1: My code uses merge to respect the existing behavior of duplicating the options hash before updating its keys, see https://github.com//pull/17096#issuecomment-57223827]

[Note 2: My code uses symbols instead of strings (e.g.: :hidden) to look forward to future version of Ruby/Raiks (GC symbols); the result of the method, however, is the same, because the symbols are stringified inside text_field_tag]

[Note 3: I had previously created a similar PR #17096 but decided to split it into multiple PRs given the feedback received in the comments]

[rafaelfranca]

If the change is the same but in different methods it is better to keep in
one commit.
On Oct 15, 2014 1:00 PM, "Claudio B." notifications@github.com wrote:

All the methods that invoke text_field_tag (such as hidden_field_tag) do
not need to call stringify_keys on their options parameter since the
text_field_tag method is already doing it internally
https://github.com/claudiob/rails/blob/4159134524f4c78d008eef9d9a17f73a3172dcc8/actionview/lib/action_view/helpers/form_tag_helper.rb#L182
:

def text_field_tag(name, value = nil, options = {})
tag :input, { "type" => "text", "name" => name, "id" => sanitize_to_id(name), "value" => value }.update(options.stringify_keys)end

[Note 1: My code uses merge to respect the existing behavior of
duplicating the options hash before updating its keys, see #17096
(comment)
https://github.com//pull/17096#issuecomment-57223827]

[Note 2: My code uses symbols instead of strings (e.g.: :hidden) to look
forward to future version of Ruby/Raiks (GC symbols); the result of the
method, however, is the same, because the symbols are stringified inside
text_field_tag]

[Note 3: I had previously created a similar PR #17096
https://github.com//pull/17096 but decided to split it into

multiple PRs given the feedback received in the comments]

You can merge this Pull Request by running

git pull https://github.com/claudiob/rails remove-duplicate-stringify-keys

Or view, comment on, or merge it at:

#17271
Commit Summary

• Remove duplicate stringify_keys in text_field_tag

File Changes

• M actionview/lib/action_view/helpers/form_tag_helper.rb
  https://github.com/rails/rails/pull/17271/files#diff-0 (24)

Patch Links:

• https://github.com/rails/rails/pull/17271.patch
• https://github.com/rails/rails/pull/17271.diff

—
Reply to this email directly or view it on GitHub
#17271.

[rafaelfranca]

I don't like the idea of having two type keys in the hash later. It just work because Hashes are now ordered by insertion order in Ruby. This may not change in the future but we don't have any guarantee that Ruby will not change this behaviour again.

[claudiob]

@rafaelfranca – yes, that was my point: Hashes are now ordered by insertion order in Ruby.

That is the rationale behind the PR.
If you fear that might change in the future, then you can close this PR.
I was just trying to harness the power of the most recent versions of Ruby that Rails supports and will support 😸

[rafaelfranca]

I guess I may be too afraid about it. I'll merge but I prefer to include #17272 in the same commit.

[claudiob]

@rafaelfranca – ok, I'll merge!

[rafaelfranca]

Could you squash your commits?

[claudiob]

@rafaelfranca Squashed!

---

On a side note, compared to my old PR, what I removed is dbf2dbe because those lines of code actually change the behavior of the methods file_field_tag and password_field_tag.

Currently, these two helpers are different from all the other ones in that they modify the options hash passed as a parameter, whereas all the other helpers duplicate options before updating it.

Therefore, if I use text_field_tag before file_field_tag:

<% options = {class: 'important'} %>
<%= text_field_tag 'Name', options %>
<%= file_field_tag 'Upload', options %>

I correctly get a text field and a file field:

<input class="important" id="Name" name="Name" type="text" value="value">
<input class="important" id="Upload" name="Upload" type="file">

but if I use file_field_tag before text_field_tag:

<% options = {class: 'important'} %>
<%= file_field_tag 'Upload', options %>
<%= text_field_tag 'Name', options %>

then I get two file fields!

<input class="important" id="Upload" name="Upload" type="file">
<input class="important" id="Name" name="Name" type="file" value="value">

I think this is a bug… if you agree I can make a new PR with that commit to fix it! What do you think?

[rafaelfranca]

Yes. It is a bug. Lets fix it.