react-final-form-arrays icon indicating copy to clipboard operation
react-final-form-arrays copied to clipboard

Calling .push() on click can result in too many fields added

Open papermana opened this issue 6 years ago • 5 comments

Are you submitting a bug report or a feature request?

This is a bug report.

What is the current behavior?

I cannot limit how many entries the user can push into an array. For instance, adding a conditional clause to a button's onClick handler doesn't always work. The problem appears when there is a high CPU utilization and several clicks can be made before a render.

I created a sandbox that shows off the problem. However, to actually see the issue you would probably need to use CPU throttling. Also, you may need to click like crazy. 😄

What is the expected behavior?

There should be a way to consistently limit how many fields can be added.

Sandbox Link

https://codesandbox.io/s/2zjl0w9l3n

What's your environment?

Just the "standard" one, like in the sandbox.

Other information

I assume the cause of this problem is that .push() is asynchronous. So multiple calls will get queued and, on a system with limited CPU power, may get executed before the next render gets finished. And the value of fields.length is only updated after render.

I'm not sure what the solution could be. Maybe provide something like an .update() method, which could be used like so:

onClick={() => fields.update(fields => {
  if (fields.length < 5) {
    fields.push('foo');
  }
})}

I can see that this issue is probably very niche. But it makes me uneasy that my users might get invalid UI (and/or any bugs that might come from e.g. sending too many fields to an API) just because they're installing an update on their computer at the same time or something.

It's also really not straightforward to get around this. For instance, the API doesn't provide a .slice() method for when we want to display only X entries.

papermana avatar Feb 13 '19 21:02 papermana

I also have problem with push my example used to work in v2 https://codesandbox.io/s/react-final-form-field-arrays-dehxx but after update to v3 it shows only 1 field instead of 5

TrejGun avatar Jun 20 '19 19:06 TrejGun

@TrejGun This is fixed by changing the dependency of your useEffect fn to just fields: https://codesandbox.io/s/react-final-form-field-arrays-onrfv

jvke avatar Dec 17 '19 00:12 jvke

do you mean

  useEffect(() => {
    if (fields.length < 5) {
      fields.push();
    }
  }, fields);

thsi results in error from react

React Hook useEffect was passed a dependency list that is not an array literal. This means we can't statically verify whether you've passed the correct dependencies. (react-hooks/exhaustive-deps)
eslint

because fields are not an array but array-like object

TrejGun avatar Dec 18 '19 02:12 TrejGun

@TrejGun That error is because the useEffect dependency argument expects an array, ie.

  useEffect(() => {
    if (fields.length < 5) {
      fields.push();
    }
  }, [fields]);

jvke avatar Dec 19 '19 23:12 jvke

When I use the useEffect function, it seems to push two into the array rather than one. Is there a way around this? componentDidMount doesn't seem to work as well.

Hyokune avatar Nov 09 '20 03:11 Hyokune