I was doing some coding on React, and encountered an issue I would like to properly deal with. Details on the matter are provided below.

The Environment

Suppose you have a component FormDemo made to handle a potentially complex form, parts of which involve the dynamic management of certain input fields. As an example, the provided code sample allows to create any amount of fields for names between 0 and (2³² - 1) fields due to JavaScript's limitations on array length.

Press Add New Name button above all name fields to append another name field. Press Remove button to the right of any input to delete it from the list.

Each name input created is handled by a separate component SubForm that takes three properties:

id: a unique generated identifier of the current field.
onChange: a function executing whenever the value of that input was changed.
onRemove: a function executing whenever the Remove button of that form was clicked.

The Sample

Here is a working sample of a code I've made on CodeSandbox provided for demonstration purposes.

The Problem

The approach used in the code sample works, but it has the eslint problem mentioned in Problems tab of CodeSandbox, and I am aware that it's not a CodeSandbox issue, as I've tested the same project in my local environment and got the same problem. Here are that problem's details taken right from the console:

React Hook useEffect has a missing dependency: 'onChange'. Either include it or remove the dependency array. If 'onChange' changes too often, find the parent component that defines it and wrap that definition in useCallback. (react-hooks/exhaustive-deps)

Following the advice from the problem directly (i.e. adding onChange to dependency list of SubForm's useEffect) results in infinite rendering, and thus is not a solution to the problem.

The Research

After some reading of the official React docs on useCallback, as well as the other part of these on useEffect, I've figured out that, when rendering a component, React creates new instances of functions declared in a component's body. Therefore, adding such functions to a dependency list of some useEffect hook that has an effect function attached to it will entail that function being called on each render.

In my approach, I pass update function to SubForm component in onChange property as a reference (proven here by React docs), hence the SubForm component's onChange property has exactly the same instance of the update function as the parent component. So, whenever the instance of the update function changes with it added to the dependencies of a useEffect hook, that executes the effect function attached to it, and, taking the above into account, this happens on each render of a parent component FormDemo.

The update function changes the value of forms, a state variable of FormDemo component, causing it to rerender. That recreates the instance of an update function. The SubForm component gets notified of that change and executes an effect function attached to a useEffect hook, calling the update function once again. In turn, this causes another change of a state variable forms, telling the parent component FormDemo to render again... and this continues indefinitely, creating an infinite loop of renders.

Some of you may ask why does it happen if an input field of the form was not changed between these two renders, thus the value passed to update function is effectively the same as before. After some testing, which you can try yourself here, I came to the conclusion that it's actually wrong: the value set to forms is always different. That's because even though the object's content is exactly the same, its instance is different, and React compares object instances instead of their contents, sending a command to rerender the component if these instances differ.

As useCallback hook memoizes the instance of the function between renders, recreating the function only when the values or instances of its dependencies change, I've assumed that wrapping update function in that hook will solve the original problem, because the instance of the function will always stay the same.

However, wrapping the update function in useCallback will result in another problem: I will need to add forms as a dependency, because I'm using it inside that function. But, taking the above into account, this will bring me the original problem back due to the instance of forms being different after each update, and that will command useCallback to recreate the instance of the function, too.

Potential Solution

With all that being said, I have a solution that I don't quite like, even though it works because it removes the need of adding the state variable forms to the list of dependencies of useCallback:

const update = useCallback((id, value) => {
  setForms(prevState => {
    const { form_list } = prevState,
          new_forms     = [...form_list],
          mod_id        = new_forms.map((e) => e.id).indexOf(id);

    new_forms[mod_id] = value;

    return { ...prevState, form_list: new_forms };
  });
}, []);

So why am I against it, if it works and gives no problems in the console?

In my humble opinion (feel free to prove me wrong), because of these issues:

Direct usage of state setter function instead of a dedicated middleware function. This decentralizes direct state management.
Duplication of an original array, which may be expensive on memory if an array has a lot of values inside, not to mention that each value itself is an object.

The Question

What is the most memory-efficient and readable solution of the stated problem in the provided case that will use a middleware function setField? Alternatively, if it's possible to debunk my issues with a potential solution, prove that it's the best way to go.

Feel free to modify the contents of setField if necessary for the solution and remember that I'm all open for answering anything related to the question.

CodePudding user response：

You probably want to separate the management of IDs from SubFrom. SubForm shouldn't be able to change it's ID.

Wrap the update & remove functions - so SubForm doesn't need to send the ID back.

  <SubForm key={e.id} id={e.id} 
          onChange={(form) => update(e.id, form)} 
          onRemove={() => remove(e.id) } />

Make sure that SubForm will not change ID as part of form

  const update = (id, value) => {
      setField(
        "form_list", 
        // subform shouldn't change id, so we overriding it (to be sure)
        forms.form_list.map(e => e.id===id?{...value, id}:e)
      );
  };

You still optionally may pass the ID to the SubForm, but the management of IDs is separated from it.

Modified code

CodePudding user response：

It seems you are duplicating state of each SubForm: you store it in parent and also in SubForm, why not store state only in parent and pass as props?

I am talking about something like this:

const SubForm = ({ id, form, onChange, onRemove }) => {
  return (
    <Form>
      <Form.Group controlId={`form_text${id}`}>
        <Form.Label>Name (ID {id})</Form.Label>
        <InputGroup>
          <Form.Control
            type="text"
            value={form.name}
            onChange={(e) => onChange(id, { ...form, name: e.target.value })}
          />
          <Button variant="danger" onClick={() => onRemove(id)}>
            Remove
          </Button>
        </InputGroup>
      </Form.Group>
      <br />
      <br />
    </Form>
  );
};

To pass each form data just do:

<SubForm key={e.id} form={e} id={e.id} onChange={update} onRemove={remove} />

No need for useEffect anymore.