0

I have a list of products and I want to add a few more data like price and quantity. The problem is I lose input focus when I start typing because the entire list is being re-rendered. I have a simple Fiddle replicating that piece of code:



const App = () => {

  // List of products
  const [products, setProducts] = React.useState([
  {
    title: 'My Product',
  },
  {
    title: 'My Product 2',
  }
  ]);
  
  
  // Simple debug to track changes
  React.useEffect(() => {
    console.log('PRODUCTS', products);
  }, [products]);
  
  
  const ProductForm = ({ data, index }) => {
    
    const handleProductChange = (name, value) => {
      const allProducts = [...products];
      const selectedProduct = {...allProducts[index]};
      allProducts[index] = {
        ...selectedProduct,
        [name]: value
      };
      setProducts([ ...allProducts ]);
    }
    
    return (
      <li>
        <h2>{data.title}</h2>
        <label>Price:</label>
        <input type="text" value={products[index].price} onChange={(e) => handleProductChange('price', e.target.value)} />
      </li>
    );
  }
  
  return <ul>{products.map((item, index) => <ProductForm key={item.title} index={index} data={item} />)}</ul>;
}

ReactDOM.render(
  <App />,
  document.getElementById('container')
);

https://jsfiddle.net/lucasbittar/zh1d6y37/23/

I've tried a bunch of solution I found here but none of them really represents what I have.

Thanks!

2 Answers2

2

Issue

You've defined ProductForm internally to App so it is recreated each render cycle, i.e. it's a completely new component each render.

Solution

Move ProductForm to be externally defined. This now has issue where the handleProductChange doesn't have access to App's functional scope products state. The solution here is to move handleProductChange back into App and update the function signature to consume the index. Use data.price as the input value, you can provide a fallback value or provide initial state for this property.

Suggestion: Name the input name="price" and simply consume it from the event object.

const ProductForm = ({ data, index, handleProductChange }) => {
    return (
      <li>
        <h2>{data.title}</h2>
        <label>Price:</label>
        <input
          name="price" // <-- name input field
          type="text"
          value={data.price || ''} // <-- use data.price or fallback value
          onChange={(e) => handleProductChange(e, index)} // <-- pass event and index
        />
      </li>
    );
  }

const App = () => {

    // List of products
    const [products, setProducts] = React.useState([
  {
    title: 'My Product',
  },
  {
    title: 'My Product 2',
  }
  ]);
  
  
  // Simple debug to track changes
  React.useEffect(() => {
    console.log('PRODUCTS', products);
  }, [products]);

  // Update signature to also take index
  const handleProductChange = (e, index) => {
      const { name, value } = e.target; // <-- destructure name and value
      const allProducts = [...products];
      const selectedProduct = {...allProducts[index]};
      allProducts[index] = {
        ...selectedProduct,
        [name]: value
      };
      setProducts([ ...allProducts ]);
    }
  
  return (
    <ul>
      {products.map((item, index) => (
        <ProductForm
          key={item.title}
          index={index}
          data={item}
          handleProductChange={handleProductChange} // <-- pass callback handler
        />)
      )}
    </ul>
  );
}

ReactDOM.render(
  <App />,
  document.getElementById('container')
);

Working jsfiddle demo

Drew Reese
  • 165,259
  • 14
  • 153
  • 181
  • Thanks Drew! That totally makes sense! Great tips too! – lucasbittar Jul 10 '20 at 12:45
  • 1
    @Drew Reese thanks for this explanation. When you told: "Move ProductForm externally" my first instinct was to move also `handleProductChange` and to pass `setProducts` and `products` into `` call. On that way `handleProductChange` can stay above in ProductForm function. I assume your approach with leaving `handleProductChange` in App is better, but can you give me reasons or explain why? :) – Predrag Davidovic May 18 '21 at 05:12
  • 1
    @PredragDavidovic You could certainly pass the state and state updater function to consuming components, but then you are relying on them to maintain the state invariant when updating. In other words, each component calling `setProducts` has to ensure it is updating state correctly and not mutating it. By keeping `handleProductChange` with the component with the state you are better able to control *how **and** where* state is updated, consuming components need only pass the data that needs to be added to state. – Drew Reese May 18 '21 at 05:21
  • Sorry for bothering, but can you explain me what this means: “but then you are relying on them to maintain the state invariant when updating.”? Thanks in advance :) – Predrag Davidovic May 18 '21 at 05:34
  • 1
    @PredragDavidovic You can define it once in `handleProductChange` and pass that around and you can count that it will work the same way, every time, ***or*** you can pass `setProducts` and each component would need to implement the same (*or very similar update function*) and there's a greater chance a mistake is made. When you do the latter you are making it a consuming component's responsibility (delegation) to update state, and *hopefully* they do it correctly. Hopefully the benefit of passing a callback over the direct state updater function is more clear. – Drew Reese May 18 '21 at 05:41
1

One way to solve the issue would be to have a separate component for <ProductForm /> and pass the required props there.

const ProductForm = ({ data, index, products, setProducts }) => {
    const handleProductChange = (name, value) => {
      const allProducts = [...products];
      const selectedProduct = {...allProducts[index]};
      allProducts[index] = {
        ...selectedProduct,
        [name]: value
      };
      setProducts([ ...allProducts ]);
    }
    
    return (
      <li>
        <h2>{data.title}</h2>
        <label>Price:</label>
        <input
          type="text"
          value={products[index].price || ''}
          onChange={(e) => handleProductChange('price', e.target.value)}
        />
      </li>
    );
}

const App = () => {
  const [products, setProducts] = React.useState([{title: 'My Product'},{title: 'My Product 2'}]);
  return (
    <ul>
      {products.map((item, index) => 
         <ProductForm
           key={item.title}
           index={index}
           data={item}
           products={products}
           setProducts={setProducts}
         />
      )}
     </ul>
  )
}

ReactDOM.render(
  <App />,
  document.getElementById('container')
);

Here is the working JSFiddle code link

himayan
  • 784
  • 3
  • 9