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

[Bug]: destructuring-assignment enforces destructuring of modern context #3536

Closed
2 tasks done
billyjanitsch opened this issue Feb 23, 2023 · 1 comment · Fixed by #3583
Closed
2 tasks done

[Bug]: destructuring-assignment enforces destructuring of modern context #3536

billyjanitsch opened this issue Feb 23, 2023 · 1 comment · Fixed by #3583

Comments

@billyjanitsch
Copy link

Is there an existing issue for this?

  • I have searched the existing issues and my issue is unique
  • My issue appears in the command-line and not only in the text editor

Description Overview

#2797 was recently merged and released. That PR requires any value returned from useContext to be destructured if it's an object:

function MyComponent() {
  const foo = useContext(FooContext)
  return <>{foo.val}</> // error
}

It seems like there may have been some confusion or miscommunication here. That PR was meant to close #2309, but as noted in #2309 (comment), the rule is only supposed to warn for destructuring of legacy context (i.e., not useContext).

As I understand it, the spirit of the rule is to require destructuring of the plain objects that React creates. That includes props, as well as legacy state and context (which are always stored in objects). But it doesn't seem like it should include modern context, because context can store any (single) value. That value may or may not be an object, and even if it is an object, it's not necessarily a plain object that serves as a container for conceptually separate values. For example, it could be an instance of a class (const logger = useContext(LoggerContext)) in which case destructuring doesn't make sense (or, at least, it doesn't seem within the spirit/scope of this rule to enforce that variety of destructuring).

The new behavior also results in an inconsistency between modern state and modern context, even though these are conceptually the same in terms of being able to store any single value:

function MyComponent() {
  const foo = useContext(FooContext)
  const bar = useState({val: 1})
  return (
    <>
      {foo.val} // error
      {bar.val} // no error
    </>
  )
}

Expected Behavior

I would expect this rule not to warn on values returned from useContext (or useState).

eslint-plugin-react version

v7.32.2

eslint version

v8.34.0

node version

v18.12.0

@ljharb
Copy link
Member

ljharb commented Feb 23, 2023

That's a fair point, and I'd support a PR that adds/updates test cases (and makes whatever fixes are needed) to reflect this viewpoint.

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

Successfully merging a pull request may close this issue.

2 participants