diff --git a/README.md b/README.md index 53999b7..045d0de 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,7 @@ ESLint plugin for [react-with-styles][react-with-styles]. ## Rules +- [react-with-styles/no-unused-styles](docs/rules/no-unused-styles.md): Require all styles that are defined to be referenced - [react-with-styles/only-spread-css](docs/rules/only-spread-css.md): Require that `css()` is only spread into a JSX element without a `className` or `style` prop [package-url]: https://npmjs.org/package/eslint-plugin-react-with-styles diff --git a/docs/rules/no-unused-styles.md b/docs/rules/no-unused-styles.md new file mode 100644 index 0000000..f6ce4a2 --- /dev/null +++ b/docs/rules/no-unused-styles.md @@ -0,0 +1,52 @@ +# Disallow unused styles + +## Rule Details + +The following patterns are considered warnings: + +``` jsx +function MyComponent({ styles }) { + return ( +
+ Foo +
+ ); +} + +export default withStyles(() => ({ + foo: { + backgroundColor: 'red', + }, + + bar: { // <--- this style is not used + backgroundColor: 'green', + }, +}))(MyComponent); +``` + +The following patterns are not warnings: + +``` jsx +function MyComponent({ styles }) { + return ( +
+ Foo +
+ ); +} + +export default withStyles(() => ({ + foo: { + backgroundColor: 'red', + }, +}))(MyComponent); +``` + +## Known limitations + +- Will not detect styles defined by computed properties. +- Will not detect styles defined by object spread. +- Will not detect styles used by Literal reference (e.g. `styles['foo']` or + `props['styles'].foo`). +- Will not handle files that contain multiple styled components very well. +- Will not handle `styles` prop that has been renamed to something else. diff --git a/docs/rules/only-spread-css.md b/docs/rules/only-spread-css.md index 21220dd..20f54ba 100644 --- a/docs/rules/only-spread-css.md +++ b/docs/rules/only-spread-css.md @@ -4,7 +4,7 @@ The shape of the object returned by the `css()` function from withStyles needs t If you need to add some inline styles (e.g. a style that depends on a non-enumerable value of a prop), `css()` can accept plain objects (example below). -## Rule Details +## Rule details The following patterns are considered warnings: diff --git a/lib/index.js b/lib/index.js index f3e6fee..553d6a0 100644 --- a/lib/index.js +++ b/lib/index.js @@ -1,10 +1,12 @@ 'use strict'; const onlySpreadCSS = require('./rules/only-spread-css'); +const noUnusedStyles = require('./rules/no-unused-styles'); module.exports = { rules: { 'only-spread-css': onlySpreadCSS, + 'no-unused-styles': noUnusedStyles, }, configs: { diff --git a/lib/rules/no-unused-styles.js b/lib/rules/no-unused-styles.js new file mode 100644 index 0000000..bc03c28 --- /dev/null +++ b/lib/rules/no-unused-styles.js @@ -0,0 +1,143 @@ +'use strict'; + +const has = require('has'); + +const findImportCSSFromWithStylesImportDeclaration = require('../util/findImportCSSFromWithStylesImportDeclaration'); +const findRequireCSSFromWithStylesCallExpression = require('../util/findRequireCSSFromWithStylesCallExpression'); + + +module.exports = { + meta: { + docs: { + description: 'Require that all styles that are defined are also referenced in the same file', + recommended: false, // TODO add to recommended for next major release + }, + + schema: [], + }, + + create: function rule(context) { + // If `css()` is imported by this file, we want to store what it is imported + // as in this variable so we can verify where it is used. + let cssFromWithStylesName; + + const usedStyles = {}; + const definedStyles = {}; + + function isCSSFound() { + return !!cssFromWithStylesName; + } + + return { + CallExpression(node) { + const found = findRequireCSSFromWithStylesCallExpression(node); + if (found !== null) { + cssFromWithStylesName = found; + } + + // foo() + if (!isCSSFound()) { + // We are not importing `css` from withStyles, so we have no work to + // do here. + return; + } + + if (node.callee.name === 'withStyles') { + const styles = node.arguments[0]; + + if (styles && styles.type === 'ArrowFunctionExpression') { + const body = styles.body; + + let stylesObj; + if (body.type === 'ObjectExpression') { + stylesObj = body; + } else if (body.type === 'BlockStatement') { + body.body.forEach((bodyNode) => { + if ( + bodyNode.type === 'ReturnStatement' && + bodyNode.argument.type === 'ObjectExpression' + ) { + stylesObj = bodyNode.argument; + } + }); + } + + if (stylesObj) { + stylesObj.properties.forEach((property) => { + if (property.computed) { + // Skip over computed properties for now. + // e.g. `{ [foo]: { ... } }` + // TODO handle this better? + return; + } + + if (property.type === 'ExperimentalSpreadProperty') { + // Skip over object spread for now. + // e.g. `{ ...foo }` + // TODO handle this better? + return; + } + + definedStyles[property.key.name] = property; + }); + } + } + + // Now we know all of the defined styles and used styles, so we can + // see if there are any defined styles that are not used. + Object.keys(definedStyles).forEach((definedStyleKey) => { + if (!has(usedStyles, definedStyleKey)) { + context.report( + definedStyles[definedStyleKey], + `Style \`${definedStyleKey}\` is unused` + ); + } + }); + } + }, + + MemberExpression(node) { + if (!isCSSFound()) { + // We are not importing `css` from withStyles, so we have no work to + // do here. + return; + } + + if (node.object.type === 'Identifier' && node.object.name === 'styles') { + usedStyles[node.property.name] = true; + return; + } + + if (node.property.type === 'Identifier' && node.property.name === 'styles') { + const parent = node.parent; + + if (node.object.object && node.object.object.type !== 'ThisExpression') { + return; + } + + if (parent.object.type === 'Identifier' && parent.object.name !== 'props') { + return; + } + + if (parent.parent.type === 'MemberExpression') { + return; + } + + usedStyles[parent.property.name] = true; + } + }, + + ImportDeclaration(node) { + if (isCSSFound()) { + // We've already found it, so there is no more work to do. + return; + } + + const found = findImportCSSFromWithStylesImportDeclaration(node); + if (found !== null) { + cssFromWithStylesName = found; + } + }, + }; + }, +}; diff --git a/lib/rules/only-spread-css.js b/lib/rules/only-spread-css.js index 0a23534..75731cd 100644 --- a/lib/rules/only-spread-css.js +++ b/lib/rules/only-spread-css.js @@ -48,7 +48,6 @@ module.exports = { context.report( node, - // eslint-disable-next-line max-len `Only spread \`${cssFromWithStylesName}()\` directly into an element, e.g. \`
\`.` ); }, diff --git a/package.json b/package.json index c91a968..82a8de8 100644 --- a/package.json +++ b/package.json @@ -51,5 +51,8 @@ "in-publish": "^2.0.0", "mocha": "^3.2.0", "safe-publish-latest": "^1.1.1" + }, + "dependencies": { + "has": "^1.0.1" } } diff --git a/tests/lib/rules/no-unused-styles.js b/tests/lib/rules/no-unused-styles.js new file mode 100644 index 0000000..f2df639 --- /dev/null +++ b/tests/lib/rules/no-unused-styles.js @@ -0,0 +1,342 @@ +/** +* @fileoverview Require that all styles that are defined are also referenced in the same file +* @author Joe Lencioni +*/ + +'use strict'; + +const rule = require('../../../lib/rules/no-unused-styles'); + +const RuleTester = require('eslint').RuleTester; + +const parserOptions = { + ecmaVersion: 6, + ecmaFeatures: { + jsx: true, + experimentalObjectRestSpread: true, + }, + sourceType: 'module', +}; + +const ruleTester = new RuleTester(); +ruleTester.run('no-unused-styles', rule, { + + valid: [ + { + parserOptions, + code: ` +
+ `.trim(), + }, + + { + parserOptions, + code: ` + import { css } from 'withStyles'; + + function Foo({ styles }) { + return ( +
+ ); + } + + export default withStyles(() => ({ + foo: {}, + }))(Foo); + `.trim(), + }, + + { + parserOptions, + code: ` + import { css } from 'withStyles'; + + function Foo(props) { + return ( +
+ ); + } + + export default withStyles(() => ({ + foo: {}, + }))(Foo); + `.trim(), + }, + + { + parserOptions, + code: ` + import { css } from 'withStyles'; + + class Foo extends React.Component { + render() { + return
; + } + } + + export default withStyles(() => ({ + foo: {}, + }))(Foo); + `.trim(), + }, + + { + parserOptions, + code: ` + import { css } from 'withStyles'; + + class Foo extends React.Component { + render() { + const { styles } = this.props; + return
; + } + } + + export default withStyles(() => ({ + foo: {}, + }))(Foo); + `.trim(), + }, + + { + parserOptions, + code: ` + import { css } from 'withStyles'; + + function Foo({ styles }) { + const something = isActive ? styles.foo : null; + return
; + } + + export default withStyles(() => ({ + foo: {}, + }))(Foo); + `.trim(), + }, + + { // TODO handle computed properties better? + parserOptions, + code: ` + import { css } from 'withStyles'; + + function Foo({ styles }) { + return ( +
+ ); + } + + export default withStyles(() => ({ + [foo]: {}, + }))(Foo); + `.trim(), + }, + + { // TODO handle object spread better? + parserOptions, + code: ` + import { css } from 'withStyles'; + + function Foo({ styles }) { + return ( +
+ ); + } + + export default withStyles(() => ({ + ...foo, + }))(Foo); + `.trim(), + }, + + { + parserOptions, + code: ` + import { css } from 'withStyles'; + + function Foo({ styles }) { + return ( +
+ ); + } + + export default withStyles(() => { + return { + foo: {}, + } + })(Foo); + `.trim(), + }, + + { + parserOptions, + code: ` + import { css } from 'withStyles'; + + function Foo({ styles }) { + return ( +
+ ); + } + + export default withStyles(() => ({ + foo: {}, + bar: {}, + }))(Foo); + `.trim(), + }, + + { + parserOptions, + code: ` + import { css } from 'withStyles'; + + function Foo({ styles }) { + return ( +
+
+
+ ); + } + + export default withStyles(() => ({ + foo: {}, + bar: {}, + }))(Foo); + `.trim(), + }, + ], + + invalid: [ + { + parserOptions, + code: ` + import { css } from 'withStyles'; + + function Foo({ styles }) { + return ( +
+ ); + } + + export default withStyles(() => ({ + foo: {}, + bar: {}, + }))(Foo); + `.trim(), + errors: [{ + message: 'Style `bar` is unused', + type: 'Property', + }], + }, + + { + parserOptions, + code: ` + import { css } from 'withStyles'; + + function Foo({ styles }) { + return ( +
+ ); + } + + export default withStyles(() => { + return { + foo: {}, + bar: {}, + } + })(Foo); + `.trim(), + errors: [{ + message: 'Style `bar` is unused', + type: 'Property', + }], + }, + + { + parserOptions, + code: ` + import { css } from 'withStyles'; + + class Foo extends React.Component { + render() { + return ( +
+ ); + } + } + + export default withStyles(() => ({ + foo: {}, + }))(Foo); + `.trim(), + errors: [{ + message: 'Style `foo` is unused', + type: 'Property', + }], + }, + + { + parserOptions, + code: ` + import { css } from 'withStyles'; + + function Foo(props) { + return ( +
+ ); + } + + export default withStyles(() => ({ + foo: {}, + }))(Foo); + `.trim(), + errors: [{ + message: 'Style `foo` is unused', + type: 'Property', + }], + }, + + { + parserOptions, + code: ` + import { css } from 'withStyles'; + + function Foo(props) { + return ( +
+ ); + } + + export default withStyles(() => ({ + bar: {}, + }))(Foo); + `.trim(), + errors: [{ + message: 'Style `bar` is unused', + type: 'Property', + }], + }, + + { + parserOptions, + code: ` + import { css } from 'withStyles'; + + function Foo(props) { + return ( +
+ ); + } + + export default withStyles(() => ({ + foo: {}, + }))(Foo); + `.trim(), + errors: [{ + message: 'Style `foo` is unused', + type: 'Property', + }], + }, + ], +});