Skip to content

Commit fe600b1

Browse files
mjames-cmjames-c
committed
Add sx support to no-conflicting-props
1 parent 20ce0b6 commit fe600b1

3 files changed

Lines changed: 204 additions & 12 deletions

File tree

‎packages/@stylexjs/eslint-plugin/README.md‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,20 +256,29 @@ specifications.
256256
### `@stylexjs/no-conflicting-props`
257257

258258
This rule disallows using `className` or `style` props on elements that spread
259-
`stylex.props()` to avoid conflicts and unexpected behavior.
259+
`stylex.props()` or use StyleX JSX shorthand to avoid conflicts and unexpected
260+
behavior.
260261

261262
#### Invalid examples
262263

263264
```jsx
264265
<div {...stylex.props(styles.foo)} className="extra" />
265266

266267
<div {...stylex.props(styles.foo)} style={{ color: 'red' }} />
268+
269+
<div sx={styles.foo} className="extra" />
270+
271+
<div sx={styles.foo} style={{ color: 'red' }} />
267272
```
268273

269274
#### Config options
270275

271276
```json
272277
{
273-
"validImports": ["stylex", "@stylexjs/stylex"]
278+
"validImports": ["stylex", "@stylexjs/stylex"],
279+
"sxPropName": "sx"
274280
}
275281
```
282+
283+
Set `sxPropName` to a string to check a custom JSX shorthand prop, or `false` to
284+
disable JSX shorthand checks.

