jsx-eslint/eslint-plugin-react

[jsx-no-constructed-context-values]: Wrongly(?) saying factory functions need to memoize an out of scope variable + comment wrongly identified as value that needs memoization

Open

#3116 aperta il 28 ott 2021

Vedi su GitHub
 (2 commenti) (0 reazioni) (0 assegnatari)JavaScript (2797 fork)batch import
help wantedtypescript

Metriche repository

Star
 (8630 star)
Metriche merge PR
 (Nessuna PR mergiata in 30 g)

Descrizione

Description When using the rule (eslint-plugin-)react/jsx-no-constructed-context-values, it (wrongly?) reports a value from a factory function as a value that must be memoized. If you are also using (eslint-plugin-)react-hooks/exhaustive-deps and attempt to memoize the value to fix the lint error from jsx-no-constructed-context-values, the exhaustive-deps rule from react hooks will warn/error that out of scope variables are not valid dependencies.

I also discovered an additional issue while creating the repro. If you put a prop in say a context provider with a comment, like so (also in below code): <MyContext.Provider value={{ /* some comment */ }}>, then this will be wrongly identified as a value that needs to be memoized (from jsx-no-constructed-context-values).

Repro https://github.com/AllySummers/eslint-react-memo-factory The issue can be seen from running npm run lint (in addition to using in-editor ESLint)

not-working.tsx

import React from 'react';

const UserContext = React.createContext({ user: null });

const UserContextProvider = ({ children }) => {
  return (
    <UserContext.Provider value={{ /* this is a comment */ }}> {/* Error: The object passed as the value prop to the Context provider (at line 7) changes every render. To fix this consider wrapping it in a useMemo hook. */}
      { children }
    </UserContext.Provider>
  )
};

const userFactoryWrapper1 = (user) => ({ children }) => (
  <UserContext.Provider value={{ user }}> {/* Error: The object passed as the value prop to the Context provider (at line 14) changes every render. To fix this consider wrapping it in a useMemo hook. */}
    { children }
  </UserContext.Provider>
)

const userFactoryWrapper2 = (user) => ({ children }) => {
  const memoizedValue = React.useMemo(() => ({
    user
  }), [user]); // Error: React Hook React.useMemo has an unnecessary dependency: 'user'. Either exclude it or remove the dependency array. Outer scope values like 'user' aren't valid dependencies because mutating them doesn't re-render the component.

  return (
    <UserContext.Provider value={memoizedValue}>
      { children }
    </UserContext.Provider>
  )
}

ESLint Config

{
  "root": true,
  "parserOptions": {
    "ecmaFeatures": {
      "jsx": true
    },
    "ecmaVersion": 12,
    "sourceType": "module"
  },
  "plugins": [
    "react",
    "react-hooks"
  ],
  "rules": {
    "react/jsx-no-constructed-context-values": "error",
    "react-hooks/exhaustive-deps": "warn"
  }
}

Expected Result

Typescript eslint should not issue a warning as the suggested code (as below)

Actual Result

The code provided in working.tsx is required due to how generics are parsed in Typescript/React, one cannot simply write <T>.

Additional Info The attempted fix is invalid code:

export const GenericComponent = <T, = unknown>(props: GenericComponentProps<T>) => (
  <div>hello</div>
);

Versions

package version
node 14.16.0
eslint 7.32.0
eslint-plugin-react 7.26.1
eslint-plugin-react-hooks 4.2.0

Guida contributor