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

jsx-no-constructed-context-values: class instance false positive #3295

Closed
mckravchyk opened this issue May 23, 2022 · 3 comments · Fixed by #3448
Closed

jsx-no-constructed-context-values: class instance false positive #3295

mckravchyk opened this issue May 23, 2022 · 3 comments · Fixed by #3448

Comments

@mckravchyk
Copy link

mckravchyk commented May 23, 2022

Consider the following code:

const app = new App(); // The 'app' new expression (at line 13) passed as the value prop to the Context provider (at line 22) changes every render. To fix this consider wrapping it in a useMemo hook.eslint[react/jsx-no-constructed-context-values](https://github.com/yannickcr/eslint-plugin-react/tree/master/docs/rules/jsx-no-constructed-context-values.md)

const root = ReactDOM.createRoot(document.getElementById('root'));

root.render(
  <React.StrictMode>
    <AppContext.Provider value={app}>
      <AppView />
    </AppContext.Provider>
  </React.StrictMode>,
);

The rule docs explain:

React Context, and all its child nodes and Consumers are rerendered whenever the value prop changes. Because each Javascript object carries its own identity, things like object expressions ({foo: 'bar'}) or function expressions get a new identity on every run through the component. This makes the context think it has gotten a new object and can cause needless rerenders and unintended consequences.

The rule description only mentions object literals and function expressions and it does not seem to me like React would re-initialize previously initialized class instance on a re-render.
`

@ljharb
Copy link
Member

ljharb commented May 23, 2022

Since there's no components defined in this code, I wouldn't expect that rule to warn. This seems like an oversight.

@greatsage-raphael
Copy link

greatsage-raphael commented Jun 20, 2022

@ljharb Could I take on this issue

@ljharb
Copy link
Member

ljharb commented Jun 21, 2022

Go for it!

golopot added a commit to golopot/eslint-plugin-react that referenced this issue Oct 3, 2022
golopot added a commit to golopot/eslint-plugin-react that referenced this issue Oct 3, 2022
golopot added a commit to golopot/eslint-plugin-react that referenced this issue Oct 3, 2022
@ljharb ljharb closed this as completed in d0da6bf Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants