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

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
});
}
};
}
};