React: Discussion: useEffect hook with array dependency that has a default value causes render loop

[gregor-mueller]

So maybe this is not a bug (because it's default JavaScript behavior) but a pitfall that should be documented in the useEffect-section of the hooks documentation. I'm not quite sure because it feels like react should handle this as default value assignment on destructuring functional component props is recommended.

When a useEffect-Hook that has an array-prop with a default value as one of its dependencies calls a function mutating any other prop, a render loop will occur.

React version: 16.12.0

Steps To Reproduce

1. Create a functional component that has 3 props:
   -- an array prop that has a default value (created within destructuring)
   -- any other prop (e.g. string)
   -- a callback function to mutate the second prop
2. Have an useEffect function that calls the callback to mutate the second prop. The useEffect needs to have the array prop as one of its dependencies

Link to code example:

https://codesandbox.io/s/smoosh-field-fqduq

The current behavior

When the default value for the array prop is set to an "anonymous" array (or object) inside of the props destructuring, the useEffect will always trigger and so the second prop will be changed via the callback function and a re-render will happen. Then the effect will be triggered again as the value for the array will have changed again.

The expected behavior

Default values for arrays (and objects) can be assigned within the destructuring of the props without causing re-renders (or even render loops) when the mentioned useEffect is not present (like, why should it trigger a re-render?).
I'd expect react to behave the same in this case (don't compare empty arrays/objects by reference). If not, there should be at least a warning in the docs of useEffect that this is something to look out for.

Workaround

If the default prop is saved as a variable outside the functional component and then assigned as the default value (read: if you have referential equality), the issue does not occur obviously (default JavaScript behavior).

[kunukn]

The pitfall or gotcha, is going into endless loop with code like these.

function MyComponent({ arrayProp = [], title, onEvent }) {  
  // arrayProp default prop value is a new object on every render

  useEffect(() => {
    console.log("arrayProp effect", arrayProp);
    onEvent(title + "1"); // title prop gets updated triggering a re-render, but only once

  // because this default prop is new on every render, this creates the endless loop
  }, [arrayProp]); 

  return (
    <div>
      <h1>{title}</h1>
      <p>You will run into a render loop</p>
    </div>
  );
}

One way to avoid the endless loop gotcha is to use default props like this.
But I am uncertain if this is recommended to use or not. This might be deprecated in the future I think?

function MyComponent({ arrayProp, title, onEvent }) {  
 // code ...
}

MyComponent.defaultProps = {
  arrayProp: []
}

[eps1lon]

To avoid this you can declare it outside e.g.

const defaultArrayProp = [];
function MyComponent({ arrayProp = defaultArrayProp, title, onEvent }) {  

Consumers of your component might still encounter this pitfall when using <MyComponent arrayProp={[]} /> if MyComponent triggers a re-render in its owner as well.

[gregor-mueller]

I know that you can work around this. It's actually in the example.

@kunukn it's not advised to use defaultProps for functional components, as you already figured.
The solution from @eps1lon is the one I generally go for.

But the thing is, that this is something that multiple developers ran into this pitfall already. As react is often learned by JavaScript beginners, this is something that should just be on the documentation for useEffect, I guess. I don't know whether anonymously created objects or arrays as props in render functions are something that is generally known to cause re-renders but it's something that I've seen done wrong plenty. Essentially it's the same and should be addressed as well - at least via documentation - to make using react more accessible.

As I said, this is not actually a "bug" but it feels a bit weird that react does not handle this (because everywhere you see people showing you that that's the way to do default props for functional components).

If you know how JavaScript works of course this is nothing new for you. 🤷‍♂

[tomasyaya]

@ffgregormueller I think this is "normal" behaviour. By defaulting to a declared variable, default stays the same and so dependency array checks ok. Putting an empty array declares a new one every time so the comparison in the array doesn't match.
I may be wrong, but I believe this is normal JS behaviour.

[gregor-mueller]

@tomasyaya No, you are not wrong, you are totally right! It absolutely is default JS behavior.

I changed the title of issue, as this is in fact not a bug.
But the point I tried to get across is that in some cases react intelligently takes care of comparing props and deciding whether or not to re-render.

I just wanted to start a discussion whether there should be a mechanism here as well that e.g. internally checks for arrays or objects to be empty instead of only referential equality and skips invoking the effect when this dependency hasn't changed.

Just to make using react a bit easier for beginners.

[sirlantis]

In case someone looks for alternative approaches, I created a small helper (which also allows for some type inference in case TS is used):

// emptyArray.ts
const instance = Object.freeze([]);

export default function emptyArray<T>(): T[] {
    // Ignoring the `readonly` part
    // @ts-ignore
    return instance;
}

function MyComponent({ arrayProp = emptyArray(), title, onEvent }) {  

[tejas-bontadka]

I'm sorry if 'm wrong. I think usage of memos/refs with empty array as initial value will overcome this issue.

[martinjaime]

I agree with @tejas-bontadka, I think memoizing the dependency with useMemo will solve the issue the react way.

[gregor-mueller]

Sure, this would be correct but also a bit too much hassle where just having a reference to an empty array would also help.
Note: having an empty array or even object as default is something I repeatedly see being done wrongly as mentioned in the start of the topic.
As beginners would run into this, I don't think it is very beginner-friendly to have them having to work around this (e.g. using useMemo). I guess this would just lead to beginners always wrapping everything in useMemo, trying to fight unnecessary re-renders which will just cause memory issues in the long run.

@tejas-bontadka, @martinjaime is this what you've been thinking of?

function MyComponent({ arrayProp, title, onEvent }) {  

  // this fixes the render loop but seems a bit over engineered
  // as a simple reference to an empty array would also fix it
  const arrayPropToUse = useMemo(() => {
    if (!Array.isArray(arrayProp)) return [];
    return arrayProp;
  }, [arrayProp]);

  useEffect(() => {
    console.log("arrayPropToUse effect", arrayPropToUse);
    onEvent(title + "1"); // title prop gets updated triggering a re-render, but only once
  }, [arrayPropToUse]); 

  return (
    <div>
      <h1>{title}</h1>
      <p>Feels a bit over engineered</p>
    </div>
  );
}

My whole point is not that there are no solutions to this but that this is nothing a beginner would know or work around effectively (without developing potentially bad habits like using useMemo everywhere). But that's just an assumption on my side (based on what I've seen), which should of course be discussed.

[stale[bot]]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[stale[bot]]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!