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

Document Why a "complex expression in the dependency array" Is a Problem #3076

Open
TomPridham opened this issue Jun 30, 2020 · 2 comments
Open

Comments

@TomPridham
Copy link

this is about the exhaustive-deps eslint rule, specifically her: https://github.com/facebook/react/blob/master/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js#L779. i've been trying to find out why having a complex expression inside of the dependency array is bad, but haven't found anything saying why something like [err?.message] would cause issues. is there somewhere it's listed in the docs that i haven't seen?

@TomPridham TomPridham changed the title Document Why a "complex expression in the dependency array" is a Problem Document Why a "complex expression in the dependency array" Is a Problem Jun 30, 2020
@rickhanlonii
Copy link
Member

Hey @TomPridham, a complex expression is anything other than a variable or property chain such as [compute(foo)], [foo[0], or [foo ? bar : baz] (see the test here). Note that as of today, an expression like err?.message would be fine (see this PR and the latest version of exhaustive-deps).

It may make sense to explain this in the note section at the bottom here, do you want to submit a PR?

@rickhanlonii
Copy link
Member

Actually a better place to document this would probably be https://reactjs.org/docs/hooks-rules.html, which is what eslint-plugin-react-hooks links to. That doc doesn't currently talk about the dependencies array, but it seems like it should, and if it did then it could explain what is and is not allowed in the array and why.

I would wait for feedback from @gaearon and @rachelnabors before making any more changes though.

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

Successfully merging a pull request may close this issue.

2 participants