‎packages/@stylexjs/eslint-plugin/__tests__/stylex-no-conflicting-props-test.js‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,22 @@ eslintTester.run('stylex-no-conflicting-props', rule.default, {
5252
}
5353
`,
5454
},
55+
{
56+
code: `
57+
import * as stylex from '@stylexjs/stylex';
58+
function Component() {
59+
return <div sx className="foo" />;
60+
}
61+
`,
62+
},
63+
{
64+
code: `
65+
import * as stylex from '@stylexjs/stylex';
66+
function Component() {
67+
return <div sx="foo" className="foo" />;
68+
}
69+
`,
70+
},
5571
{
5672
code: `
5773
import * as stylex from '@stylexjs/stylex';
@@ -109,6 +125,40 @@ eslintTester.run('stylex-no-conflicting-props', rule.default, {
109125
}
110126
`,
111127
},
128+
{
129+
code: `
130+
import * as stylex from '@stylexjs/stylex';
131+
const styles = stylex.create({
132+
main: { color: 'red' },
133+
});
134+
function Component() {
135+
return <div sx={styles.main} data-testid="x" />;
136+
}
137+
`,
138+
},
139+
{
140+
code: `
141+
import * as stylex from '@stylexjs/stylex';
142+
const styles = stylex.create({
143+
main: { color: 'red' },
144+
});
145+
function Component() {
146+
return <CustomComponent sx={styles.main} className="foo" />;
147+
}
148+
`,
149+
},
150+
{
151+
options: [{ sxPropName: false }],
152+
code: `
153+
import * as stylex from '@stylexjs/stylex';
154+
const styles = stylex.create({
155+
main: { color: 'red' },
156+
});
157+
function Component() {
158+
return <div sx={styles.main} className="foo" />;
159+
}
160+
`,
161+
},
112162
],
113163
invalid: [
114164
{
@@ -304,5 +354,91 @@ eslintTester.run('stylex-no-conflicting-props', rule.default, {
304354
},
305355
],
306356
},
357+
{
358+
code: `
359+
import * as stylex from '@stylexjs/stylex';
360+
const styles = stylex.create({
361+
main: { color: 'red' },
362+
});
363+
function Component() {
364+
return <div sx={styles.main} className="foo" />;
365+
}
366+
`,
367+
errors: [
368+
{
369+
message:
370+
'The `className` prop should not be used with the `sx` StyleX prop to avoid conflicts.',
371+
},
372+
],
373+
},
374+
{
375+
code: `
376+
import * as stylex from '@stylexjs/stylex';
377+
const styles = stylex.create({
378+
main: { color: 'red' },
379+
});
380+
function Component() {
381+
return <div className="foo" sx={styles.main} />;
382+
}
383+
`,
384+
errors: [
385+
{
386+
message:
387+
'The `className` prop should not be used with the `sx` StyleX prop to avoid conflicts.',
388+
},
389+
],
390+
},
391+
{
392+
code: `
393+
import * as stylex from '@stylexjs/stylex';
394+
const styles = stylex.create({
395+
main: { color: 'red' },
396+
});
397+
function Component() {
398+
return <div sx={styles.main} style={{ margin: 10 }} />;
399+
}
400+
`,
401+
errors: [
402+
{
403+
message:
404+
'The `style` prop should not be used with the `sx` StyleX prop to avoid conflicts.',
405+
},
406+
],
407+
},
408+
{
409+
code: `
410+
import * as stylex from '@stylexjs/stylex';
411+
const styles = stylex.create({
412+
main: { color: 'red' },
413+
});
414+
function Component() {
415+
return <div sx={styles.main} {...{ className: 'foo' }} />;
416+
}
417+
`,
418+
errors: [
419+
{
420+
message:
421+
'The `className` prop should not be used with the `sx` StyleX prop to avoid conflicts.',
422+
},
423+
],
424+
},
425+
{
426+
options: [{ sxPropName: 'css' }],
427+
code: `
428+
import * as stylex from '@stylexjs/stylex';
429+
const styles = stylex.create({
430+
main: { color: 'red' },
431+
});
432+
function Component() {
433+
return <div css={styles.main} className="foo" />;
434+
}
435+
`,
436+
errors: [
437+
{
438+
message:
439+
'The `className` prop should not be used with the `css` StyleX prop to avoid conflicts.',
440+
},
441+
],
442+
},
307443
],
308444
});

‎packages/@stylexjs/eslint-plugin/src/stylex-no-conflicting-props.js‎

Lines changed: 57 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,15 @@ type JSXIdentifier = {
1818
+name: string,
1919
};
2020

21+
type JSXExpressionContainer = {
22+
+type: 'JSXExpressionContainer',
23+
+expression: Node | { +type: 'JSXEmptyExpression' },
24+
};
25+
2126
type JSXAttribute = {
2227
+type: 'JSXAttribute',
2328
+name: JSXIdentifier | { +type: string },
29+
+value?: Node | JSXExpressionContainer | null,
2430
};
2531

2632
type JSXSpreadAttribute = {
@@ -30,15 +36,22 @@ type JSXSpreadAttribute = {
3036

3137
type JSXOpeningElement = {
3238
+type: 'JSXOpeningElement',
39+
+name: JSXIdentifier | { +type: string },
3340
+attributes: $ReadOnlyArray<JSXAttribute | JSXSpreadAttribute>,
3441
};
3542

43+
const STYLEX_PROPS_CONFLICTING_PROPS_MESSAGE =
44+
'The `{{propName}}` prop should not be used when spreading `stylex.props()` to avoid conflicts.';
45+
46+
const STYLEX_SHORTHAND_CONFLICTING_PROPS_MESSAGE =
47+
'The `{{propName}}` prop should not be used with the `{{sxPropName}}` StyleX prop to avoid conflicts.';
48+
3649
const stylexNoConflictingProps = {
3750
meta: {
3851
type: 'problem',
3952
docs: {
4053
description:
41-
'Disallow using `className` or `style` props on elements that spread `stylex.props()`',
54+
'Disallow using `className` or `style` props on elements that spread `stylex.props()` or use StyleX JSX shorthand',
4255
category: 'Best Practices',
4356
recommended: true,
4457
},
@@ -62,14 +75,20 @@ const stylexNoConflictingProps = {
6275
},
6376
default: ['stylex', '@stylexjs/stylex'],
6477
},
78+
sxPropName: {
79+
oneOf: [{ type: 'string' }, { enum: [false] }],
80+
default: 'sx',
81+
},
6582
},
6683
additionalProperties: false,
6784
},
6885
],
6986
},
7087
create(context: Rule.RuleContext): { ... } {
71-
const { validImports: importsToLookFor = ['stylex', '@stylexjs/stylex'] } =
72-
context.options[0] || {};
88+
const {
89+
validImports: importsToLookFor = ['stylex', '@stylexjs/stylex'],
90+
sxPropName = 'sx',
91+
} = context.options[0] || {};
7392

7493
const importTracker = createImportTracker(importsToLookFor);
7594

@@ -85,6 +104,30 @@ const stylexNoConflictingProps = {
85104
);
86105
}
87106

107+
function isLowercaseHostElement(node: JSXOpeningElement): boolean {
108+
return (
109+
node.name.type === 'JSXIdentifier' &&
110+
typeof node.name.name === 'string' &&
111+
node.name.name[0] === node.name.name[0].toLowerCase()
112+
);
113+
}
114+
115+
function hasStylexShorthandProp(node: JSXOpeningElement): boolean {
116+
return (
117+
typeof sxPropName === 'string' &&
118+
isLowercaseHostElement(node) &&
119+
node.attributes.some(
120+
(attr) =>
121+
attr.type === 'JSXAttribute' &&
122+
attr.name.type === 'JSXIdentifier' &&
123+
attr.name.name === sxPropName &&
124+
attr.value != null &&
125+
attr.value.type === 'JSXExpressionContainer' &&
126+
attr.value.expression.type !== 'JSXEmptyExpression',
127+
)
128+
);
129+
}
130+
88131
return {
89132
ImportDeclaration: importTracker.ImportDeclaration,
90133

@@ -96,10 +139,16 @@ const stylexNoConflictingProps = {
96139
isStylexPropsCallee(attr.argument.callee),
97140
);
98141

99-
if (!hasStylexPropsSpread) {
142+
const hasStylexShorthand = hasStylexShorthandProp(node);
143+
144+
if (!hasStylexPropsSpread && !hasStylexShorthand) {
100145
return;
101146
}
102147

148+
const message = hasStylexShorthand
149+
? STYLEX_SHORTHAND_CONFLICTING_PROPS_MESSAGE
150+
: STYLEX_PROPS_CONFLICTING_PROPS_MESSAGE;
151+
103152
for (const attr of node.attributes) {
104153
if (
105154
attr.type === 'JSXAttribute' &&
@@ -109,9 +158,8 @@ const stylexNoConflictingProps = {
109158
context.report({
110159
// $FlowFixMe[incompatible-type]
111160
node: attr,
112-
message:
113-
'The `{{propName}}` prop should not be used when spreading `stylex.props()` to avoid conflicts.',
114-
data: { propName: attr.name.name },
161+
message,
162+
data: { propName: attr.name.name, sxPropName },
115163
});
116164
} else if (
117165
attr.type === 'JSXSpreadAttribute' &&
@@ -127,9 +175,8 @@ const stylexNoConflictingProps = {
127175
context.report({
128176
// $FlowFixMe[incompatible-type]
129177
node: prop,
130-
message:
131-
'The `{{propName}}` prop should not be used when spreading `stylex.props()` to avoid conflicts.',
132-
data: { propName: prop.key.name },
178+
message,
179+
data: { propName: prop.key.name, sxPropName },
133180
});
134181
}
135182
}

0 commit comments

Comments
 (0)