Reactjs - React: Clean way to define useCallback for functions taking arguments in loop

tobiasBora
June 16, 2023
132 views
0 votes
2 Answers

The problem

In react, I really often need to memoize functions in lists of items (created via loops) using something like useCallback to avoid re-rendering all components if a single element changes, due to a mismatching reference identifier… Unfortunately, this is surprisingly hard to due. For instance, consider this code:

const MyComp = memo({elements} => {
  {
    elements.map((elt, i) => {
      <>{elt.text}<Button onClick={(e) => dispatch(removeElement({id: i}))}> <>
    })
  }
})

where Button is an external component provided by, e.g., ant design. Then, this function reference will be different at every render since it is inlined, hence forcing a re-render.

A (bad) solution

To avoid that issue, I can think of one other solution: create a new component MyButton, that accepts two props index={i} and onClick instead of a single onClick, and that appends the arguement index to any call to onClick:

const MyButton = ({myOnClick, id, ...props}) => {
  const myOnClickUnwrap = useCallback(e => myOnClick(e, id), [myOnClick]);
  return <Button onClick={myOnClickUnwrap} ...props/>
};

const MyComp = memo({elements} => {
  const myOnClick = useCallback((e, id) => dispatch(removeElement({id: id})), []);
  return 
    {
      elements.map((elt, i) => {
      <>{elt.text}<Button id={i} onClick={myOnClick}> <>
    })
  }
)

Why I want a better approach

While this does work, this is highly non-practical for many reasons:

the code is cluttered
I need to wrap all elements from external libraries like Button and rewrite components that were not meant to deal with this kind of nesting… which defeats modularity and complicates the code even more
this composes poorly: if I want to nest elements inside multiple lists, it will be even dirtier as I need to add one new index per level of list like <MyButton index1={index1} index2={index2} index3={index3} onClick={myFunction}>, which means that I need in full generality to create an even more complicated version of MyButton to check the number of nested level. I can’t use index={[index1, index2, index3]} as this is an array and has therefore no stable reference.
as far as I see there is no convention an the naming of indexes, which means that it is harder to share code between projects or develop libraries

Is there a better solution I am missing? Given how omnipresent lists are, I can’t believe that there is no proper solution for this, and I’m surprised to see very little documentation on this.

Edit
I tried to do:

// Define once:
export const WrapperOnChange = memo(({onChange, index, Component, ...props}) => {
    const onChangeWrapped = useCallback(e => onChange(e, index), [onChange, index]);
    return <Component {...props} onChange={onChangeWrapped} />
});

export const WrapperOnClick = memo(({onClick, index, Component, ...props}) => {
    const onClickWrapped = useCallback(e => onClick(e, index), [onClick, index]);
    return <Component {...props} onClick={onClickWrapped} />
});

and to use it like:

const myactionIndexed = useCallback((e, i) => dispatch(removeSolverConstraint({id: i})), []);
return <WrapperOnClick index={i} Component={Button} onClick={myactionIndexed} danger><CloseCircleOutlined /></WrapperOnClick>

but this is still not perfect, in particular I need one wrapper for different nested levels, I need to create a new version whenever I target a new attribute (onClick, onChange, …), it would not work directly if I have multiple attributes (e.g. both onClick and onChange), and I never saw this before so I guess there is a better solution.

edit
I tried various ideas, including using fast-memoize, but I still don’t understand all the results: sometimes, fast-memoize works, while sometimes it fails… And I don’t know if fast-memoize is the recommended solution: seems weird to use a third-party tool for such a common usecase. See my tests here https://codesandbox.io/embed/magical-dawn-67mgxp?fontsize=14&hidenavigation=1&theme=dark

Tags: react-hooks reactjs

Answers

Chosen as BEST ANSWER

WARNING: I’m not a react expert (hence my question!), so please comment below and/or add +1 if you think this solution is the canonical way to proceed in React (or -1 of not ^^). I’m also curious to understand why some other solutions failed (e.g. based on proxy-memoize (that is actually 10x longer than without cache, and that does not cache at all) or fast-memoize (that do not always cache depending on how I use it)), so if you know I’m interested to know)

