Skip to content

Commit

Permalink
Add no-unused-styles rule
Browse files Browse the repository at this point in the history
There are a few edge cases that I decided to punt on for now. I tried to
note as many as I could think of in the documentation and in code
comments.

Fixes #2.
  • Loading branch information
lencioni committed Feb 17, 2017
1 parent 14427f4 commit 19d8530
Show file tree
Hide file tree
Showing 8 changed files with 544 additions and 2 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
52 changes: 52 additions & 0 deletions docs/rules/no-unused-styles.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Disallow unused styles

## Rule Details

The following patterns are considered warnings:

``` jsx
function MyComponent({ styles }) {
return (
<div {...css(styles.foo)}>
Foo
</div>
);
}

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 (
<div {...css(styles.foo)}>
Foo
</div>
);
}

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.
2 changes: 1 addition & 1 deletion docs/rules/only-spread-css.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
2 changes: 2 additions & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
@@ -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: {
Expand Down
143 changes: 143 additions & 0 deletions lib/rules/no-unused-styles.js
Original file line number Diff line number Diff line change
@@ -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;
}
},
};
},
};
1 change: 0 additions & 1 deletion lib/rules/only-spread-css.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ module.exports = {

context.report(
node,
// eslint-disable-next-line max-len
`Only spread \`${cssFromWithStylesName}()\` directly into an element, e.g. \`<div {...${cssFromWithStylesName}(foo)} />\`.`
);
},
Expand Down
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,8 @@
"in-publish": "^2.0.0",
"mocha": "^3.2.0",
"safe-publish-latest": "^1.1.1"
},
"dependencies": {
"has": "^1.0.1"
}
}
Loading

0 comments on commit 19d8530

Please sign in to comment.