Skip to content

Commit

Permalink
[test] Lint useThemeVariants with custom rules plugin (#21963)
Browse files Browse the repository at this point in the history
  • Loading branch information
eps1lon authored Jul 28, 2020
1 parent 3dfb429 commit 56a1141
Show file tree
Hide file tree
Showing 5 changed files with 238 additions and 0 deletions.
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions packages/eslint-plugin-material-ui/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
1 change: 1 addition & 0 deletions packages/eslint-plugin-material-ui/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
};
Original file line number Diff line number Diff line change
@@ -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.`,
});
}
});
}
},
};
},
};
Original file line number Diff line number Diff line change
@@ -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,
},
],
},
],
});

0 comments on commit 56a1141

Please sign in to comment.