React derived state. Whats wrong with this code

Coming from angular, i would like to know what's wrong with this code. I've been told that i should use useMemo (for the fitlering part) and useRef (to store products). I know how to do that, but i want to better understand what's wrong with this. I have compared the number of renderings whenever searchTerm changes, and i see no difference.

export function ProductList({ productService }) {

  const [products, setProducts] = useState([])
  const [filteredProducts, setFilteredProducts] = useState([])

  const [searchTerm, setSearchTerm] = useState(null)

  function fetchData() {
    productService
      .allProducts()
      .then((products) => {
        setProducts(products)
        setFilteredProducts(products)
      })
      .catch(console.error)
  }

  function filterProductsByName(searchTerm) {
    setSearchTerm(searchTerm)
    setFilteredProducts(filterByName(products, searchTerm))
  }

  useEffect(fetchData, [])

  return (
    <div>
      <h1>Products</h1>
      <form onSubmit={(e) => e.preventDefault()}>
        <div>
          <label htmlFor="searchTerm">Search</label>
          <input
            type="text"
            name="searchTerm"
            id="searchTerm"
            onChange={(e) => filterProductsByName(e.target.value)}
          />
        </div>
      </form>
      <div>
        {filteredProducts.map((p) => (
          <Product key={p.id} product={p} />
        ))}
      </div>
    </div>
  )
}

I have compared both versions (with an without useMemo)

I've been told that i should use useMemo (for the fitlering part) and useRef (to store products).

Using useMemo to manage derived state is good advice, using a ref to store state ... I'd question that decision. There are arguments to do this, but that's a separate discussion.

I have compared the number of renderings whenever searchTerm changes, and i see no difference.

This ain't an issue of performance but one where people can't always keep track of all the possible states they may find themselves in.

This is a fairly simple component, so it's fairly simple to update the filteredProducts when you load the products and whenever the searchTerm changes, right? What about this:

.then((products) => {
    setProducts(products)
    setFilteredProducts(products)
})

when you load the products you set the filteredProducts to the same list. That implies that there is no searchTerm. So you assume that this request will happen so fast that nobody would have time to enter a searchTerm before you set the products ... not even the browsers autocomplete? And what about a slow connection?

Even in this simple code, you've already overlooked a possibility which may lead to filteredProducts not reflecting the searchTerm. It's unlikely/rare, but possible.

OK, then we'll simply setFilteredProducts(filterByName(products, searchTerm)); and things are fine, right? But searchTerm in this context is a closure over the initial value, not its current value, so this doesn't work either.

Do you see the issue?

The trivial solution would be to just do

const filteredProducts = filterByName(products, searchTerm);

but this may compute unnecessarily often, so we useMemo to only recompute that list when its dependencies change:

const filteredProducts = useMemo(() => filterByName(products, searchTerm), [products, searchTerm]);

that's the "textbook solution";

Then again, this is a pretty simple component. Tell me, what besides loading the products or changing the searchTerm would trigger a rerender of this component? Doesn't that mean that you have to compute the filteredProducts on basically ever render anyways? useMemo is usually cheaper than the operation done within, but it ain't free.

I'd suggest you try

function ProductList({ productService }) {

  const [products, setProducts] = useState([]);
  const [searchTerm, setSearchTerm] = useState(null);

  function fetchData() {
    productService
      .allProducts()
      .then(setProducts)
      .catch(console.error);
  }

  useEffect(fetchData, [productService]);

  return (
    <div>
      <h1>Products</h1>
      <form onSubmit={(e) => e.preventDefault()}>
        <div>
          <label htmlFor="searchTerm">Search</label>
          <input
            type="text"
            name="searchTerm"
            id="searchTerm"
            value={searchTerm}
            onChange={(e) => setSearchTerm(e.target.value)}
          />
        </div>
      </form>
      <div>
        {filterByName(products, searchTerm).map((p) => (
          <Product key={p.id} product={p} />
        ))}
      </div>
    </div>
  )
}

One source of unnecessary rerenders may be the parent element. If that is the case you may want to consider React.memo().

Also take a look on the topic of "controlled vs uncontrolled input" in react, maybe start here: React - Controlled Components.