[New] jsx-no-constructed-context-values: add rule for unstable context values

[dylanOshima]

Rule JSX-no-constructed-context-values

Summary

Adds a new rule that checks if the value prop passed to a Context Provider will cause needless rerenders. A common example is inline object declaration such as:

<Context.Provider value={{foo: 'bar'}}>
...
</Context.Provider>

which will cause the Context to rerender every time this code is run because the object is defined with a new reference. This is true for many other types of JS objects and variables.

Rationale

This can lead to performance hits since not only will this rerender all the children of a context, it will also update all consumers of the value. The search React does to search the tree can also be very expensive. Some other instances that this rule covers are: array declarations, function declarations (i.e. arrow functions), or new class object instantiates.

Possible Fixes

For inline values this can be fixed by wrapping the in-line declaration with useMemo or useCallback as the lint rule suggests. I think this can depend on the usage and specific implementation and thus an auto-fix rule is not provided. For the example above this would look like:

const contextVal = useMemo(() => {foo: 'bar'}, []);
<Context.Provider value={contextVal}>
...
</Context.Provider>

Improvements

1. It currently doesn't provide any possible auto-fixes, although it provides two suggestions: useCallback for functions declarations/definitions, and useMemo for everything else. I was considering suggesting moving the reference outside of the component, but like I mentioned before I feel this can be case-by-case and I'm hesitant making this an auto-fix rule.
2. It doesn't handle variables being reassigned. It's noted in one of the tests and I would greatly appreciate help if there's an easy fix I'm not seeing. For example:

function Component(foo) {
  let bar = 10;
  bar = {};
  return (<Context.Provider value={bar}></Context.Provider>);
}
// Does not fire the rule because it was initialized to a Literal

3. Not sure if this should be a recommended lint rule, I decided to be more conservative and chose not to.

Contributions

Thanks to @captbaritone for noticing the issue and creating the React hooks version which this is largely based off of, and huge thanks to @bradzacher for suggesting I make this rule open source here 👍

[captbaritone]

Looks like tests are failing on ESLint version 4

[captbaritone]

Interesting! If destructing is not supported, I wonder if there's a lint rule we could add to this repo's eslint config that would disallow it.

That would certainly make it easier for the next person.

[ljharb]

Is this rule only useful with React 16.3+'s Context? What about regular context?

[dylanOshima]

Is this rule only useful with React 16.3+'s Context? What about regular context?

@ljharb This rule was built with React 16.3's contexts in mind and that's where the test coverage mainly lies. But its functionality is general in that it will detect any value that could change identity. Could you elaborate more on "regular contexts"?

[ljharb]

I mean like, pre-16.3 React context, like the getChildContext class component method in particular.

[bradzacher]

This rule won't work for pre <16.3 contexts because that mechanism is fundamentally different to the context used for 16.3+

I don't know too much about the legacy context API, but reading the old docs, I believe it only lets you pass objects down the chain, returned from the method.
I'd assume that API compares the objects by property rather than equality (or I'd hope so, considering its a lifecycle style method which always returns a new object).

[ljharb]

Fair enough.

[dylanOshima]

Hi @ljharb just wanted to double check if there was still things that needed changing.

[captbaritone]

Following up here to see if there's anything else needed before this can be merged.

[ljharb]

Rebased this; I think it looks good but I want to give it another look.

[mqklin]

It's not released yet, right? I can see it in README, but eslint says that Definition for rule 'react/jsx-no-constructed-context-values' was not found

[ljharb]

Yes - check the readme on the latest tag, not on master, to see what’s released.