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

Manually calling field.push generates 2 elements in FieldArray

Open JonatanSalas opened this issue 5 years ago • 9 comments

Are you submitting a bug report or a feature request?

I'm submitting a bug report.

What is the current behavior?

FieldArray initializes empty, if I manually call fields.push("") only when fields.length is zero, it should add only one element, but, it's adding 2 elements.

Here is a codesandbox to show the behaviour: https://codesandbox.io/s/react-final-form-arrays-bug-cww5f

What is the expected behavior?

If I call fields.push("") validating that fields.length is zero, it should add only one element to the array, not two.

Sandbox Link

https://codesandbox.io/s/react-final-form-arrays-bug-cww5f

What's your environment?

  • React Final Form Arrays: v3.1.1

  • React Final Form: v.6.3.0

  • Final Form Arrays: v3.0.1

  • Final Form: v4.18.5

  • OS: Mac OS Mojave v10.14.6

  • Browser: Chrome/Firefox/Safari

  • Node Version: v11.12.0

Other information

I have done a little of debugging and it seems an issue related to the function changeValue injected by the mutator. When I call fields.push("") for first time, it does not change the value, because it doesn't change the value, in the second iteration the array length is still zero.

In the second iteration, it seems that the value is correctly updated, but, because it has been called two times to produce an update, the array now has two elements.

JonatanSalas avatar Oct 13 '19 17:10 JonatanSalas

For now I managed to solve the issue with the following work-around:

import React, { useState, useEffect } from 'react';
import { FieldArray } from 'react-final-form-arrays';

const InnerFieldArray = ({ fields, children }) => {
  const [isFirstIteration, setFirstIteration] = useState(false);

  useEffect(() => {
    if (!isFirstIteration && fields.length === 0) {
      setFirstIteration(true);
      fields.push({});
    }
  }, []);

  return children({ fields });
};

const CustomFieldArray = ({ children, ...rest }) => (
  <FieldArray {...rest}>
    {props => <InnerFieldArray {...props}>{children}</InnerFieldArray>}
  </FieldArray>
);

CustomFieldArray.displayName = 'CustomFieldArray';

export default CustomFieldArray;

JonatanSalas avatar Oct 13 '19 17:10 JonatanSalas

First of all, field.push should be considered a side effect, it should be wrapped in a useEffect, Doing mutations in the render method is highly not recommended (I'm talking about what you did in CodeSandbox, your example here should be less of an issue, but still, wrapping the logic in useEffect make your intentions clearer).

Secondly, I think your issue is related to this one #116

My discoveries is that there is a desynchronization between the real form state, and the array one, that's why, during 1 render just after the push, fields.length is still 0

edit : You can test the length with the real form state instead => form.values[name].length

myagoo avatar Oct 14 '19 12:10 myagoo

Yes, I know it's a side effect, in the original use case, I have wrapped fields.push("") with useEffect hook.

Interesting that accessing the real form state gives the correct length. Nice catch.

JonatanSalas avatar Oct 14 '19 18:10 JonatanSalas

For now I managed to solve the issue with the following work-around:

import React, { useState, useEffect } from 'react';
import { FieldArray } from 'react-final-form-arrays';

const InnerFieldArray = ({ fields, children }) => {
  const [isFirstIteration, setFirstIteration] = useState(false);

  useEffect(() => {
    if (!isFirstIteration && fields.length === 0) {
      setFirstIteration(true);
      fields.push({});
    }
  }, []);

  return children({ fields });
};

const CustomFieldArray = ({ children, ...rest }) => (
  <FieldArray {...rest}>
    {props => <InnerFieldArray {...props}>{children}</InnerFieldArray>}
  </FieldArray>
);

CustomFieldArray.displayName = 'CustomFieldArray';

export default CustomFieldArray;

I've tried doing the same thing by creating a state and checking the state in the useEffect but it doesn't render again to display the the mapped children.

Is there no better way to initialise the FieldArray (and nested FieldArrays) so there is always one element on render? I just can't seem to get the useEffect to work so it only pushed one item into the array.

Hyokune avatar Nov 10 '20 08:11 Hyokune

For now I managed to solve the issue with the following work-around:

import React, { useState, useEffect } from 'react';
import { FieldArray } from 'react-final-form-arrays';

const InnerFieldArray = ({ fields, children }) => {
  const [isFirstIteration, setFirstIteration] = useState(false);

  useEffect(() => {
    if (!isFirstIteration && fields.length === 0) {
      setFirstIteration(true);
      fields.push({});
    }
  }, []);

  return children({ fields });
};

const CustomFieldArray = ({ children, ...rest }) => (
  <FieldArray {...rest}>
    {props => <InnerFieldArray {...props}>{children}</InnerFieldArray>}
  </FieldArray>
);

CustomFieldArray.displayName = 'CustomFieldArray';

export default CustomFieldArray;

I've tried doing the same thing by creating a state and checking the state in the useEffect but it doesn't render again to display the the mapped children.

Is there no better way to initialise the FieldArray (and nested FieldArrays) so there is always one element on render? I just can't seem to get the useEffect to work so it only pushed one item into the array.

@Hyokune - any luck with this? I'm working on the same and haven't found a nice solution

mischa-s avatar Nov 22 '20 21:11 mischa-s

@mischa-s Currently it seems like the only way to initialise an array is by using the useEffect. I found out that the reason why useEffect wasn't working for me was because the form was causing the array component to re-render hence the useEffect called twice with the same fields prop (causing it to push two elements into the array due to fields.length being 0).

useEffect(() => {
    if (fields.length !== undefined && fields.length === 0) {
      fields.push({} as ArrayType)
    }
}, [fields])

But I was able to eventually fix this issue by setting the subscription prop on my form (subscription={{}}) and on all of my fields to prevent the form from doing unnecessary renders, so you might be able to fix your issue with the same way.

Hyokune avatar Nov 22 '20 21:11 Hyokune

Thanks for the quick response @Hyokune!

In the end I have decided to setup a default instance of the field on InitialValues as this seemed like the simplest solution:

  const initialValues = {
    [fieldName]: [
      {
        value1: '',
        value2: ''
      }
    ]
  };
...
    <Form
        onSubmit={onSubmit}
        initialValues={initialValues}
        mutators={{
            ...arrayMutators
        }}

mischa-s avatar Nov 23 '20 00:11 mischa-s

Thanks for the quick response @Hyokune!

In the end I have decided to setup a default instance of the field on InitialValues as this seemed like the simplest solution:

  const initialValues = {
    [fieldName]: [
      {
        value1: '',
        value2: ''
      }
    ]
  };
...
    <Form
        onSubmit={onSubmit}
        initialValues={initialValues}
        mutators={{
            ...arrayMutators
        }}

@mischa-s Just a note when using initialValues, I initially did it the same way but came to an issue when using nested dynamic arrays as you can only initialise the first element (unless you want all the array fields to show) so had to switch over to figure out using the useEffect hook.

Hyokune avatar Nov 23 '20 21:11 Hyokune

So is this going to get fixed?

MikeTaylor avatar Nov 02 '23 20:11 MikeTaylor