diff --git a/.eslintrc.js b/.eslintrc.js index 320cbbcd6a492a..47290a422ffbba 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -61,6 +61,7 @@ module.exports = { 'jsx-a11y/label-has-associated-control': 'off', // doesn't work? 'jsx-a11y/no-autofocus': 'off', // We are a library, we need to support it too 'material-ui/docgen-ignore-before-comment': 'error', + 'material-ui/rules-of-use-theme-variants': 'error', 'react-hooks/exhaustive-deps': ['error', { additionalHooks: 'useEnhancedEffect' }], 'react-hooks/rules-of-hooks': 'error', 'react/destructuring-assignment': 'off', // It's fine. diff --git a/packages/eslint-plugin-material-ui/README.md b/packages/eslint-plugin-material-ui/README.md index 2b503947e0710b..a9f2ed702e6460 100644 --- a/packages/eslint-plugin-material-ui/README.md +++ b/packages/eslint-plugin-material-ui/README.md @@ -40,3 +40,8 @@ Removed in favor of [`no-restricted-imports`](https://eslint.org/docs/rules/no-r } } ``` + +### rules-of-use-theme-variants + +Ensures correct usage of `useThemeVariants` so that all props are passed as well +as their resolved default values. diff --git a/packages/eslint-plugin-material-ui/src/index.js b/packages/eslint-plugin-material-ui/src/index.js index 8bfc6c79735012..f8073ee3d55d2c 100644 --- a/packages/eslint-plugin-material-ui/src/index.js +++ b/packages/eslint-plugin-material-ui/src/index.js @@ -3,4 +3,5 @@ module.exports.rules = { 'disallow-active-element-as-key-event-target': require('./rules/disallow-active-element-as-key-event-target'), 'docgen-ignore-before-comment': require('./rules/docgen-ignore-before-comment'), 'no-hardcoded-labels': require('./rules/no-hardcoded-labels'), + 'rules-of-use-theme-variants': require('./rules/rules-of-use-theme-variants'), }; diff --git a/packages/eslint-plugin-material-ui/src/rules/rules-of-use-theme-variants.js b/packages/eslint-plugin-material-ui/src/rules/rules-of-use-theme-variants.js new file mode 100644 index 00000000000000..8a8f92337478c3 --- /dev/null +++ b/packages/eslint-plugin-material-ui/src/rules/rules-of-use-theme-variants.js @@ -0,0 +1,108 @@ +module.exports = { + meta: { + type: 'problem', + }, + create(context) { + function getComponentProps(componentBlockNode) { + // finds the declarator in `const {...} = props;` + let componentPropsDeclarator = null; + componentBlockNode.body.forEach((node) => { + if (node.type === 'VariableDeclaration') { + const propsDeclarator = node.declarations.find((declarator) => { + return declarator.init.name === 'props'; + }); + if (propsDeclarator !== undefined) { + componentPropsDeclarator = propsDeclarator; + } + } + }); + + return componentPropsDeclarator !== null ? componentPropsDeclarator.id : undefined; + } + + function getComponentBlockNode(hookCallNode) { + let node = hookCallNode.parent; + while (node !== undefined) { + if (node.type === 'BlockStatement') { + return node; + } + node = node.parent; + } + return null; + } + + return { + CallExpression(node) { + if (node.callee.name === 'useThemeVariants') { + const componentBlockNode = getComponentBlockNode(node); + + const componentProps = getComponentProps(componentBlockNode); + const defaultProps = + componentProps === undefined + ? [] + : componentProps.properties.filter((objectProperty) => { + return ( + objectProperty.type === 'Property' && + objectProperty.value.type === 'AssignmentPattern' + ); + }); + + const [variantProps] = node.arguments; + + const unsupportedComponentPropsNode = + componentProps !== undefined && componentProps.type !== 'ObjectPattern'; + + if (unsupportedComponentPropsNode) { + context.report({ + node: componentProps, + message: `Can only analyze object patterns but found '${componentProps.type}'. Prefer \`const {...} = props;\``, + }); + } + + if (defaultProps.length === 0) { + return; + } + + if (variantProps.type !== 'ObjectExpression') { + context.report({ + node: variantProps, + message: `Can only analyze object patterns but found '${variantProps.type}'. Prefer \`{...props}\`.`, + }); + return; + } + + const variantPropsRestNode = variantProps.properties.find((objectProperty) => { + return objectProperty.type === 'SpreadElement'; + }); + + if ( + variantPropsRestNode !== undefined && + variantProps.properties.indexOf(variantPropsRestNode) !== 0 && + defaultProps.length > 0 + ) { + context.report({ + node: variantPropsRestNode, + message: + 'The props spread must come first in the `useThemeVariants` props. Otherwise destructured props with default values could be overridden.', + }); + } + + defaultProps.forEach((componentProp) => { + const isPassedToVariantProps = + variantProps.properties.find((variantProp) => { + return ( + variantProp.type === 'Property' && componentProp.key.name === variantProp.key.name + ); + }) !== undefined; + if (!isPassedToVariantProps) { + context.report({ + node: variantProps, + message: `Prop \`${componentProp.key.name}\` is not passed to \`useThemeVariants\` props.`, + }); + } + }); + } + }, + }; + }, +}; diff --git a/packages/eslint-plugin-material-ui/src/rules/rules-of-use-theme-variants.test.js b/packages/eslint-plugin-material-ui/src/rules/rules-of-use-theme-variants.test.js new file mode 100644 index 00000000000000..243bbdf5b23aa8 --- /dev/null +++ b/packages/eslint-plugin-material-ui/src/rules/rules-of-use-theme-variants.test.js @@ -0,0 +1,123 @@ +const eslint = require('eslint'); +const rule = require('./rules-of-use-theme-variants'); + +const ruleTester = new eslint.RuleTester({ + parser: require.resolve('@typescript-eslint/parser'), + parserOptions: { + ecmaFeatures: { jsx: true }, + }, +}); +ruleTester.run('rules-of-use-theme-variants', rule, { + valid: [ + // allowed but dangerous + ` +{ + const useCustomThemeVariants = props => useThemeVariants(props); +}`, + ` +{ + useThemeVariants(props); +} +`, + ` +{ + const { className, value: valueProp, ...other } = props; + useThemeVariants(props); +} +`, + ` +{ + const { className, disabled = false, value: valueProp, ...other } = props; + useThemeVariants({ ...props, disabled }); +} +`, + ` +{ + const { className, value: valueProp, ...other } = props; + const [stateA, setStateA] = React.useState(0); + const [stateB, setStateB] = React.useState(0); + useThemeVariants({ stateA, ...props, stateB }); +} +`, + // unneccessary spread but it's not the responsibility of this rule to catch "unnecessary" spread + ` +{ + const { className, value: valueProp, ...other } = props; + useThemeVariants({ ...props}); +} + `, + ], + invalid: [ + { + code: ` +{ + const { disabled = false, ...other } = props; + useThemeVariants({ ...props}); +} + `, + errors: [ + { + message: 'Prop `disabled` is not passed to `useThemeVariants` props.', + line: 4, + column: 20, + endLine: 4, + endColumn: 31, + }, + ], + }, + { + code: ` +{ + const { disabled = false, variant = 'text', ...other } = props; + useThemeVariants({ ...props, disabled }); +} + `, + errors: [ + { + message: 'Prop `variant` is not passed to `useThemeVariants` props.', + line: 4, + column: 20, + endLine: 4, + endColumn: 42, + }, + ], + }, + { + code: ` +{ + const { disabled = false, ...other } = props; + useThemeVariants({ disabled, ...props }); +} + `, + errors: [ + { + message: + 'The props spread must come first in the `useThemeVariants` props. Otherwise destructured props with default values could be overridden.', + line: 4, + column: 32, + endLine: 4, + endColumn: 40, + }, + ], + }, + // this is valid code but not analyzeable by this rule + { + code: ` +{ + const { disabled = false, ...other } = props; + const themeVariantProps = { ...props, disabled }; + useThemeVariants(themeVariantProps); +} + `, + errors: [ + { + message: "Can only analyze object patterns but found 'Identifier'. Prefer `{...props}`.", + line: 5, + column: 20, + endLine: 5, + endColumn: 37, + }, + ], + }, + ], +});