Skip to content

Commit 28c927c

Browse files
dddlrdddlr
authoredOct 19, 2023
Refactor no-css-prop-without-css-function, fix edge case (#1530)
* Refactor no-css-prop-without-css-function, fix edge case * Add changeset
·
@compiled/webpack-loader@0.19.5@compiled/babel-plugin@0.24.1
1 parent 7591a97 commit 28c927c

File tree

3 files changed

+143
-113
lines changed

3 files changed

+143
-113
lines changed
 

‎.changeset/orange-dragons-train.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@compiled/eslint-plugin': patch
3+
---
4+
5+
Fix edge case where no-css-prop-without-css-function crashes

‎packages/eslint-plugin/src/rules/no-css-prop-without-css-function/__tests__/rule.test.ts

+14
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,20 @@ tester.run(
116116
const otherStyles = css({ background: 'blue' });
117117
const Component = (props) => <MyComponent css={[props.myBoolean() && styles, otherStyles]} />;
118118
`,
119+
outdent`
120+
import { css } from '@compiled/react';
121+
122+
const FloatingContainer = (css) => {
123+
const floatingContainerStyles = css({ color: 'pink' });
124+
const cssStyles = [floatingContainerStyles, css({ color: 'blue' })];
125+
126+
return (
127+
<div ref={ref} css={cssStyles} {...rest}>
128+
{children}
129+
</div>
130+
);
131+
}
132+
`,
119133
],
120134

121135
invalid: [

‎packages/eslint-plugin/src/rules/no-css-prop-without-css-function/index.ts

+124-113
Original file line numberDiff line numberDiff line change
@@ -30,146 +30,157 @@ const findNodeReference = (
3030
return references.find((reference) => reference.identifier === node);
3131
};
3232

33-
const handleIdentifier = (node: TSESTree.Identifier, references: Reference[], context: Context) => {
34-
// Resolve the variable for the reference
35-
const reference = findNodeReference(references, node);
36-
const definition = reference?.resolved?.defs.find(
37-
(def): def is VariableDefinition => def.type === 'Variable'
38-
);
39-
40-
// Traverse to the variable value
41-
if (definition && definition.node.init) {
42-
findStyleNodes(definition.node.init, references, context);
43-
} else {
44-
const isImported = reference?.resolved?.defs.find((def) => def.type === 'ImportBinding');
45-
const isFunctionParameter = reference?.resolved?.defs.find((def) => def.type === 'Parameter');
46-
47-
const jsxElement = traverseUpToJSXOpeningElement(node);
48-
49-
// css property on DOM elements are always fine, e.g.
50-
// <div css={...}> instead of <MyComponent css={...}>
51-
if (jsxElement.name.type === 'JSXIdentifier' && isDOMElement(jsxElement.name.name)) {
52-
return;
33+
class NoCssPropWithoutCssFunctionRunner {
34+
private references: Reference[];
35+
36+
constructor(private baseNode: TSESTree.JSXExpressionContainer, private context: Context) {
37+
this.references = context.getScope().references;
38+
}
39+
40+
private handleIdentifier(node: TSESTree.Identifier) {
41+
// Resolve the variable for the reference
42+
const reference = findNodeReference(this.references, node);
43+
const definition = reference?.resolved?.defs.find(
44+
(def): def is VariableDefinition => def.type === 'Variable'
45+
);
46+
47+
// Traverse to the variable value
48+
if (definition && definition.node.init) {
49+
this.findStyleNodes(definition.node.init);
50+
} else {
51+
const isImported = reference?.resolved?.defs.find((def) => def.type === 'ImportBinding');
52+
const isFunctionParameter = reference?.resolved?.defs.find((def) => def.type === 'Parameter');
53+
54+
const jsxElement = traverseUpToJSXOpeningElement(this.baseNode);
55+
56+
// css property on DOM elements are always fine, e.g.
57+
// <div css={...}> instead of <MyComponent css={...}>
58+
if (
59+
jsxElement &&
60+
jsxElement.name.type === 'JSXIdentifier' &&
61+
isDOMElement(jsxElement.name.name)
62+
) {
63+
return;
64+
}
65+
66+
if (isImported) {
67+
this.context.report({
68+
messageId: 'importedInvalidCssUsage',
69+
node,
70+
});
71+
} else if (isFunctionParameter) {
72+
this.context.report({
73+
messageId: 'functionParameterInvalidCssUsage',
74+
node,
75+
});
76+
} else {
77+
this.context.report({
78+
messageId: 'otherInvalidCssUsage',
79+
node,
80+
});
81+
}
5382
}
83+
}
5484

55-
if (isImported) {
56-
context.report({
57-
messageId: 'importedInvalidCssUsage',
58-
node,
59-
});
60-
} else if (isFunctionParameter) {
61-
context.report({
85+
private handleMemberExpression(node: TSESTree.MemberExpression) {
86+
const reference = findNodeReference(this.references, node.object);
87+
const definition = reference?.resolved?.defs.find(
88+
(def): def is ParameterDefinition => def.type === 'Parameter'
89+
);
90+
91+
if (definition) {
92+
this.context.report({
6293
messageId: 'functionParameterInvalidCssUsage',
6394
node,
6495
});
65-
} else {
66-
context.report({
67-
messageId: 'otherInvalidCssUsage',
68-
node,
69-
});
7096
}
7197
}
72-
};
7398

74-
const handleMemberExpression = (
75-
node: TSESTree.MemberExpression,
76-
references: Reference[],
77-
context: Context
78-
) => {
79-
const reference = findNodeReference(references, node.object);
80-
const definition = reference?.resolved?.defs.find(
81-
(def): def is ParameterDefinition => def.type === 'Parameter'
82-
);
83-
84-
if (definition) {
85-
context.report({
86-
messageId: 'functionParameterInvalidCssUsage',
87-
node,
88-
});
89-
}
90-
};
91-
92-
const fixWrapper = (node: CSSValue, context: Context) => {
93-
function* fix(fixer: TSESLint.RuleFixer) {
94-
const compiledImports = findTSCompiledImportDeclarations(context);
95-
const source = context.getSourceCode();
99+
private fixWrapper(node: CSSValue, context: Context) {
100+
function* fix(fixer: TSESLint.RuleFixer) {
101+
const compiledImports = findTSCompiledImportDeclarations(context);
102+
const source = context.getSourceCode();
96103

97-
// The string that `css` from `@compiled/css` is imported as
98-
const cssImportName = getImportedName(compiledImports, 'css');
104+
// The string that `css` from `@compiled/css` is imported as
105+
const cssImportName = getImportedName(compiledImports, 'css');
99106

100-
if (compiledImports.length > 0) {
101-
if (!cssImportName) {
102-
// Import found, add the specifier to it
103-
const [firstCompiledImport] = compiledImports;
104-
const specifiersString = addImportToDeclaration(firstCompiledImport, ['css']);
107+
if (compiledImports.length > 0) {
108+
if (!cssImportName) {
109+
// Import found, add the specifier to it
110+
const [firstCompiledImport] = compiledImports;
111+
const specifiersString = addImportToDeclaration(firstCompiledImport, ['css']);
105112

106-
yield fixer.replaceText(firstCompiledImport, specifiersString);
113+
yield fixer.replaceText(firstCompiledImport, specifiersString);
114+
}
115+
} else {
116+
// Import not found, add a new one
117+
yield fixer.insertTextAfter(
118+
source.ast.body[0],
119+
`\n${buildImportDeclaration('css', '@compiled/react')}`
120+
);
107121
}
108-
} else {
109-
// Import not found, add a new one
110-
yield fixer.insertTextAfter(
111-
source.ast.body[0],
112-
`\n${buildImportDeclaration('css', '@compiled/react')}`
113-
);
114-
}
115122

116-
const cssFunctionName = cssImportName ?? 'css';
123+
const cssFunctionName = cssImportName ?? 'css';
117124

118-
if (node.type === 'ObjectExpression') {
119-
const parent = node.parent;
120-
if (parent && parent.type === 'TSAsExpression') {
121-
yield fixer.replaceText(parent, `${cssFunctionName}(${source.getText(node)})`);
125+
if (node.type === 'ObjectExpression') {
126+
const parent = node.parent;
127+
if (parent && parent.type === 'TSAsExpression') {
128+
yield fixer.replaceText(parent, `${cssFunctionName}(${source.getText(node)})`);
129+
} else {
130+
yield fixer.insertTextBefore(node, `${cssFunctionName}(`);
131+
yield fixer.insertTextAfter(node, ')');
132+
}
122133
} else {
123-
yield fixer.insertTextBefore(node, `${cssFunctionName}(`);
124-
yield fixer.insertTextAfter(node, ')');
134+
yield fixer.insertTextBefore(node, cssFunctionName);
125135
}
126-
} else {
127-
yield fixer.insertTextBefore(node, cssFunctionName);
128136
}
137+
138+
return fix;
129139
}
130140

131-
return fix;
132-
};
141+
private findStyleNodes(node: CSSValue): void {
142+
if (node.type === 'ArrayExpression') {
143+
node.elements.forEach((arrayElement) => {
144+
if (arrayElement && arrayElement.type !== 'SpreadElement') {
145+
this.findStyleNodes(arrayElement);
146+
}
147+
});
148+
} else if (node.type === 'LogicalExpression') {
149+
this.findStyleNodes(node.right);
150+
} else if (node.type === 'ConditionalExpression') {
151+
// Traverse both return values in the conditional expression
152+
this.findStyleNodes(node.consequent);
153+
this.findStyleNodes(node.alternate);
154+
} else if (node.type === 'Identifier') {
155+
this.handleIdentifier(node);
156+
} else if (node.type === 'MemberExpression') {
157+
this.handleMemberExpression(node);
158+
} else if (node.type === 'ObjectExpression' || node.type === 'TemplateLiteral') {
159+
// We found an object expression that was not wrapped, report
160+
this.context.report({
161+
messageId: 'noCssFunction',
162+
node,
163+
fix: this.fixWrapper(node, this.context),
164+
});
165+
} else if (node.type === 'TSAsExpression') {
166+
// TSAsExpression is anything in the form "X as Y", e.g.:
167+
// const abc = { ... } as const;
168+
return this.findStyleNodes(node.expression);
169+
}
170+
}
133171

134-
const findStyleNodes = (node: CSSValue, references: Reference[], context: Context): void => {
135-
if (node.type === 'ArrayExpression') {
136-
node.elements.forEach((arrayElement) => {
137-
if (arrayElement && arrayElement.type !== 'SpreadElement') {
138-
findStyleNodes(arrayElement, references, context);
139-
}
140-
});
141-
} else if (node.type === 'LogicalExpression') {
142-
findStyleNodes(node.right, references, context);
143-
} else if (node.type === 'ConditionalExpression') {
144-
// Traverse both return values in the conditional expression
145-
findStyleNodes(node.consequent, references, context);
146-
findStyleNodes(node.alternate, references, context);
147-
} else if (node.type === 'Identifier') {
148-
handleIdentifier(node, references, context);
149-
} else if (node.type === 'MemberExpression') {
150-
handleMemberExpression(node, references, context);
151-
} else if (node.type === 'ObjectExpression' || node.type === 'TemplateLiteral') {
152-
// We found an object expression that was not wrapped, report
153-
context.report({
154-
messageId: 'noCssFunction',
155-
node,
156-
fix: fixWrapper(node, context),
157-
});
158-
} else if (node.type === 'TSAsExpression') {
159-
// TSAsExpression is anything in the form "X as Y", e.g.:
160-
// const abc = { ... } as const;
161-
return findStyleNodes(node.expression, references, context);
172+
run() {
173+
this.findStyleNodes(this.baseNode.expression);
162174
}
163-
};
175+
}
164176

165177
const createNoCssPropWithoutCssFunctionRule =
166178
(): TSESLint.RuleModule<string>['create'] => (context) => ({
167179
'JSXAttribute[name.name="css"] JSXExpressionContainer': (
168180
node: TSESTree.JSXExpressionContainer
169181
): void => {
170-
const { references } = context.getScope();
171-
172-
findStyleNodes(node.expression, references, context);
182+
const runner = new NoCssPropWithoutCssFunctionRunner(node, context);
183+
runner.run();
173184
},
174185
});
175186

0 commit comments

Comments
 (0)
Please sign in to comment.