Wrong input name parsing

[strelok1111]

Bug Report

When form input name contents integer value as key, StimulusReflex inserts nil in params as many times as the key number is. And if the number is big enough, the browser hangs.

To Reproduce

<form>
  <input name="foo[100][bar]" />
  <a data-reflex="click->TestReflex#test" data-reflex-root='#bar'>click me</>
  <div id="bar"></div>
</form>

TestReflex

def test
  morph '#bar', params.inspect
end

When I click <a> element, I get this in <div id="bar"></div> element:

<:parameters>"page", "action"=>"index", "foo"=>[nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, {"bar"=>"123"}]} permitted: 1>

If I change field name to foo[q_100][bar] everything works as expected:

<:parameters>"page", "action"=>"index", "foo"=>{"q_100"=>{"bar"=>"123"}}} permitted: 1>

Expected behavior

<:parameters>"page", "action"=>"index", "foo"=>{"100"=>{"bar"=>"123"}}} permitted: 1>

Versions

StimulusReflex

• Gem: 3.3.0
• Node package: 3.3.0

External tools

• Ruby: 2.6.3
• Rails: 5.2.4.3
• Node: 10.21.0

Browser

• Browser chrome, firefox
• Version 80.0.3987.149, 81.0

[marcoroth]

Hey @strelok1111! Thanks for reporting this bug. I was able to reproduce your issue in my app too.

For me it breaks at <input name="foo[10000][bar]" />.

The issue is caused when we serialize the form data with the serializeForm() function here:

https://github.com/hopsoft/stimulus_reflex/blob/95a9998f321445a5b879f95cf51405570a04e48b/javascript/stimulus_reflex.js#L179-L185

I'm wondering if this really is an issue though. In theory you are using an index and with that we have to fill the array with nil/null values in order to place your value at the right position in this "array".

But either way, I assume this issue needs to be addressed in the form-serialize library then.

[leastbad]

Interesting find, but ultimately Marco is right - you're getting what you're specifying.

What are you doing that you have to use what looks like an array index in your name, if you don't have an array?

[strelok1111]

When rails create new nested objects on the form it uses timestamp as key for new objects, and ids for existing objects as keys.

[marcoroth]

Ah I see!

Example from https://guides.rubyonrails.org/form_helpers.html#using-form-helpers

<%= form_with model: @person do |person_form| %>
  <%= person_form.text_field :name %>
  <% @person.addresses.each do |address| %>
    <%= person_form.fields_for address, index: address.id do |address_form| %>
      <%= address_form.text_field :city %>
    <% end %>
  <% end %>
<% end %>

Which results in:

<form accept-charset="UTF-8" action="/people/1" data-remote="true" method="post">
  <input name="_method" type="hidden" value="patch" />
  <input id="person_name" name="person[name]" type="text" />
  <input id="person_address_23_city" name="person[address][23][city]" type="text" />
  <input id="person_address_45_city" name="person[address][45][city]" type="text" />
</form>

Even though you can specify the index: if think we have to build something around this.

Is there any reason we couldn't just interpret every number as string in the serialization?

Rails handles this similarly if you submit the form via http:

{'person' => {'name' => 'Bob', 'address' => {'23' => {'city' => 'Paris'}, '45' => {'city' => 'London'}}}}

[leastbad]

I'm out of my experience/comfort zone on this (converting numbers to strings sounds fine, but I am not qualified to say if it's bad) but it goes without saying that if we can easily patch this, we will.

It might also be possible to look at https://www.npmjs.com/package/form-serialize#indexed-arrays or creating a custom serializer?

[strelok1111]

Is there any reason we couldn't just interpret every number as string in the serialization?

form-serialize has not any options to do that. I'll try to write feature request to https://github.com/defunctzombie/form-serialize maybe something like index_as_key option

[strelok1111]

It might also be possible to look at https://www.npmjs.com/package/form-serialize#indexed-arrays or creating a custom serializer?

I don't think this is a good solution, because we need to copy almost whole form-serialize.

But last commit in form-serialize was 3 years ago... I think it abandoned.

[RolandStuder]

Maybe then it might be better to ditch form-serialize and use what UJS is using:

Either copy it or depend on it: here is what they use for form serialization.

https://github.com/rails/rails/blob/8f37e2555a7f74e754026e7dc9e36908f7a789ff/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee#L7

Here is a codepen with the extracted coffeescript, as you see they create an encoded string rather than a hash: https://codepen.io/RolandStuder/pen/oNLNWKZ

[marcoroth]

serializeForm() also creates an encoded string if you pass no options. I assume we are passing thehash: true option so we can merge them with the existing params.

[leastbad]

Shamelessly quoting myself on Discord:

So Obie, Jared and Nate are all excited about Shoelace, which means we should make sure that SR supports it and things like it subtext here is that if we/you can help make sure the eventual solution can work on forms or form-like structures, that makes SR better for everyone For example, there could be a mechanism where we introduce a data attribute that marks a non-native form element as a form so that we can force whatever serializer to pretend it's a form