Skip to content

Commit f141983

Browse files
majapwmajapw
authored
Merge pull request #27 from airbnb/maja-fix-css-prop-reliance
Remove requirement that css be imported from the global in no-unused-styles
2 parents 11b7312 + 98b4ffd commit f141983

File tree

4 files changed

+41
-160
lines changed

4 files changed

+41
-160
lines changed

‎lib/rules/no-unused-styles.js

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,6 @@
22

33
const has = require('has');
44

5-
const findImportCSSFromWithStylesImportDeclaration = require('../util/findImportCSSFromWithStylesImportDeclaration');
6-
const findRequireCSSFromWithStylesCallExpression = require('../util/findRequireCSSFromWithStylesCallExpression');
7-
85
function getBasicIdentifier(node) {
96
if (node.type === 'Identifier') {
107
// styles.foo
@@ -37,31 +34,11 @@ module.exports = {
3734
},
3835

3936
create: function rule(context) {
40-
// If `css()` is imported by this file, we want to store what it is imported
41-
// as in this variable so we can verify where it is used.
42-
let cssFromWithStylesName;
43-
4437
const usedStyles = {};
4538
const definedStyles = {};
4639

47-
function isCSSFound() {
48-
return !!cssFromWithStylesName;
49-
}
50-
5140
return {
5241
CallExpression(node) {
53-
const found = findRequireCSSFromWithStylesCallExpression(node);
54-
if (found !== null) {
55-
cssFromWithStylesName = found;
56-
}
57-
58-
// foo()
59-
if (!isCSSFound()) {
60-
// We are not importing `css` from withStyles, so we have no work to
61-
// do here.
62-
return;
63-
}
64-
6542
if (node.callee.name === 'withStyles') {
6643
const styles = node.arguments[0];
6744

@@ -117,12 +94,6 @@ module.exports = {
11794
},
11895

11996
MemberExpression(node) {
120-
if (!isCSSFound()) {
121-
// We are not importing `css` from withStyles, so we have no work to
122-
// do here.
123-
return;
124-
}
125-
12697
if (node.object.type === 'Identifier' && node.object.name === 'styles') {
12798
const style = getBasicIdentifier(node.property);
12899
if (style) {
@@ -172,18 +143,6 @@ module.exports = {
172143
usedStyles[style] = true;
173144
}
174145
},
175-
176-
ImportDeclaration(node) {
177-
if (isCSSFound()) {
178-
// We've already found it, so there is no more work to do.
179-
return;
180-
}
181-
182-
const found = findImportCSSFromWithStylesImportDeclaration(node);
183-
if (found !== null) {
184-
cssFromWithStylesName = found;
185-
}
186-
},
187146
};
188147
},
189148
};

‎lib/rules/only-spread-css.js

Lines changed: 6 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
'use strict';
22

3-
const findImportCSSFromWithStylesImportDeclaration = require('../util/findImportCSSFromWithStylesImportDeclaration');
4-
const findRequireCSSFromWithStylesCallExpression = require('../util/findRequireCSSFromWithStylesCallExpression');
5-
63
module.exports = {
74
meta: {
85
docs: {
@@ -14,29 +11,10 @@ module.exports = {
1411
},
1512

1613
create: function rule(context) {
17-
// If `css()` is imported by this file, we want to store what it is imported
18-
// as in this variable so we can verify where it is used.
19-
let cssFromWithStylesName;
20-
21-
function isCSSFound() {
22-
return !!cssFromWithStylesName;
23-
}
24-
14+
const CSS_METHOD_NAME = 'css';
2515
return {
2616
CallExpression(node) {
27-
const found = findRequireCSSFromWithStylesCallExpression(node);
28-
if (found !== null) {
29-
cssFromWithStylesName = found;
30-
}
31-
32-
// foo()
33-
if (!isCSSFound()) {
34-
// We are not importing `css` from withStyles, so we have no work to
35-
// do here.
36-
return;
37-
}
38-
39-
if (node.callee.name !== cssFromWithStylesName) {
17+
if (node.callee.name !== CSS_METHOD_NAME) {
4018
// foo()
4119
return;
4220
}
@@ -48,30 +26,11 @@ module.exports = {
4826

4927
context.report(
5028
node,
51-
`Only spread \`${cssFromWithStylesName}()\` directly into an element, e.g. \`<div {...${cssFromWithStylesName}(foo)} />\`.`
29+
`Only spread \`${CSS_METHOD_NAME}()\` directly into an element, e.g. \`<div {...${CSS_METHOD_NAME}(foo)} />\`.`
5230
);
5331
},
5432

55-
ImportDeclaration(node) {
56-
if (isCSSFound()) {
57-
// We've already found it, so there is no more work to do.
58-
return;
59-
}
60-
61-
const found = findImportCSSFromWithStylesImportDeclaration(node);
62-
if (found !== null) {
63-
cssFromWithStylesName = found;
64-
}
65-
},
66-
6733
JSXSpreadAttribute(node) {
68-
// <div {...foo()} />
69-
if (!isCSSFound()) {
70-
// We are not importing `css` from withStyles, so we have no work to
71-
// do here.
72-
return;
73-
}
74-
7534
if (node.argument.type !== 'CallExpression') {
7635
// <div {...foo} />
7736
//
@@ -81,7 +40,7 @@ module.exports = {
8140
return;
8241
}
8342

84-
if (node.argument.callee.name !== cssFromWithStylesName) {
43+
if (node.argument.callee.name !== CSS_METHOD_NAME) {
8544
// <div {...foo()} />
8645
return;
8746
}
@@ -109,15 +68,15 @@ module.exports = {
10968
if (attribute.name.name === 'className') {
11069
context.report(
11170
attribute,
112-
`Do not use \`className\` with \`{...${cssFromWithStylesName}()}\`.`
71+
`Do not use \`className\` with \`{...${CSS_METHOD_NAME}()}\`.`
11372
);
11473
return;
11574
}
11675

11776
if (attribute.name.name === 'style') {
11877
context.report(
11978
attribute,
120-
`Do not use \`style\` with \`{...${cssFromWithStylesName}()}\`.`
79+
`Do not use \`style\` with \`{...${CSS_METHOD_NAME}()}\`.`
12180
);
12281
}
12382
});

‎tests/lib/rules/no-unused-styles.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,21 @@ ruleTester.run('no-unused-styles', rule, {
486486
}))(Foo);
487487
`.trim(),
488488
},
489+
490+
{
491+
parserOptions,
492+
code: `
493+
function Foo({ css, styles }) {
494+
return (
495+
<div {...css(styles.foo)} />
496+
);
497+
}
498+
499+
export default withStyles(() => ({
500+
foo: {},
501+
}))(Foo);
502+
`.trim(),
503+
},
489504
],
490505

491506
invalid: [
@@ -620,5 +635,25 @@ ruleTester.run('no-unused-styles', rule, {
620635
type: 'Property',
621636
}],
622637
},
638+
639+
{
640+
parserOptions,
641+
code: `
642+
function Foo({ css, styles }) {
643+
return (
644+
<div {...css(styles.foo)} />
645+
);
646+
}
647+
648+
export default withStyles(() => ({
649+
foo: {},
650+
bar: {},
651+
}))(Foo);
652+
`.trim(),
653+
errors: [{
654+
message: 'Style `bar` is unused',
655+
type: 'Property',
656+
}],
657+
},
623658
],
624659
});

‎tests/lib/rules/only-spread-css.js

Lines changed: 0 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -54,54 +54,6 @@ ruleTester.run('only-spread-css', rule, {
5454
<div {...css(foo)} {...bar()} />
5555
`.trim(),
5656
},
57-
58-
{
59-
parserOptions,
60-
code: `
61-
import { css } from 'somethingElse';
62-
<div {...css(foo)} className="foo" />
63-
`.trim(),
64-
},
65-
66-
{
67-
parserOptions,
68-
code: `
69-
import { css } from 'somethingElse';
70-
<div {...css(foo)} style={{ color: 'red' }} />
71-
`.trim(),
72-
},
73-
74-
{
75-
parserOptions,
76-
code: `
77-
const { css } = require('somethingElse');
78-
<div {...css(foo)} className="foo" />
79-
`.trim(),
80-
},
81-
82-
{
83-
parserOptions,
84-
code: `
85-
const { css } = require('somethingElse');
86-
<div {...css(foo)} style={{ color: 'red' }} />
87-
`.trim(),
88-
},
89-
90-
{
91-
parserOptions,
92-
code: `
93-
require(foo);
94-
<div {...css(foo)} style={{ color: 'red' }} />
95-
`.trim(),
96-
},
97-
98-
{
99-
parserOptions,
100-
code: `
101-
require();
102-
<div {...css(foo)} style={{ color: 'red' }} />
103-
`.trim(),
104-
},
10557
],
10658

10759
invalid: [
@@ -219,18 +171,6 @@ ruleTester.run('only-spread-css', rule, {
219171
],
220172
},
221173

222-
{
223-
parserOptions,
224-
code: `
225-
import { css as bar } from 'withStyles';
226-
<div {...bar(foo)} className="foo" />
227-
`.trim(),
228-
errors: [{
229-
message: 'Do not use `className` with `{...bar()}`.',
230-
type: 'JSXAttribute',
231-
}],
232-
},
233-
234174
{
235175
parserOptions,
236176
code: `
@@ -254,17 +194,5 @@ ruleTester.run('only-spread-css', rule, {
254194
type: 'JSXAttribute',
255195
}],
256196
},
257-
258-
{
259-
parserOptions,
260-
code: `
261-
const { css: bar } = require('withStyles');
262-
<div {...bar(foo)} className="foo" />
263-
`.trim(),
264-
errors: [{
265-
message: 'Do not use `className` with `{...bar()}`.',
266-
type: 'JSXAttribute',
267-
}],
268-
},
269197
],
270198
});

0 commit comments

Comments
 (0)