Johannes Kettmann

Posted on Nov 11, 2022 • Edited on Jan 6, 2023 • Originally published at profy.dev

Refactoring A Junior’s React Code - 43% Less Code With A Better Data Structure

#react #javascript #webdev #frontend

As an (aspiring) Junior React developer it’s hard to understand if you’re code is any good and how you can improve it. It would be great to get a review from an experienced dev. But sharing your code takes guts. And the reviews you get in online communities are often shallow.

Recently, a developer was brave enough and requested a code review. Kudos. The feature was interesting. The code wasn’t bad. But there were a few mistakes that are very common among Junior React developers.

So here’s your opportunity to learn from these mistakes. On this page, you can find a code review and a step-by-step refactoring journey. By

using a better data structure
removing unnecessary state and
a few other refactorings and cleanup

we increase the performance of the code, make it easier to read and maintain, and shorten the component from initially 177 to 102 lines (not that this is a good metric).

If you’re rather a video person you can watch the complete review and refactoring session here.

Note: the code on this page is slightly improved compared to the code in the video using a Set instead of a Map.

The Component To Review
1. The Functionality
2. The Original Code
Problems With The Original Code
The Refactoring
Final Code
Takeaways

The Component To Review

The Functionality

The component is a table with a few somewhat complex features. Here’s a quick video:

The table renders a list of issues. An issue can have a status “open” or “resolved” that is reflected in the last column. Issues with the status “resolved” can’t be selected.

The checkbox at the top is partially checked when some but not all rows are selected. Clicking on that checkbox allows the user to select or deselect all issues with the status “open”.

The Original Code

The original code isn’t bad. It works and does its job. For the most part, it’s quite readable. Especially on a high level. And it’s similar to what many Junior React devs would produce. At the same time, there are a few problems that are again common among Junior devs. And we’ll discuss them in a bit.

But first things first. Here’s the component’s code. You can also find a functioning version on CodeSandbox.

import { useState } from "react";
import classes from "./Table.module.css";

