Text entered into an input-field is being duplicated to the state of another input field's value - why?

Question

As you can see in below screencast, manipulating the second input field in my form also changes the value of the first one. I've completely reproduced it on JSfiddle in an isolated environment to track down the bug but didn't find it.

Quick overview of my code base:

A functional component ModuleBuilder has an Array in its state called blocks and a renderFunction which renders for each element of blocks one instance of a subcomponent called ModuleElement.

The subcomponent ModuleElement displays an input form element. When I create n instances of the subcomponent and try to type something in the the form element of the second instance then the first one's value gets updated as well.

const ModuleElement = (props) => {

  const blockElement = props.config;

  const key = props.index;

  const changeValue = (e) => {
    blockElement[e.target.name] = e.target.value;
    props.updateBlock(key, blockElement);
  };

  return (
      <div key={`block_${key}`}>
        <input
          key={`block_input_${key}`}
          type="text"
          id={`fieldname_${key}`}
          name="fieldName"
          onChange={changeValue}
          placeholder="Field Name"
          defaultValue={blockElement.fieldName}
        />
      </div>
  );
};
 
 
const ModuleBuilder = () => {
 
  const emptyBlock = {
    fieldName: "",
    required: true,
  };

  const [blocks, setBlocks] = React.useState([emptyBlock]);

  const updateBlock = (key, data) => {
    //console.log(`i was asked to update element #${key}`);
    //console.log("this is data", data);

    let copyOfBlocks = blocks;

    //copyOfBlocks[key] = data; // WHY DOES COMMENTING THIS OUT STILL MAKE THE UPDATE WORK??

    setBlocks([...copyOfBlocks]);
   // console.log("this is blocks", blocks); 
  };

  const add1Block = () => {
    setBlocks([...blocks, emptyBlock]);
  };

  const renderBlockFormElements = () => {
    return blocks.map((value, index) => {
      return (
        <li key={index}>
          <b>Subcomponent with Index #{index}</b>
          <ModuleElement
            index={index}
            config={value}
            updateBlock={updateBlock}
          />
        </li>
      );
    });
  };

  return (
    <div>
      <h1>
        Click twice on "add another field" then enter something into the second
        field.
      </h1>
      <h2>Why is 1st field affected??</h2>
      <form>
        <ul className="list-group">{renderBlockFormElements()}</ul>
        <button type="button" onClick={add1Block}>
          Add another field
        </button>
      </form>
      <br></br>
      <h2>This is the state:</h2>
      {blocks.map((value, index) => (
        <p key={`checkup_${index}`}>
          <span>{index}: </span>
          <span>{JSON.stringify(value)}</span>
        </p>
      ))}
    </div>
  );
};

ReactDOM.render(<ModuleBuilder />, document.querySelector("#app"));

see full code on https://jsfiddle.net/g8yc39Lv/3/

UPDATE 1:

• I solved the problem (see my own answer below) but am still confused about one thing: Why is copyOfBlocks[key] = data; in the updateBlocks() not necessary to update the state correctly? Could it be that the following code is manipulating the props directly??

const changeValue = (e) => {
    blockElement[e.target.name] = e.target.value;
    props.updateBlock(key, blockElement);
  };

If yes, what would be the react way to structure my use case?

UPDATE 2:

It turns out indeed that I was manipulating the props directly. I changed now the whole setup as follows.

The Module Element component now has its own state. OnChange the state is updated and the new target.value is pushed to the props.updateblocks method. See updated JSfiddle here. Is this the react way to do it ?

Answer 1

Suggested changes to your new answer:

Your updated answer is mostly correct, and more React-ish. I would change a couple things:

1. You have two state variables, which means you have two sources of truth. It works now because they're always in sync, but this has the potential to cause future bugs, and is not actually necessary. ModuleElement doesn't need a state variable at all; you can just render props.config.fieldName:

   <input
     ...
     onChange={(e) => {
       props.updateBlock(key, {fieldName:e.target.value, required:true})
     }}
     value={props.config.fieldName}
   />
   

   Then, you can eliminate the state variable in ModuleElement:

   const ModuleElement = (props) => {
     const key = props.index;
   
     return (
       <React.Fragment>
         ...
   

2. I would write updateBlock a little differently. copyOfBlocks is not actually a copy, so copyOfBlocks[key] = data; actually mutates the original data, and a copy is not made until setBlocks([...copyOfBlocks]);. This isn't a problem per se, but a clearer way to write it could be:

   const updateBlock = (key, data) => {
     let copyOfBlocks = [...blocks];
   
     copyOfBlocks[key] = data;
   
     setBlocks(copyOfBlocks);
   };
   

   Now, copyOfBlocks is actually a shallow copy ([...blocks] is what causes the copy), and not a pointer to the same data.

Here's an updated fiddle.

---

Answers to your remaining questions:

//copyOfBlocks[key] = data; // WHY DOES COMMENTING THIS OUT STILL MAKE THE UPDATE WORK??

Because this line doesn't actually copy:

let copyOfBlocks = blocks;

// This outputs `true`!
console.log(copyOfBlocks === blocks);

This means that, when you do this in ModuleElement - changeValue...

blockElement[e.target.name] = e.target.value;

... you're mutating the same value as when you do this in ModuleBuilder - updateBlock ...

copyOfBlocks[key] = data;

... because copyOfBlocks[key] === blocks[key], and blocks[key] === ModuleBuilder's blockElement. Since both updaters had a reference to the same object, you were updating the same object twice.

But beyond that, mutation of state variables in React is an anti-pattern. This is because React uses === to detect changes. In this case, React cannot tell that myState has changed, and so will not re-render the component:

const [myState, setMyState] = useState({'existing key': 'existing value'});
// ...

// This is BAD:
myState['key'] = ['value'];

const [myState, setMyState] = useState({'existing key': 'existing value'});
// ...

// This is still BAD, because the state isn't being copied:
const newState = myState;
newState['key'] = ['value'];

// At this point, myState === newState

// React will not recognize this change!
setMyState(newState);

Instead, you should write code that

1. performs a shallow copy on myState, so the old state !== the new state, and
2. uses setMyState() to tell React that the state has changed:

const [myState, setMyState] = useState({'existing key': 'existing value'});
// ...

// This is correct, and uses ES6 spread syntax to perform a shallow copy of `myState`:
const newState = {...myState, key: 'value'};

// Now, newState !== myState. This is good :)

setMyState(newState);

Or, as a one-liner:

setMyState({...myState, key: 'value'});

---

The Module Element component now has its own state. onChange the state is updated and the new target.value is pushed to the props.updateblocks method. Is this the react way to do it?

For the most part, yes. You're duplicating your state in two variables, which is unnecessary and more likely to cause bugs in the future. See above for a suggestion on how to eliminate the additional state variable.

Answer 2

I was able to solve the problem after coincidentally reading this question.

I replaced the setBlocks line here:

const emptyBlock = {
    fieldName: "",
    dataType: "string",
    required: true,
  };

  const add1Block = () => {
    setBlocks([
      ...blocks,
      emptyBlock
    ]);
  };

by this statement

  const add1Block = () => {
    setBlocks([
      ...blocks,
      { fieldName: "", dataType: "string", required: true },
    ]);
  };

and it turned out that by placing emptyBlock as a default value for a new element I was just apparently just re-referencing it.