Code Review: Avoid Declaring React Component Inside Parent Component

A few days ago, one of my colleagues asked me to review his Pull Request (PR). The PR is about implementing a dynamic list of text inputs. The program will let users add as many fields as they want via an “Add” button.

This guy ran into one problem: the inputs will lose focus when users type in a key. ReactJs seems to rerender all inputs whenever users type a key; hence, no one focuses. He tried many solutions, but no one worked:

• Using controlled components, store states as an array of strings
• Using controlled components, store states as an array of objects
• Using uncontrolled components, access values of inputs via ref

After checking his code, I found that everything is fine, except for one major thing. The code looks similar to the below example:

import { useState } from 'react';

const Form = () => {
  const [values, setValues] = useState(['', '']);

  const onChange = (value, index) => {
    const newValues = [...values];
    newValues[index] = value;
    setValues(newValues);
  };

  const Input = (props) => {
    return <input type='text' {...props} />;
  };

  return (
    <>
      {values.map((value, index) => (
        <Input
          key={index}
          value={value}
          onChange={(e) => onChange(e.target.value, index)}
        />
      ))}
      <button onClick={() => setValues([...values, ''])}>Add</button>
    </>
  );
};

The problem is, he declared the Input component inside its parent (Form). Whenever the Form rerenders, it redeclares the Input component. Although we use the same name (in fact, its name only matters to humans, not machines), the Input of the 2nd render is a different component than the Input in the previous render. And that’s why all the text inputs rerender.

To solve the problem, we only need to move the Input outside of the Form.

import { useState } from 'react';

const Input = (props) => {
  return <input type='text' {...props} />;
};

const Form = () => {
  const [values, setValues] = useState(['', '']);

  const onChange = (value, index) => {
    const newValues = [...values];
    newValues[index] = value;
    setValues(newValues);
  };

  return (
    <>
      {values.map((value, index) => (
        <Input
          key={index}
          value={value}
          onChange={(e) => onChange(e.target.value, index)}
        />
      ))}
      <button onClick={() => setValues([...values, ''])}>Add</button>
    </>
  );
};

While I didn’t make the same mistake earlier, I hadn’t indeed against it in code review. After the section above, I put one note into my Code Review checklist for React: avoid declaring a component within another component.

=====

This article is part of my series about Code Review, which talks about my notes while reviewing code. Each of them could be tiny or nothing special for some people. But because a slight mistake can cost developers a lot to figure out, it could be helpful for others to mind all the possibilities.

I’ve put the samples above in a Github repository in case anyone wants to try them out.