function Table({ issues }) {
  const [checkedState, setCheckedState] = useState(
    new Array(issues.length).fill({
      checked: false,
      backgroundColor: "#ffffff",
    })
  );
  const [selectDeselectAllIsChecked, setSelectDeselectAllIsChecked] =
    useState(false);
  const [numCheckboxesSelected, setNumCheckboxesSelected] = useState(0);

  const handleOnChange = (position) => {
    const updatedCheckedState = checkedState.map((element, index) => {
      if (position === index) {
        return {
          ...element,
          checked: !element.checked,
          backgroundColor: element.checked ? "#ffffff" : "#eeeeee",
        };
      }
      return element;
    });
    setCheckedState(updatedCheckedState);

    const totalSelected = updatedCheckedState
      .map((element) => element.checked)
      .reduce((sum, currentState, index) => {
        if (currentState) {
          return sum + issues[index].value;
        }
        return sum;
      }, 0);
    setNumCheckboxesSelected(totalSelected);

    handleIndeterminateCheckbox(totalSelected);
  };

  const handleIndeterminateCheckbox = (total) => {
    const indeterminateCheckbox = document.getElementById(
      "custom-checkbox-selectDeselectAll"
    );
    let count = 0;

    issues.forEach((element) => {
      if (element.status === "open") {
        count += 1;
      }
    });

    if (total === 0) {
      indeterminateCheckbox.indeterminate = false;
      setSelectDeselectAllIsChecked(false);
    }
    if (total > 0 && total < count) {
      indeterminateCheckbox.indeterminate = true;
      setSelectDeselectAllIsChecked(false);
    }
    if (total === count) {
      indeterminateCheckbox.indeterminate = false;
      setSelectDeselectAllIsChecked(true);
    }
  };

  const handleSelectDeselectAll = (event) => {
    let { checked } = event.target;

    const allTrueArray = [];
    issues.forEach((element) => {
      if (element.status === "open") {
        allTrueArray.push({ checked: true, backgroundColor: "#eeeeee" });
      } else {
        allTrueArray.push({ checked: false, backgroundColor: "#ffffff" });
      }
    });

    const allFalseArray = new Array(issues.length).fill({
      checked: false,
      backgroundColor: "#ffffff",
    });
    checked ? setCheckedState(allTrueArray) : setCheckedState(allFalseArray);

    const totalSelected = (checked ? allTrueArray : allFalseArray)
      .map((element) => element.checked)
      .reduce((sum, currentState, index) => {
        if (currentState && issues[index].status === "open") {
          return sum + issues[index].value;
        }
        return sum;
      }, 0);
    setNumCheckboxesSelected(totalSelected);
    setSelectDeselectAllIsChecked((prevState) => !prevState);
  };

  return (
    <table className={classes.table}>
      <thead>
        <tr>
          <th>
            <input
              className={classes.checkbox}
              type={"checkbox"}
              id={"custom-checkbox-selectDeselectAll"}
              name={"custom-checkbox-selectDeselectAll"}
              value={"custom-checkbox-selectDeselectAll"}
              checked={selectDeselectAllIsChecked}
              onChange={handleSelectDeselectAll}
            />
          </th>
          <th className={classes.numChecked}>
            {numCheckboxesSelected
              ? `Selected ${numCheckboxesSelected}`
              : "None selected"}
          </th>
        </tr>
        <tr>
          <th />
          <th>Name</th>
          <th>Message</th>
          <th>Status</th>
        </tr>
      </thead>

      <tbody>
        {issues.map(({ name, message, status }, index) => {
          let issueIsOpen = status === "open";
          let onClick = issueIsOpen ? () => handleOnChange(index) : null;
          let stylesTr = issueIsOpen
            ? classes.openIssue
            : classes.resolvedIssue;

          return (
            <tr
              className={stylesTr}
              style={checkedState[index]}
              key={index}
              onClick={onClick}
            >
              <td>
                {issueIsOpen ? (
                  <input
                    className={classes.checkbox}
                    type={"checkbox"}
                    id={`custom-checkbox-${index}`}
                    name={name}
                    value={name}
                    checked={checkedState[index].checked}
                    onChange={() => handleOnChange(index)}
                  />
                ) : (
                  <input
                    className={classes.checkbox}
                    type={"checkbox"}
                    disabled
                  />
                )}
              </td>
              <td>{name}</td>
              <td>{message}</td>
              <td>
                {issueIsOpen ? (
                  <span className={classes.greenCircle} />
                ) : (
                  <span className={classes.redCircle} />
                )}
              </td>
            </tr>
          );
        })}
      </tbody>
    </table>
  );
}

export default Table;

The issues array that is rendered in the table looks like this.

[
  {
    "id": "c9613c41-32f0-435e-aef2-b17ce758431b",
    "name": "TypeError",
    "message": "Cannot read properties of undefined (reading 'length')",
    "status": "open",
    "numEvents": 105,
    "numUsers": 56,
    "value": 1
  },
  {
    "id": "1f62d084-cc32-4c7b-943d-417c5dac896e",
    "name": "TypeError",
    "message": "U is not a function",
    "status": "resolved",
    "numEvents": 45,
    "numUsers": 34,
    "value": 1
  },
  ...
]

The component is more on the complex side with 177 lines of code (LOC). But as mentioned, on the high level it’s quite readable. This is what you see when the functions are collapsed.

function Table({ issues }) {
  // decides which checkboxes is selected
  const [checkedState, setCheckedState] = useState(...);
  // decides if the top checkbox is checked or not
  const [selectDeselectAllIsChecked, setSelectDeselectAllIsChecked] =
    useState(false);
  // represents the number of selected checkboxes
  const [numCheckboxesSelected, setNumCheckboxesSelected] = useState(0);

  // handles the click on a row / checkbox
  const handleOnChange = (position) => { ... };

  // responsible for setting the partially checked state of the top checkbox
  const handleIndeterminateCheckbox = (total) => { ... };

  // handles the click on the top checkbox
  const handleSelectDeselectAll = (event) => { ... };

  return (
    <table>
      ...
    </table>
  );
}

