Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A note on function components default parameters #16905

Closed
melloc01 opened this issue Sep 26, 2019 · 7 comments
Closed

A note on function components default parameters #16905

melloc01 opened this issue Sep 26, 2019 · 7 comments

Comments

@melloc01
Copy link

melloc01 commented Sep 26, 2019

When you define default parameters for a function component, the component will have brand new parameters on every render (function call) - in case they aren't defined.
Turns out that by doing this way we are not actually defining defaultProps (as possibly wanted) but creating objects in every function call, causing an infinite loop if Hook dependency array relies on the default parameter.

Maybe not a real issue here, but found that today and felt there should be a note about this somewhere in the docs.

Relevant code:

See: https://codesandbox.io/s/jest-test-mfz1z?fontsize=14

// index.test.js:
it("renders", () => {
  const container = render(<SimpleComponent />);

  expect(1).toBe(1);
});

// SimpleComponent.js:
import React from "react";

let x;
let tries = 0;

// a new array will be created in every render 
export default function SimpleComponent({ openedList = [] }) {
  const [opened, setOpened] = React.useState([]);

  React.useEffect(() => {
    console.warn({ x, openedList });
    console.warn("equality = ", x === openedList); // always false
    x = openedList;

    tries = tries + 1;

    if (tries > 100) throw new Error("This is an infinite loop");

    setOpened(openedList);
  }, [openedList]);

  return null;
}
@bvaughn
Copy link
Contributor

bvaughn commented Sep 26, 2019

This issue has come up in a few discussions around default parameters, and our recommendation has generally been to use a stable "empty array" value (or object, or no-op function), if this is a concern - e.g.

const emptyArray = [];

function Example({ optionalArray = emptyArray }) {
  // ...
}

Our docs are open source, also on GitHub, and PRs that improve them are always welcome from the community!

https://github.com/reactjs/reactjs.org

@bvaughn
Copy link
Contributor

bvaughn commented Sep 26, 2019

I'm going to close this issue but a PR would be welcome in the other repo!

@bvaughn bvaughn closed this as completed Sep 26, 2019
@melloc01
Copy link
Author

Cool @bvaughn - (kudos on the Profiler btw!)

Using a scope object like this could cause (in my real use case) a bug when the array is mutated by an instance and next renders would have the mutated array as default argument. I ended up using defaultProps with a getter like so:

Example.defaultProps = { get optionalArray () { return [] } }

I'll create a PR on the docs repo 💯

@nortonwong
Copy link

A defaultProps getter still results in a potential infinite loop when <SimpleComponent />'s props are updated from its parent. It may only hide the issue.

Mutation is discouraged in props. It sounds like a useRef might better suit your use case.

@bvaughn
Copy link
Contributor

bvaughn commented Sep 27, 2019

Using a scope object like this could cause (in my real use case) a bug when the array is mutated by an instance

Props should always be read-only. This is one of the core rules of React:
https://reactjs.org/docs/components-and-props.html#props-are-read-only

If your component needs to mutate an incoming array, it could store a shallow clone of the array in state and mutate that? e.g.

const emptyArray = [];

function Example({ optionalArray = emptyArray }) {
  const [editableArray, setEditableArray] = useState(() => Array.from(optionalArray));
  // ...
}

@melloc01
Copy link
Author

True that @nortonwong, on every new props that component would have also a new array as a default param.

@bvaughn I wasn't aware that useState could receive a function as argument - I believe it only calls once to get the first parameter, right? Cool - fits perfectly :) thanks guys 👍

@bvaughn
Copy link
Contributor

bvaughn commented Sep 29, 2019

Yup! It only calls the initializer function when the state is initialized.

https://reactjs.org/docs/hooks-reference.html#lazy-initial-state

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants