Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

useState hook can only set one object at the time and return the other object of the same array to initial state

I have data in this form:

books = [{
    id: 1,
    name: "book-1",
    audiences: [
      {
        audienceId: 1,
        name: "Cat A",
        critics: [
          { id: 5, name: "Jack" },
          { id: 45, name: "Mike" },
          { id: 32, name: "Amanda" }
        ],
        readers: [
          { id: 11, fullName: "Mike Ross" },
          { id: 76, fullName: "Natalie J" },
          { id: 34, fullName: "Harvey Spectre" }
        ]
      }]

The forms are nested. For each book, there are critics and readers, and would render each form depending on audienceId value

<div className="App">
      {books.map((book, index) => (
        <div key={book.id}>
          <h1>Books {index + 1}</h1>
          {book.audiences.map((au) => (
            <div key={au.audienceId}>
              <h3>{au.name}</h3>
              <Recap
                audiences={au}
                shouldRenderCritics={au.audienceId === 2}
                shouldRenderReaders={au.audienceId === 1}
                criticsChildren={renderByAudience(au.audienceId)}
                readersChildren={renderByAudience(au.audienceId)}
              />
            </div>
          ))}
        </div>
      ))}
    </div>

This is form to render depending on audienceId

return (
    <div className="root">
      <div>
        {props.audienceId === 1 && (
          <A
            setReader={setReader(readerIdx)}
            reader={selectedReader as IreaderState}
          />
        )}
      </div>
      <div>
        {props.audienceId === 2 && (
          <B
            setCritic={setCritic(criticIdx)}
            critic={selectedCritic as IcriticState}
          />
        )}
      </div>
    </div>
  );

Finally, for each reader/critic, there is a form to input.

export default function A(props: Aprops) {
  const handleChange = (
    event: ChangeEvent<{ value: number | string; name: string }>
  ) => {
    const { name, value } = event.target;

    const r = { ...props.reader };
    r[name] = value;
    props.setReader(r);
  };

  return (
    <div>
      <TextField
        name="rate"
        type="number"
        value={get(props.reader, "rate", "")}
        onChange={handleChange}
      />
      <TextField
        name="feedback"
        value={get(props.reader, "feedback", "")}
        onChange={handleChange}
      />
    </div>
  );

The issue

Each time I fill in a field of critic/reader, it set state for that object but also set the initial state for the rest of objects. it only set state of the form in which in focus and doesnt keep the values of other forms

I don't know what causes the problem.

Here is a sandbox reproducing the issue https://codesandbox.io/s/project-u0m5n

like image 744
Ndx Avatar asked Mar 06 '21 18:03

Ndx


1 Answers

Wow, this was a doozy. The ultimate problem stems from the fact that you're calling the following component:

export default function FormsByAudience(props: FormsByAudienceProps) {
  const [critics, setCritics] = useCritics(props.books);

  const setCritic = (index: number) => (critic: IcriticState) => {
    const c = [...critics];

    c[index] = critic;
    setCritics(c);
  };

  // ...
  // in render method:
    setCritic={setCritic(criticIdx)}

for every single different critic (and reader):

props.audiences.critics.map((c) => (
    <div key={c.id}>
        <div>{c.name}</div>
        {props.shouldRenderCritics &&
            props.criticsChildren &&
            cloneElement(props.criticsChildren, { criticId: c.id })}
    </div>
))}

where props.criticsChildren contains <FormsByAudience audienceId={id} books={books} />.

As a result, inside a single render, there are lots of separate bindings for the critics variable. You don't have a single critics for the whole application, or for a given audience; you have multiple critics arrays, one for each critic. Setting the state for one critic's critics array in FormsByAudience does not result in changes to other critics arrays that the other critics' React elements close over.

To fix it, given that the critics array is being created from props.books, the critics state should be put near the same level where the books are used, and definitely not inside a component in a nested mapper function. Then pass down the state and the state setters to the children.

The exact same thing applies to the readers state.

Here is a minimal live Stack Snippet illustrating this problem:

const people = ['bob', 'joe'];
const Person = ({name, i}) => {
  const [peopleData, setPeopleData] = React.useState(people.map(() => 0));
  console.log(peopleData.join(','));
  const onChange = e => {
    setPeopleData(
      peopleData.map((num, j) => j === i ? e.target.value : num)
    )
  };
  return (
    <div>
      <div>{name}</div>
      <input type="number" value={peopleData[i]} onChange={onChange} />
    </div>
  );
};
const App = () => {
    return people.map(
      (name, i) => <Person key={i} {...{ name, i }} />
    );
};

ReactDOM.render(<App />, document.querySelector('.react'));
<script crossorigin src="https://unpkg.com/react@16/umd/react.development.js"></script>
<script crossorigin src="https://unpkg.com/react-dom@16/umd/react-dom.development.js"></script>
<div class='react'></div>

To make the passing of props down easier to type and read, you could consider consolidating the 4 variables (2 data and 2 setters) into a single object.

export default function App() {
  const [critics, setCritics] = useCritics(books);
  const [readers, setReaders] = useReaders(books);
  const dataAndSetters = { critics, setCritics, readers, setReaders };
  const renderByAudience = (id: number) => {
    return <FormsByAudience audienceId={id} {...{ books, dataAndSetters }} />;
  };

Pass dataAndSetters down until you get to

export default function FormsByAudience(props: FormsByAudienceProps) {
  const { critics, setCritics, readers, setReaders } = props.dataAndSetters;

Live demo:

https://codesandbox.io/s/project-forked-woqhf


If I could offer a few other pieces of advice, to up your code quality significantly:

  • Change Recap so that FormsByAudience is imported and called in Recap instead of in the parent. It's really weird (and hard to easily understand at a glance) for renderByAudience to create a React element that sometimes won't ever get used.

  • Recap has the critics and readers switched around. This is probably unintentional. This:

    <h5>Readers</h5>
      {!isEmpty(props.audiences.critics) &&
        props.audiences.critics.map((c) => (
    

    should probably be

    <h5>Readers</h5>
      {props.audiences.readers.map((c) => (
    

    (there's no need for an isEmpty check before mapping an array)

  • The whole

    const selectedReader =
      readers.find((r) => r.readerId === (props.readerId as number)) || {};
    const readerIdx = readers.indexOf(selectedReader as IreaderState);
    const selectedCritic =
      critics.find((r) => r.criticId === (props.criticId as number)) || {};
    const criticIdx = critics.indexOf(selectedCritic as IcriticState);
    

    is unnecessary. Just pass down the reader/critic index from the parent component instead, and use critics[criticIdx] to get to the critic in question.

  • In hook.ts, this:

    const byAudience = books
      .map((b) => b.audiences)
      .flat()
      .filter((bk) => [1].includes(bk.audienceId));
    

    simplifies to

    const byAudience = books
      .flatMap(b => b.audiences)
      .filter(bk => bk.audienceId === 1);
    

    And this:

    byAudience.forEach((el) => {
      el.readers.map((r) => {
        return readersToSet.push({ feedback: "", rate: "", readerId: r.id });
      });
      return readersToSet;
    });
    

    doesn't make sense because forEach ignores its return value, and .map should only be used when returning a value from the callback to construct a new array. Either use flatMap on byAudience, or push inside a for..of loop.

    const readersToSet = byAudience.flatMap(
      el => el.readers.map(
        r => ({ feedback: "", rate: "", readerId: r.id })
      )
    );
    
like image 165
CertainPerformance Avatar answered Oct 07 '22 17:10

CertainPerformance