Not too bad. But there’s room for improvement.

Problems With The Original Code

Problem 1: Sub-Optimal Data Structure

Let’s have a look at the top of the component.

function Table({ issues }) {
  const [checkedState, setCheckedState] = useState(
    new Array(issues.length).fill({ checked: false, backgroundColor: "#ffffff" })
  );

  ...

The checkedState variable is an array.
It’s initialized to have the same length as the issues array.
Each entry is an object that represents the checked state of an issue.

The first problem is that an array of objects is always hard to mutate. We have to access the correct object via its index inside the array. So we need to find and pass indices around.

The second problem is that this approach doesn’t scale very well. For example, if we had 10k issues we’d have a 10k long checkedState array filled with mostly false boolean values.

The alternative: We could use a different data structure like an object or a Map that maps an issue id to its checked value.

// alternative data structure (example when two checkboxes are checked)
{
  "issue-id-1": true,
  "issue-id-4": true,
}

Even better, JavaScript has a native Set that is similar to an array but can only hold unique values and is optimized for accessing them in a performant way.

Problem 2: Not Deriving Variables From State

Again at the top of the component, we can see another very common problem in the code of Junior developers: Unnecessary state variables that could be derived from props or another state.

function Table({ issues }) {
  const [checkedState, setCheckedState] = useState(
    new Array(issues.length).fill({ checked: false, backgroundColor: "#ffffff" })
  );
  const [selectDeselectAllIsChecked, setSelectDeselectAllIsChecked] =
    useState(false);
  const [numCheckboxesSelected, setNumCheckboxesSelected] = useState(0);

  ...

In this code, we have three unnecessary pieces of state:

backgroundColor is a style that can be derived from the value of checked (it’s either #eee if checked is true otherwise #fff)
selectDeselectAllIsChecked is used to set the checked prop for the checkbox at the top (can be derived from checkedState and issues)
numCheckboxesSelected is the number next to that checkbox (can be derived from checkedState)

Problem 3: Directly Accessing The DOM

A bit further down the code, we can find the function that handles the indeterminate state of the top checkbox:

function Table({ issues }) {
  ...

  const handleIndeterminateCheckbox = (total) => {
    const indeterminateCheckbox = document.getElementById(
      "custom-checkbox-selectDeselectAll"
    );
    let count = 0;

    issues.forEach((element) => {
      if (element.status === "open") {
        count += 1;
      }
    });

    if (total === 0) {
      indeterminateCheckbox.indeterminate = false;
      setSelectDeselectAllIsChecked(false);
    }
    if (total > 0 && total < count) {
      indeterminateCheckbox.indeterminate = true;
      setSelectDeselectAllIsChecked(false);
    }
    if (total === count) {
      indeterminateCheckbox.indeterminate = false;
      setSelectDeselectAllIsChecked(true);
    }
  };

  ...

We can see that the top checkbox is accessed via document.getElementById(...). This is relatively unusual in React apps and mostly not necessary.

Note that setting indeterminateCheckbox.indeterminate directly on the DOM element is necessary. We can’t set indeterminate as a prop on the checkbox.

Problem 4: Variable Naming

The author of this code put some effort into selecting good names for the variables and functions. Kudos to that.

Unfortunately, some names are a bit confusing. We can find some examples at the top of the component:

function Table({ issues }) {
  const [checkedState, setCheckedState] = useState(
    new Array(issues.length).fill({ checked: false, backgroundColor: "#ffffff" })
  );
  const [selectDeselectAllIsChecked, setSelectDeselectAllIsChecked] =
    useState(false);
  const [numCheckboxesSelected, setNumCheckboxesSelected] = useState(0);

  ...

For example, checkedState is redundant and misleading. I’d expect a boolean here but this is an array of objects. A better name might be checkedItems.

selectDeselectAllIsChecked kind of is understandable but a bit hard to read. It tells us whether all checkboxes are checked or not but I had to read it like 5 times. A better name could be isEveryItemChecked.

The Refactoring

Now that we’ve analyzed some of the problems with the original code let’s start to refactor it step by step.

Step 1: Refactor Data Structure From Array To Set

The most impactful step is to use a Set instead of the array as the checkedState. We could have used an object as well but the Set has slight advantages like direct access to its size which we’ll use quite a bit. Compared to a Map, the Set is a slightly better fit here as we only need to know the ids of all selected items instead of a key-value pair.

As we’re already touching this code we also remove the backgroundColor from the state and rename the state variable.

//before
const [checkedState, setCheckedState] = useState(
  new Array(issues.length).fill({ checked: false, backgroundColor: "#ffffff" })
);

// after
const [checkedById, setCheckedById] = useState(new Set());

As a reminder, the shape of the checkedById state (represented as an array) looks now like this:

// shape of checkedById (when two checkboxes are checked)
["issue-id-1", "issue-id-4"]

This change isn’t trivial. It has implications for the rest of our code. We’ll see one by one how each part of the code is affected.

Let’s start with the handler function that’s connected to the checkboxes in the table.

Here’s the before and after of the code:

// before
const handleOnChange = (position) => {
  const updatedCheckedState = checkedState.map((element, index) => {
    if (position === index) {
      return {
        ...element,
        checked: !element.checked,
        backgroundColor: element.checked ? "#ffffff" : "#eeeeee",
      };
    }
    return element;
  });
  setCheckedState(updatedCheckedState);

  const totalSelected = updatedCheckedState
    .map((element) => element.checked)
    .reduce((sum, currentState, index) => {
      if (currentState) {
        return sum + issues[index].value;
      }
      return sum;
    }, 0);
  setNumCheckboxesSelected(totalSelected);

  handleIndeterminateCheckbox(totalSelected);
};

// after
const handleOnChange = (id) => {
  const updatedCheckedById = new Set(checkedById);
  if (updatedCheckedById.has(id)) {
    updatedCheckedById.delete(id);
  } else {
    updatedCheckedById.add(id);
  }

  setCheckedById(updatedCheckedById);

  const totalSelected = updatedCheckedById.size;
  setNumCheckboxesSelected(totalSelected);
  handleIndeterminateCheckbox(totalSelected);
};

We got rid of quite some clutter. Some quick notes:

The line return sum + issues[index].value in the original code is interesting. It uses a value in the data to increment the sum and calculate the number of selected elements. This seems over-complex.
In the refactored code we can now simply use updatedCheckedById.size to get the number of selected elements.
Since we don’t want any false values in our state we have to delete the corresponding value when a checkbox is deselected via updatedCheckedById.delete(id).

Let’s continue with the handler function that is connected to the top checkbox.

Here’s the before and after.

// before
const handleSelectDeselectAll = (event) => {
  let { checked } = event.target;

  const allTrueArray = [];
  issues.forEach((element) => {
    if (element.status === "open") {
      allTrueArray.push({ checked: true, backgroundColor: "#eeeeee" });
    } else {
      allTrueArray.push({ checked: false, backgroundColor: "#ffffff" });
    }
  });

  const allFalseArray = new Array(issues.length).fill({
    checked: false,
    backgroundColor: "#ffffff",
  });
  checked ? setCheckedState(allTrueArray) : setCheckedState(allFalseArray);

  const totalSelected = (checked ? allTrueArray : allFalseArray)
    .map((element) => element.checked)
    .reduce((sum, currentState, index) => {
      if (currentState && issues[index].status === "open") {
        return sum + issues[index].value;
      }
      return sum;
    }, 0);
  setNumCheckboxesSelected(totalSelected);
  setSelectDeselectAllIsChecked((prevState) => !prevState);
};

// after
const handleSelectDeselectAll = (event) => {
    if (event.target.checked) {
      const openIssues = issues.filter(({ status }) => status === "open");
      const allChecked = new Set(openIssues.map(({ id }) => id));
      setCheckedById(allChecked);
      setNumCheckboxesSelected(allChecked.size);
    } else {
      setCheckedById(new Set());
      setNumCheckboxesSelected(0);
    }

    setSelectDeselectAllIsChecked((prevState) => !prevState);
  };

Again a few quick notes:

The original implementation is a bit cumbersome with the allTrueArray and allFalseArray. We could refactor this as well but since we use an object now we can simply remove this code.
Because we use a Set in the new implementation creating the new allChecked state is very straightforward. We filter all open issues and then construct a new Set from an array of ids.

The last piece of code that we have to adjust is the JSX returned from the component. In particular the code inside the <tbody> element.

// before
<tbody>
  {issues.map(({ name, message, status }, index) => {
    let issueIsOpen = status === "open";
    // we have to use the id of the issue instead of the index here
    let onClick = issueIsOpen ? () => handleOnChange(index) : null;
    let stylesTr = issueIsOpen
      ? classes.openIssue
      : classes.resolvedIssue;

    return (
      <tr
        className={stylesTr}
        // the backgroundColor isn't part of checkedState anymore
        // so we have to adjust this
        style={checkedState[index]}
        key={index}
        onClick={onClick}
      >
        <td>
          {issueIsOpen ? (
            <input
              className={classes.checkbox}
              type={"checkbox"}
              id={`custom-checkbox-${index}`}
              name={name}
              value={name}
              // we have to use the id instead of the index here
              checked={checkedState[index].checked}
              onChange={() => handleOnChange(index)}
            />
          ) : (
            <input
              className={classes.checkbox}
              type={"checkbox"}
              disabled
            />
          )}
        </td>
        ...
      </tr>
    );
  })}
</tbody>

// after
<tbody>
  {issues.map(({ id, name, message, status }, index) => {
    let issueIsOpen = status === "open";
    let onClick = issueIsOpen ? () => handleOnChange(id) : null;
    let stylesTr = issueIsOpen
      ? classes.openIssue
      : classes.resolvedIssue;

    return (
      <tr
        key={id}
        className={stylesTr}
        style={{ backgroundColor: checkedById.has(id) ? "#eee" : "#fff" }}
        onClick={onClick}
      >
        <td>
          {issueIsOpen ? (
            <input
              className={classes.checkbox}
              type={"checkbox"}
              id={`custom-checkbox-${index}`}
              name={name}
              value={name}
              checked={checkedById.has(id)}
              onChange={() => handleOnChange(id)}
            />
          ) : (
            <input
              className={classes.checkbox}
              type={"checkbox"}
              disabled
            />
          )}
        </td>
        ...
      </tr>
    );
  })}
</tbody>

Again a few quick notes:

We have to change most of the occurrences of index in this code.
Instead of writing key={index} on the <tr> element we use the id as key now. This doesn’t change anything in the current version as the order of issues doesn’t change. But it’s best practice and it’s likely that some kind of sorting will be introduced to our table in the future.
Previously the background color of the row was set via this line: style={checkedState[index]}. This takes a moment to understand. For any given row this translates to e.g. style={{ checked: true, backgroundColor: "#fff" }}.
The new version is much more explicit in this regard: style={{ backgroundColor: checkedById[id] ? "#eee" : "#fff" }}

You can find all code changes here on GitHub and a function version of this step here on CodeSandbox (note that you need to change App.jsx to see this version of the component rendered in the browser).

Step 2: Using Derived State

The second problem we discussed above is the unnecessary state variables selectDeselectAllIsChecked and numCheckboxesSelected. As mentioned, these can easily be derived from the checkedById state and the issues data array.

// before
function Table({ issues }) {
  const [checkedById, setCheckedById] = useState(new Set());
  const [selectDeselectAllIsChecked, setSelectDeselectAllIsChecked] =
    useState(false);
  const [numCheckboxesSelected, setNumCheckboxesSelected] = useState(0);

  ...

// after
function Table({ issues }) {
  const [checkedById, setCheckedById] = useState(new Set());
  const openIssues = issues.filter(({ status }) => status === "open");
  const numOpenIssues = openIssues.length;
  const numCheckedIssues = checkedById.size;

  ...

We don’t necessarily need the openIssues variable here. But this way we can reuse it inside the refactored handleSelectDeselectAll function (which is not shown here).

On top of these changes, we also have to remove the calls to the state setters from the rest of the code. These changes have two advantages:

We get rid of a few lines of code.
More importantly, we don’t need to manage the additional state anymore. This is great because we can easily forget to update the state and thus introduce bugs.

As the last step, we can memoize the calculation of numOpenIssues. This is not required for the current version but might be necessary once we work with larger data sets.

const openIssues = useMemo(
  () => issues.filter(({ status }) => status === "open"),
  [issues]
);

You can see the complete list of code changes here on GitHub and a functioning version of this step here on CodeSandbox.

Step 3: Remove document.getElementById()

The final important step in our refactoring journey is to access the top checkbox the React way instead of using document.getElementById().

What’s the React way? We use a ref.

// before
function Table({ issues }) {
  ...

  const handleIndeterminateCheckbox = (total) => {
    const indeterminateCheckbox = document.getElementById(
      "custom-checkbox-selectDeselectAll"
    );
    let count = 0;

    issues.forEach((element) => {
      if (element.status === "open") {
        count += 1;
      }
    });

    if (total === 0) {
      indeterminateCheckbox.indeterminate = false;
    }
    if (total > 0 && total < count) {
      indeterminateCheckbox.indeterminate = true;
    }
    if (total === count) {
      indeterminateCheckbox.indeterminate = false;
    }
  };

  ...

// after
function Table({ issues }) {
  ...

  const topCheckbox = useRef();
  const handleIndeterminateCheckbox = (checkedById) => {
    const numSelected = checkedById.size;
    if (numSelected === 0) {
      topCheckbox.current.indeterminate = false;
    } else if (numSelected === numOpenIssues) {
      topCheckbox.current.indeterminate = false;
    } else {
      topCheckbox.current.indeterminate = true;
    }
  };

  ...

  return (
    <table className={classes.table}>
      <thead>
        <tr>
          <th>
            <input
              ref={topCheckbox}
              ...

A few quick notes:

The most important thing is that we replaced document.getElementById() by useRef(). This also allows us to remove the id from the checkbox.
The line issues.forEach((element) => { in the original code is responsible for calculating the number of open issues. This is a bit cumbersome approach and could have easily been replaced with issues.filter(...).length. But we already have defined the numOpenIssues variable at the top of our component. So no need for this code anymore.
We used if ... else if ... else instead of three if statements. We also switched the conditions around to have the only true value at the bottom. This makes it easier to process for our brains in my opinion.

Actually, we don’t even need the if ... else if ... else statement. We can simply replace it with a single line of code:

topCheckbox.current.indeterminate =
      numSelected > 0 && numSelected < numOpenIssues;

Using this line it also doesn’t make much sense to keep the handleIndeterminateCheckbox function anymore.

You can find all the code changes here on GitHub and a functioning version of this step here on CodeSandbox.

Final Code

After a bit of additional cleanup (here are the changes) we’re able to reduce the code from initially 177 LOC to 102 LOC.

import { useMemo, useState, useRef } from "react";
import classes from "./Table.module.css";

function Table({ issues }) {
  const topCheckbox = useRef();
  const [checkedById, setCheckedById] = useState(new Set());

  const openIssues = useMemo(
    () => issues.filter(({ status }) => status === "open"),
    [issues]
  );
  const numOpenIssues = openIssues.length;
  const numCheckedIssues = checkedById.size;

  const handleOnChange = (id) => {
    const updatedCheckedById = new Set(checkedById);
    if (updatedCheckedById.has(id)) {
      updatedCheckedById.delete(id);
    } else {
      updatedCheckedById.add(id);
    }
    setCheckedById(updatedCheckedById);

    const updatedNumChecked = updatedCheckedById.size;
    topCheckbox.current.indeterminate =
      updatedNumChecked > 0 && updatedNumChecked < numOpenIssues;
  };

  const handleSelectDeselectAll = (event) => {
    if (event.target.checked) {
      const allChecked = new Set(openIssues.map(({ id }) => id));
      setCheckedById(allChecked);
    } else {
      setCheckedById(new Set());
    }
  };

  return (
    <table className={classes.table}>
      <thead>
        <tr>
          <th>
            <input
              type="checkbox"
              ref={topCheckbox}
              className={classes.checkbox}
              checked={numOpenIssues === numCheckedIssues}
              onChange={handleSelectDeselectAll}
            />
          </th>
          <th className={classes.numChecked}>
            {numCheckedIssues
              ? `Selected ${numCheckedIssues}`
              : "None selected"}
          </th>
        </tr>
        <tr>
          <th />
          <th>Name</th>
          <th>Message</th>
          <th>Status</th>
        </tr>
      </thead>

      <tbody>
        {issues.map(({ id, name, message, status }) => {
          const isIssueOpen = status === "open";
          return (
            <tr
              key={id}
              className={
                isIssueOpen ? classes.openIssue : classes.resolvedIssue
              }
              style={{ backgroundColor: checkedById.has(id) ? "#eee" : "#fff" }}
            >
              <td>
                <input
                  type="checkbox"
                  className={classes.checkbox}
                  checked={checkedById.has(id)}
                  disabled={!isIssueOpen}
                  onChange={() => handleOnChange(id)}
                />
              </td>
              <td>{name}</td>
              <td>{message}</td>
              <td>
                <span
                  className={
                    isIssueOpen ? classes.greenCircle : classes.redCircle
                  }
                />
              </td>
            </tr>
          );
        })}
      </tbody>
    </table>
  );
}

export default Table;

You can find all code changes here on GitHub and a version of the final component here on CodeSandbox.

From my perspective, this is not only more concise but also much more readable.

Takeaways

Watch out for unnecessary state that can be derived from other state or data.
Using an object, a map, or a set can often be easier and more efficient than an array.
The React way of accessing DOM elements is useRef() and not document.getElementById().

Top comments (11)

Yuki Kimoto • Nov 16 '22

Thanks for React information. I use Perl in my web development. I can combinate with Perl and React.

Augusto Collerone • Nov 21 '22

Nice video. Great to see your thought process

Schleidens.dev • Nov 13 '22

Thank you 🖐

DR • Nov 11 '22

Really interesting. Thanks for this informative article!

Johannes Kettmann • Nov 11 '22

Thanks, I'm glad you liked it

Adetoba Adedeji • Nov 12 '22

Thanks for this. I learnt a lot following your explanations. But I have a point to make. The reason he had the onClick function on the row is to be able to click the row to check and uncheck the box, removing it won't give that opportunity anymore. You can now only click the box to check and uncheck after removing it.

Johannes Kettmann • Nov 12 '22

Yeah that's true. I mention in the video that removing the click handler from the tr is a change in behavior. But in the original code there's a click handler on the tr and on the input which doesn't make sense imo. And if we wanted to make the tr clickable we would have to adjust more things like setting a tab index to make it accessible. So I just opted for using the input

_jcyber • Nov 17 '22

This is really cool to see how in depth you went with this. As a junior react developer myself I’ve never seen any utilize the document object within a react application. Is this as common as it seems?

Johannes Kettmann • Nov 17 '22

Yeah, it's not very common. There are scenarios where it's required. For example, when you need to attach a scroll or click listener to the HTML body. Or maybe when you work with third-party tools where you can't use a ref. But in most cases there's a more React-like alternative.