top | item 44716227

(no title)

wfleming | 7 months ago

> Adding non-primitive props you get passed into your component to internal dependency arrays is rarely right, because this component has no control over the referential stability of those props.

I think this is wrong? If you memoize a callback with useCallback and that callback uses something from props without putting it in the dependency array, and then the props change & the callback runs, the callback will use the original/stale value from the props and that's almost certainly a bug.

They might be trying to say you just shouldn't use useCallback in that situation, but at best it's very confusingly written there because it sure sounds like it's saying using useCallback but omitting a dependency is acceptable.

IMHO useCallback is still a good idea in those situations presuming you care about the potential needless re-renders (which for a lot of smaller apps probably aren't really a perf issue, so maybe you don't). If component A renders component B and does not memoize its own callback that it passes to B as a prop, that is A's problem, not B's. Memoization is easy to get wrong by forgetting to memoize one spot which cascades downwards like the screenshots example, but that doesn't mean memoization is never helpful or components shouldn't do their best to optimize for performance.

discuss

order

p1necone|7 months ago

In my experience you're correct yeah - 95% of the time if you use it it should be included in the dependency array, and any edge cases where that's not true you should endeavour to understand and document. There's a 'react-hooks/exhaustive-deps' eslint rule for exactly this purpose.

tkdodo|7 months ago

author here , sorry for the confusion. I did not mean to imply that leaving out the prop from useCallback is better - you _always_ have to adhere to the exhaustive-deps lint rule to avoid stale closures (I have a separate blog post on this topic).

What I meant is that the API design of B, where the hook only works as intended if the consumer A memoizes the input props, is not ideal because A has no way of knowing that they have to memoize it unless they look at the implementation or it’s clearly documented (which it rarely is). It’s not A’s problem because B could’ve used the latest ref pattern (or, in the future, useEffectEvent) to work without passing that burden onto its consumers.

Izkata|7 months ago

> > Adding non-primitive props you get passed into your component to internal dependency arrays is rarely right, because this component has no control over the referential stability of those props.

> I think this is wrong? If you memoize a callback with useCallback and that callback uses something from props without putting it in the dependency array, and then the props change & the callback runs, the callback will use the original/stale value from the props and that's almost certainly a bug.

The problem is up a level in the tree, not in the component being defined. See the following code block, where they show how the OhNo component is used - the prop is always different.

skydhash|7 months ago

The parent is the source of truth for the children nodes. Which means don't send anything down that you don't intend to be stable. As the children will (and should) treat it as gospel. That also means that the root component is god for the whole view.

Most source of bugs happens when people ignore this simple fact. Input start from the root and get transformed along the way. Each node can bring things from external, but it should be treated careful as the children down won't care. It's all props to them.

Which also means don't create new objects out of the blue to pass down. When you need to do so, memoize. As the only reason you need to do so is because you want:

- to join two or more props or local state variables

- or to transform the value of a prop or a local state variable (in an immutable fashion, as it should be).

- or both.

strogonoff|7 months ago

I think one of the most awkward things about React (stemming from its virtue of being just a JavaScript library, as opposed to a framework with some custom DSL that could handle memoization behind the scenes) is that it has to be stated explicitly that non-primitive props should be memoized. React makes it way too easy to pass someProp={[1, 2, 3]}, yet to me it is always a caller’s bug.