Skip to content

Commit

Permalink
[New] jsx-no-constructed-context-values: add new rule which checks …
Browse files Browse the repository at this point in the history
…when the value passed to a Context Provider will cause needless rerenders

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:
```js
<Context.Provider value={{foo: 'bar'}}>
...
</Context.Provider>
```
which will cause the Context to rerender every time this is run because the object is defined with a new reference. 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 instantians.
  • Loading branch information
Dylan Oshima authored and ljharb committed Aug 22, 2020
1 parent fd94b95 commit ac98c0f
Show file tree
Hide file tree
Showing 6 changed files with 665 additions and 0 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -8,8 +8,10 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
### Added
* [`jsx-key`]: added `checkKeyMustBeforeSpread` option for new jsx transform ([#2835][] @morlay)
* [`jsx-newline`]: add new rule ([#2693][] @jzabala)
* [`jsx-no-constructed-context-values`]: add new rule which checks when the value passed to a Context Provider will cause needless rerenders ([#2763][] @dylanOshima)

[#2835]: https://github.com/yannickcr/eslint-plugin-react/pull/2835
[#2763]: https://github.com/yannickcr/eslint-plugin-react/pull/2763
[#2693]: https://github.com/yannickcr/eslint-plugin-react/pull/2693

## [7.21.5] - 2020.10.19
Expand Down Expand Up @@ -3219,3 +3221,4 @@ If you're still not using React 15 you can keep the old behavior by setting the
[`no-adjacent-inline-elements`]: docs/rules/no-adjacent-inline-elements.md
[`function-component-definition`]: docs/rules/function-component-definition.md
[`jsx-newline`]: docs/rules/jsx-newline.md
[`jsx-no-constructed-context-values`]: docs/rules/jsx-no-constructed-context-values.md
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -175,6 +175,7 @@ Enable the rules that you would like to use.
* [react/jsx-newline](docs/rules/jsx-newline.md): Enforce a new line after jsx elements and expressions (fixable)
* [react/jsx-no-bind](docs/rules/jsx-no-bind.md): Prevents usage of Function.prototype.bind and arrow functions in React component props
* [react/jsx-no-comment-textnodes](docs/rules/jsx-no-comment-textnodes.md): Comments inside children section of tag should be placed inside braces
* [react/jsx-no-constructed-context-values](docs/rules/jsx-no-constructed-context-values.md): Prevents JSX context provider values from taking values that will cause needless rerenders.
* [react/jsx-no-duplicate-props](docs/rules/jsx-no-duplicate-props.md): Enforce no duplicate props
* [react/jsx-no-literals](docs/rules/jsx-no-literals.md): Prevent using string literals in React component definition
* [react/jsx-no-script-url](docs/rules/jsx-no-script-url.md): Forbid `javascript:` URLs
Expand Down
37 changes: 37 additions & 0 deletions docs/rules/jsx-no-constructed-context-values.md
@@ -0,0 +1,37 @@
# Prevent react contexts from taking non-stable values (react/jsx-no-constructed-context-values)

This rule prevents non-stable values (i.e. object identities) from being used as a value for `Context.Provider`.

## Rule Details

One way to resolve this issue may be to wrap the value in a `useMemo()`. If it's a function then `useCallback()` can be used as well.

If you _expect_ the context to be rerun on each render, then consider adding a comment/lint supression explaining why.

## Examples

Examples of **incorrect** code for this rule:

```
return (
<SomeContext.Provider value={{foo: 'bar'}}>
...
</SomeContext.Provider>
)
```

Examples of **correct** code for this rule:

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

## Legitimate Uses
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.

This can be a pretty large performance hit because not only will it cause the context providers and consumers to rerender with all the elements in its subtree, the processing for the tree scan react does to render the provider and find consumers is also wasted.
1 change: 1 addition & 0 deletions index.js
Expand Up @@ -34,6 +34,7 @@ const allRules = {
'jsx-newline': require('./lib/rules/jsx-newline'),
'jsx-no-bind': require('./lib/rules/jsx-no-bind'),
'jsx-no-comment-textnodes': require('./lib/rules/jsx-no-comment-textnodes'),
'jsx-no-constructed-context-values': require('./lib/rules/jsx-no-constructed-context-values'),
'jsx-no-duplicate-props': require('./lib/rules/jsx-no-duplicate-props'),
'jsx-no-literals': require('./lib/rules/jsx-no-literals'),
'jsx-no-script-url': require('./lib/rules/jsx-no-script-url'),
Expand Down
215 changes: 215 additions & 0 deletions lib/rules/jsx-no-constructed-context-values.js
@@ -0,0 +1,215 @@
/**
* @fileoverview Prevents jsx context provider values from taking values that
* will cause needless rerenders.
* @author Dylan Oshima
*/

'use strict';

const docsUrl = require('../util/docsUrl');

// ------------------------------------------------------------------------------
// Helpers
// ------------------------------------------------------------------------------

// Recursively checks if an element is a construction.
// A construction is a variable that changes identity every render.
function isConstruction(node, callScope) {
switch (node.type) {
case 'Literal':
if (node.regex != null) {
return {type: 'regular expression', node};
}
return null;
case 'Identifier': {
const variableScoping = callScope.set.get(node.name);

if (variableScoping == null || variableScoping.defs == null) {
// If it's not in scope, we don't care.
return null; // Handled
}

// Gets the last variable identity
const variableDefs = variableScoping.defs;
const def = variableDefs[variableDefs.length - 1];
if (def != null
&& def.type !== 'Variable'
&& def.type !== 'FunctionName'
) {
// Parameter or an unusual pattern. Bail out.
return null; // Unhandled
}

if (def.node.type === 'FunctionDeclaration') {
return {type: 'function declaration', node: def.node, usage: node};
}

const init = def.node.init;
if (init == null) {
return null;
}

const initConstruction = isConstruction(init, callScope);
if (initConstruction == null) {
return null;
}

return {
type: initConstruction.type,
node: initConstruction.node,
usage: node
};
}
case 'ObjectExpression':
// Any object initialized inline will create a new identity
return {type: 'object', node};
case 'ArrayExpression':
return {type: 'array', node};
case 'ArrowFunctionExpression':
case 'FunctionExpression':
// Functions that are initialized inline will have a new identity
return {type: 'function expression', node};
case 'ClassExpression':
return {type: 'class expression', node};
case 'NewExpression':
// `const a = new SomeClass();` is a construction
return {type: 'new expression', node};
case 'ConditionalExpression':
return (isConstruction(node.consequent, callScope)
|| isConstruction(node.alternate, callScope)
);
case 'LogicalExpression':
return (isConstruction(node.left, callScope)
|| isConstruction(node.right, callScope)
);
case 'MemberExpression': {
const objConstruction = isConstruction(node.object, callScope);
if (objConstruction == null) {
return null;
}
return {
type: objConstruction.type,
node: objConstruction.node,
usage: node.object
};
}
case 'JSXFragment':
return {type: 'JSX fragment', node};
case 'JSXElement':
return {type: 'JSX element', node};
case 'AssignmentExpression': {
const construct = isConstruction(node.right);
if (construct != null) {
return {
type: 'assignment expression',
node: construct.node,
usage: node
};
}
return null;
}
case 'TypeCastExpression':
case 'TSAsExpression':
return isConstruction(node.expression);
default:
return null;
}
}

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: 'Prevents JSX context provider values from taking values that will cause needless rerenders.',
category: 'Best Practices',
recommended: false,
url: docsUrl('jsx-no-constructed-context-values')
},
messages: {
withIdentifierMsg:
"The '{{variableName}}' {{type}} (at line {{nodeLine}}) passed as the value prop to the Context provider (at line {{usageLine}}) changes every render. To fix this consider wrapping it in a useMemo hook.",
withIdentifierMsgFunc:
"The '{{variableName}}' {{type}} (at line {{nodeLine}}) passed as the value prop to the Context provider (at line {{usageLine}}) changes every render. To fix this consider wrapping it in a useCallback hook.",
defaultMsg:
'The {{type}} passed as the value prop to the Context provider (at line {{nodeLine}}) changes every render. To fix this consider wrapping it in a useMemo hook.',
defaultMsgFunc:
'The {{type}} passed as the value prop to the Context provider (at line {{nodeLine}}) changes every render. To fix this consider wrapping it in a useCallback hook.'
}
},

create(context) {
return {
JSXOpeningElement(node) {
const openingElementName = node.name;
if (openingElementName.type !== 'JSXMemberExpression') {
// Has no member
return;
}

const isJsxContext = openingElementName.property.name === 'Provider';
if (!isJsxContext) {
// Member is not Provider
return;
}

// Contexts can take in more than just a value prop
// so we need to iterate through all of them
const jsxValueAttribute = node.attributes.find(
(attribute) => attribute.type === 'JSXAttribute' && attribute.name.name === 'value'
);

if (jsxValueAttribute == null) {
// No value prop was passed
return;
}

const valueNode = jsxValueAttribute.value;
if (valueNode.type !== 'JSXExpressionContainer') {
// value could be a literal
return;
}

const valueExpression = valueNode.expression;
const invocationScope = context.getScope();

// Check if the value prop is a construction
const constructInfo = isConstruction(valueExpression, invocationScope);
if (constructInfo == null) {
return;
}

// Report found error
const constructType = constructInfo.type;
const constructNode = constructInfo.node;
const constructUsage = constructInfo.usage;
const data = {
type: constructType, nodeLine: constructNode.loc.start.line
};
let messageId = 'defaultMsg';

// Variable passed to value prop
if (constructUsage != null) {
messageId = 'withIdentifierMsg';
data.usageLine = constructUsage.loc.start.line;
data.variableName = constructUsage.name;
}

// Type of expression
if (constructType === 'function expression'
|| constructType === 'function declaration'
) {
messageId += 'Func';
}

context.report({
node: constructNode,
messageId,
data
});
}
};
}
};

0 comments on commit ac98c0f

Please sign in to comment.