Since I got little interest in this question, I tried to benchmark a bunch of solutions (14!), depending on various choices (no memoization, using on external library (fast-memoize vs proxy-memoize), using a Wrapper, using an external component etc…

The best approach seems to be to create a new component containing the whole element of the list, and not only the final button for instance. This allows a reasonably clean code (even if I need to create two components for the list and for the items, at least it semantically makes sense), avoids external libraries, and seems to be more efficient than all other approaches I tried (at least on my example):

const Version13Aux = memo(({ onClickIndexed, index, e }) => {
  const action = useCallback((e) => onClickIndexed(e, index), [
    index,
    onClickIndexed
  ]);
  return (
    <li>
      <Text>{e}</Text>
      <Button onClick={action}>Delete</Button>
    </li>
  );
});

const Version13 = memo(({ deleteElement, elements }) => {
  const actionOnClickIndexed = useCallback((e, i) => deleteElement(i), [
    deleteElement
  ]);
  return (
      <ul>
        {elements.map((e, i) => (
          <Version13Aux index={i} onClickIndexed={actionOnClickIndexed} e={e} />
        ))}
      </ul>
  );
});

I still don’t incredibly like this solution as I need to forward many stuff from the parent component to the children component, but it seems like it’s the best solution I can get…

You can see my list of attempts here, where I used the code below. And here is the view of the profiler (technically there is not a huge difference in term of time between all versions (except for version 7 that uses proxy-memoize, that I removed because it was wayyyyy longer, maybe 10x and was making the graph harder to read), but I expect this difference to be much larger on longer lists where items are more complicated to draw (here I just have one text and one button). Note that all versions are not exactly the same (some use <button>, some </Button>, some normal lists, some Ant design lists…), so time comparison only make sense between versions that do the same thing. Anyway, I mostly care about seeing what is cached and what is not, which is clearly visible in the profiler (light grey blocks are cached):

Another interesting fact is that you might want to benchmark before memoizing, as the improvement might not be significant, at least for simple components (here size 5, only one text and one button).

import "./styles.css";
import { Button, List, Typography } from "antd";
import { useState, useCallback, memo, useMemo } from "react";
import { memoize, memoizeWithArgs } from "proxy-memoize";
import memoizeFast from "fast-memoize";
const { Text } = Typography;

const Version1 = memo(({ deleteElement, elements }) => {
  return (
    <>
      <h2>
        Version 1: naive version that should be inneficient (normal button)
      </h2>
      <p>
        Interestingly, since button is not a component, but a normal html
        component, nothing is redrawn.
      </p>
      <ul>
        {elements.map((e, i) => (
          <li>
            <Text>{e}</Text>{" "}
            <button onClick={(e) => deleteElement(i)}>Delete</button>
          </li>
        ))}
      </ul>
    </>
  );
});

const Version2 = memo(({ deleteElement, elements }) => {
  return (
    <>
      <h2>
        Version 2: naive version that should be inneficient (Ant design button)
      </h2>
      <p>
        Using for instance Ant Design's component instead of button shows the
        issue. Because onClick is inlined, the reference is different on every
        call which triggers a redraw.
      </p>
      <ul>
        {elements.map((e, i) => (
          <li>
            <Text>{e}</Text>{" "}
            <Button onClick={(e) => deleteElement(i)}>Delete</Button>
          </li>
        ))}
      </ul>
    </>
  );
});

const Version3AuxButton = memo(({ onClickIndexed, index }) => {
  const action = (e) => onClickIndexed(e, index);
  return <Button onClick={action}>Delete</Button>;
});

const Version3 = memo(({ deleteElement, elements }) => {
  const actionOnClickIndexed = useCallback((e, i) => deleteElement(i), [
    deleteElement
  ]);
  return (
    <>
      <h2>Version 3: works but really dirty (needs a new wrapper)</h2>
      <p>
        This works, but I don't like this solution because I need to manually
        create a new component, which makes the code more complicated, and it
        composes poorly since I need to create a new version for every
        nested-level.
      </p>
      <ul>
        {elements.map((e, i) => (
          <li>
            <Text>{e}</Text>
            <Version3AuxButton
              index={i}
              onClickIndexed={actionOnClickIndexed}
            />
          </li>
        ))}
      </ul>
    </>
  );
});

// We try to create a wrapper to automatically do the above code
const WrapperOnClick = memo(
  ({ onClickIndexed, index, Component, ...props }) => {
    const onClickWrapped = useCallback((e) => onClickIndexed(e, index), [
      onClickIndexed,
      index
    ]);
    return <Component {...props} onClick={onClickWrapped} />;
  }
);

const Version4 = memo(({ deleteElement, elements }) => {
  const actionOnClickIndexed = useCallback((e, i) => deleteElement(i), [
    deleteElement
  ]);
  return (
    <>
      <h2>Version 4: using a wrapper, it does work</h2>
      <p>
        Using a wrapper gives slightly less ugly code (at least I don’t need to
        redefine one wrapper per object), but still it’s not perfect (need to
        improve it to deal with nested level, different names (onChange,
        onClick, myChange…), multiple elements (what if you have both onClick
        and onChange that you want to update?), and still I don't see how to use
        it with List.item from Ant Design)
      </p>
      <ul>
        {elements.map((e, i) => (
          <li>
            <Text>{e}</Text>{" "}
            <WrapperOnClick
              Component={Button}
              index={i}
              onClickIndexed={actionOnClickIndexed}
            >
              Delete
            </WrapperOnClick>
          </li>
        ))}
      </ul>
    </>
  );
});

const Version5naive = memo(({ deleteElement, elements }) => {
  return (
    <>
      <h2>
        Version 5 naive: using no wrapper but List from Ant design. I don’t
        cache anything nor use usecallback: it does NOT work
      </h2>
      <p>
        Sometimes, with this version I got renders every second without apparent
        reason. Not sure why I don’t have this issue here.
      </p>
      <List
        header={<div>Header</div>}
        footer={<div>Footer</div>}
        bordered
        dataSource={elements}
        renderItem={(e, i) => (
          <List.Item>
            <Text>{e}</Text>{" "}
            <Button onClick={(e) => deleteElement(i)}>Delete</Button>
          </List.Item>
        )}
      />
    </>
  );
});

const Version5 = memo(({ deleteElement, elements }) => {
  const header = useMemo((e) => <div>Header</div>, []);
  const footer = useMemo((e) => <div>Footer</div>, []);
  const renderItem = useCallback(
    (e, i) => (
      <List.Item>
        <Text>{e}</Text>{" "}
        <Button onClick={(e) => deleteElement(i)}>Delete</Button>
      </List.Item>
    ),
    [deleteElement]
  );
  return (
    <>
      <h2>
        Version 5: like version 5 naive (using no wrapper but List from Ant
        design) with an additional useCallback: it does NOT work
      </h2>
      <p></p>
      <List
        header={header}
        footer={footer}
        bordered
        dataSource={elements}
        renderItem={renderItem}
      />
    </>
  );
});

const Version6 = memo(({ deleteElement, elements }) => {
  const header = useMemo((e) => <div>Header</div>, []);
  const footer = useMemo((e) => <div>Footer</div>, []);
  const actionOnClickIndexed = useCallback((e, i) => deleteElement(i), [
    deleteElement
  ]);
  const renderItem = useCallback(
    (e, i) => (
      <List.Item>
        <Text>{e}</Text>{" "}
        <WrapperOnClick
          Component={Button}
          index={i}
          onClickIndexed={actionOnClickIndexed}
        >
          Delete
        </WrapperOnClick>
      </List.Item>
    ),
    [actionOnClickIndexed]
  );
  return (
    <>
      <h2>Version 6: using a wrapper + List</h2>
      <p>
        This kind of work… at least the button seems to be cached, but not
        perfect as it shares all issues of the wrappers. I’m also unsure how to,
        e.g., memoize the whole item, and not just the button.
      </p>
      <List
        header={header}
        footer={footer}
        bordered
        dataSource={elements}
        renderItem={renderItem}
      />
    </>
  );
});

const Version7 = memo(({ deleteElement, elements }) => {
  const header = useMemo((e) => <div>Header</div>, []);
  const footer = useMemo((e) => <div>Footer</div>, []);
  const renderItem = useMemo(
    () =>
      // if we use memoizeWithArgs from proxy-memoize instead, the preprocessing is much longer
      //  and does not even work.
      memoizeWithArgs((e, i) => (
        <List.Item>
          <Text>{e}</Text>{" "}
          <Button onClick={(e) => deleteElement(i)}>Delete</Button>
        </List.Item>
      )),
    [deleteElement]
  );
  return (
    <>
      <h2>
        Version 7: using no wrapper but memoizeWithArgs from proxy-memoize: it
        does NOT work, wayyy longer than anything else.
      </h2>
      <p>
        I don't know why, but using proxy-memoize gives a much bigger render
        time, and does not even cache the elements.
      </p>
      <List
        header={header}
        footer={footer}
        bordered
        dataSource={elements}
        renderItem={renderItem}
      />
    </>
  );
});

const Version8 = memo(({ deleteElement, elements }) => {
  const header = useMemo((e) => <div>Header</div>, []);
  const footer = useMemo((e) => <div>Footer</div>, []);
  const renderItem = useMemo(
    () =>
      // if we use memoizeWithArgs from proxy-memoize instead, the preprocessing is much longer
      //  and does not even work.
      memoizeFast((e, i) => (
        <List.Item>
          <Text>{e}</Text>{" "}
          <Button onClick={(e) => deleteElement(i)}>Delete</Button>
        </List.Item>
      )),
    [deleteElement]
  );
  return (
    <>
      <h2>
        Version 8: using no wrapper but memoize from fast-memoize: it does work
      </h2>
      <p></p>
      <List
        header={header}
        footer={footer}
        bordered
        dataSource={elements}
        renderItem={renderItem}
      />
    </>
  );
});

const Version9 = memo(({ deleteElement, elements }) => {
  const computeElement = useMemo(
    () =>
      memoizeFast((e, i) => (
        <li>
          <Text>{e}</Text>{" "}
          <Button onClick={(e) => deleteElement(i)}>Delete</Button>
        </li>
      )),
    [deleteElement]
  );
  return (
    <>
      <h2>
        Version 9: like version 2, but use fast-memoize on whole element: does
        NOT work
      </h2>
      <p>I don't understand why this fails while Version 8 works.</p>
      <ul>{elements.map(computeElement)}</ul>
    </>
  );
});

const Version10 = memo(({ deleteElement, elements }) => {
  const del = useMemo(() => memoizeFast((i) => (e) => deleteElement(i)), [
    deleteElement
  ]);
  return (
    <>
      <h2>
        Version 10: like version 2 (+Text), but use fast-memoize only on delete
      </h2>
      <p>
        I don't understand why this fails while Version 8 works (but to be
        honest, I'm not even sure if it fails, since buttons sometimes just
        don't appear at all, while other renders from scratch without saying
        why): to be more precise, it does not involve caching from the library…
        or maybe this kind of cache is not shown by the tools since it is done
        by another external library? But then, why are the item grey in version
        8?
      </p>
      <ul>
        {elements.map((e, i) => (
          <li>
            <Text>{e}</Text> <Button onClick={del(i)}>Delete</Button>
          </li>
        ))}
      </ul>
    </>
  );
});

const Version11 = memo(({ deleteElement, elements }) => {
  const del = useMemo(() => memoizeFast((i) => (e) => deleteElement(i)), [
    deleteElement
  ]);
  const computeElement = useMemo(
    () =>
      memoizeFast((e, i) => (
        <li>
          <Text>{e}</Text> <Button onClick={del(i)}>Delete</Button>
        </li>
      )),
    [del]
  );
  return (
    <>
      <h2>Version 11: like version 9 + 10, does NOT work</h2>
      <p>Not sure why it fails, even worse than 9 and 10 separately.</p>
      <ul>{elements.map(computeElement)}</ul>
    </>
  );
});

const Version12 = memo(({ deleteElement, elements }) => {
  const MemoizedList = useMemo(
    () => () => {
      return elements.map((e, i) => (
        <li key={e}>
          <Text>{e}</Text>{" "}
          <Button onClick={(e) => deleteElement(i)}>Delete</Button>
        </li>
      ));
    },
    [elements, deleteElement]
  );
  return (
    <>
      <h2>Version 12: memoize the whole list: not what I want</h2>
      <p>
        Answer proposed in
        https://stackoverflow.com/questions/76446359/react-clean-way-to-define-usecallback-for-functions-taking-arguments-in-loop/76462654#76462654,
        but it fails as if a single element changes, the whole list is redrawn.
      </p>
      <ul>
        <MemoizedList />
      </ul>
    </>
  );
});

const Version13Aux = memo(({ onClickIndexed, index, e }) => {
  const action = useCallback((e) => onClickIndexed(e, index), [
    index,
    onClickIndexed
  ]);
  return (
    <li>
      <Text>{e}</Text>
      <Button onClick={action}>Delete</Button>
    </li>
  );
});

const Version13 = memo(({ deleteElement, elements }) => {
  const actionOnClickIndexed = useCallback((e, i) => deleteElement(i), [
    deleteElement
  ]);
  return (
    <>
      <h2>
        Version 13: simple list (not Ant): works but I don’t like the fact that
        we need to create auxiliary elements.
      </h2>
      <p>
        This works, but I don't like this solution because I need to manually
        create a new component, which can make the code more complicated.
      </p>
      <ul>
        {elements.map((e, i) => (
          <Version13Aux index={i} onClickIndexed={actionOnClickIndexed} e={e} />
        ))}
      </ul>
    </>
  );
});

const Version14Aux = memo(({ onClickIndexed, index, e }) => {
  const action = useCallback((e) => onClickIndexed(e, index), [
    index,
    onClickIndexed
  ]);
  return (
    <List.Item>
      <Text>{e}</Text> <Button onClick={action}>Delete</Button>
    </List.Item>
  );
});

const Version14 = memo(({ deleteElement, elements }) => {
  const actionOnClickIndexed = useCallback((e, i) => deleteElement(i), [
    deleteElement
  ]);

  const header = useMemo((e) => <div>Header</div>, []);
  const footer = useMemo((e) => <div>Footer</div>, []);
  const renderItem = useCallback(
    (e, i) => (
      <Version14Aux index={i} onClickIndexed={actionOnClickIndexed} e={e} />
    ),
    [actionOnClickIndexed]
  );
  return (
    <>
      <h2>Version 14: like version 13, but for Ant lists</h2>
      <p>
        This works, but I don't like this solution so much because I need to
        manually create a new component, which can make the code slightly more
        complicated. But it seems the most efficient solution (better than
        memoize etc), and the code is still not too bloated while avoiding third
        party libraries… So it might be the best solution.
      </p>
      <List
        header={header}
        footer={footer}
        bordered
        dataSource={elements}
        renderItem={renderItem}
      />
    </>
  );
});

const initialState = ["Egg", "Milk", "Potatoes", "Tomatoes"];
export default function App() {
  const [elements, setElements] = useState(initialState);
  const restart = useCallback((e) => setElements(initialState), []);
  const deleteElement = useCallback(
    (index) => setElements((elts) => elts.filter((e, i) => i !== index)),
    []
  );
  return (
    <div className="App">
      <h1>Trying to avoid redraw</h1>
      <button onClick={restart}>Restart</button>
      <Version1 elements={elements} deleteElement={deleteElement} />
      <Version2 elements={elements} deleteElement={deleteElement} />
      <Version3 elements={elements} deleteElement={deleteElement} />
      <Version4 elements={elements} deleteElement={deleteElement} />
      <Version5naive elements={elements} deleteElement={deleteElement} />
      <Version5 elements={elements} deleteElement={deleteElement} />
      <Version6 elements={elements} deleteElement={deleteElement} />
      <Version8 elements={elements} deleteElement={deleteElement} />
      <Version9 elements={elements} deleteElement={deleteElement} />
      <Version10 elements={elements} deleteElement={deleteElement} />
      <Version11 elements={elements} deleteElement={deleteElement} />
      <Version12 elements={elements} deleteElement={deleteElement} />
      <Version13 elements={elements} deleteElement={deleteElement} />
      <Version14 elements={elements} deleteElement={deleteElement} />
      {
        // Version 7 is soo long that I need to put it in the end or
        // on the profiler I can’t click on other items that
        // are too close to the scroll bar
        // <Version7 elements={elements} deleteElement={deleteElement} />
      }
    </div>
  );
}

(Edit)

First of all, it is not recommended to use index as a parameter or props or key, because when you delete the first one, all subcomponents will be re-rendering.
And according to your scenario if you want to avoid re-rendering, I have some ideas, you can For reference, like this:

const WrapperEvent = (Component) => {
  return memo(function Hoc({ onClick, onChange, onOtherEvent, eventData, ...restProps }) {
    return (
      <Component onClick={() => onClick?.(eventData)} onChange={() => onChange?.(eventData)} onOtherEvent={() => onOtherEvent?.(eventData)} {...restProps} />
    )
  })
}

const WrapperButton = WrapperEvent(MyButton)

const Version2 = memo(({ deleteElement, elements }) => {
  return (
    <>
      <ul>
        {elements.map((e) => (
          <li key={e}>
            <Text>{e}</Text>{" "}
            <WrapperButton eventData={e} onClick={deleteElement}>Delete</WrapperButton>
          </li>
        ))}
      </ul>
    </>
  );
});

const initialState = ["Egg", "Milk", "Potatoes", "Tomatoes"].concat(
  [...Array(0).keys()].map((e) => e.toString())
);
export default function App() {
  const [elements, setElements] = useState(initialState);
  const restart = useCallback((e) => setElements(initialState), []);
  const deleteElement = useCallback(
    (name) => setElements((elts) => elts.filter((e, i) => e !== name)),
    []
  );
  return (
    <div className="App">
      <h1>Trying to avoid redraw</h1>
      <button onClick={restart}>Restart</button>
      <Version2 elements={elements} deleteElement={deleteElement} />
    </div>
  );
}

The test here https://codesandbox.io/s/sharp-wind-rd48q4?file=/src/App.js

Please signup or login to give your own answer.

Click here to cancel reply.

Reactjs – React: Clean way to define useCallback for functions taking arguments in loop

The problem

A (bad) solution

Why I want a better approach

Answers