Skip to content

Commit 9270d40

Browse files
Copilotsiddharthkp
Copilot
and
siddharthkp
authored
Add eslint rule to ensure spread props come before other props (#437)
* Initial plan * Add spread-props-first ESLint rule implementation Co-authored-by: siddharthkp <1863771+siddharthkp@users.noreply.github.com> * Add clarifying comment for ESLint version compatibility Co-authored-by: siddharthkp <1863771+siddharthkp@users.noreply.github.com> * Remove ESLint version compatibility code Co-authored-by: siddharthkp <1863771+siddharthkp@users.noreply.github.com> * Add changeset for spread-props-first rule Co-authored-by: siddharthkp <1863771+siddharthkp@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: siddharthkp <1863771+siddharthkp@users.noreply.github.com>
1 parent 368f49c commit 9270d40

File tree

5 files changed

+276
-0
lines changed

5 files changed

+276
-0
lines changed

‎.changeset/upset-cobras-cross.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'eslint-plugin-primer-react': minor
3+
---
4+
5+
Add spread-props-first rule to ensure spread props come before other props

‎docs/rules/spread-props-first.md‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
# Ensure spread props come before other props (spread-props-first)
2+
3+
Spread props should come before other named props to avoid unintentionally overriding props. When spread props are placed after named props, they can override the named props, which is often unintended and can lead to UI bugs.
4+
5+
## Rule details
6+
7+
This rule enforces that all spread props (`{...rest}`, `{...props}`, etc.) come before any named props in JSX elements.
8+
9+
👎 Examples of **incorrect** code for this rule:
10+
11+
```jsx
12+
/* eslint primer-react/spread-props-first: "error" */
13+
14+
// ❌ Spread after named prop
15+
<Example className="..." {...rest} />
16+
17+
// ❌ Spread in the middle
18+
<Example className="..." {...rest} id="foo" />
19+
20+
// ❌ Multiple spreads after named props
21+
<Example className="..." {...rest} {...other} />
22+
```
23+
24+
👍 Examples of **correct** code for this rule:
25+
26+
```jsx
27+
/* eslint primer-react/spread-props-first: "error" */
28+
29+
// ✅ Spread before named props
30+
<Example {...rest} className="..." />
31+
32+
// ✅ Multiple spreads before named props
33+
<Example {...rest} {...other} className="..." />
34+
35+
// ✅ Only spread props
36+
<Example {...rest} />
37+
38+
// ✅ Only named props
39+
<Example className="..." id="foo" />
40+
```
41+
42+
## Why this matters
43+
44+
Placing spread props after named props can cause unexpected behavior:
45+
46+
```jsx
47+
// ❌ Bad: className might get overridden by rest
48+
<Button className="custom-class" {...rest} />
49+
50+
// If rest = { className: "other-class" }
51+
// Result: className="other-class" (custom-class is lost!)
52+
53+
// ✅ Good: className will override any className in rest
54+
<Button {...rest} className="custom-class" />
55+
56+
// If rest = { className: "other-class" }
57+
// Result: className="custom-class" (as intended)
58+
```
59+
60+
## Options
61+
62+
This rule has no configuration options.
63+
64+
## When to use autofix
65+
66+
This rule includes an autofix that will automatically reorder your props to place all spread props first. The autofix is safe to use as it preserves the order of spreads relative to each other and the order of named props relative to each other.

‎src/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ module.exports = {
2020
'enforce-css-module-identifier-casing': require('./rules/enforce-css-module-identifier-casing'),
2121
'enforce-css-module-default-import': require('./rules/enforce-css-module-default-import'),
2222
'use-styled-react-import': require('./rules/use-styled-react-import'),
23+
'spread-props-first': require('./rules/spread-props-first'),
2324
},
2425
configs: {
2526
recommended: require('./configs/recommended'),

‎src/rules/__tests__/spread-props-first.test.js‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
const rule = require('../spread-props-first')
2+
const {RuleTester} = require('eslint')
3+
4+
const ruleTester = new RuleTester({
5+
languageOptions: {
6+
ecmaVersion: 'latest',
7+
sourceType: 'module',
8+
parserOptions: {
9+
ecmaFeatures: {
10+
jsx: true,
11+
},
12+
},
13+
},
14+
})
15+
16+
ruleTester.run('spread-props-first', rule, {
17+
valid: [
18+
// Spread props before named props
19+
`<Example {...rest} className="foo" />`,
20+
// Multiple spreads before named props
21+
`<Example {...rest} {...other} className="foo" id="bar" />`,
22+
// Only spread props
23+
`<Example {...rest} />`,
24+
// Only named props
25+
`<Example className="foo" id="bar" />`,
26+
// Empty element
27+
`<Example />`,
28+
// Spread first, then named props
29+
`<Example {...rest} className="foo" onClick={handleClick} />`,
30+
// Multiple spreads at the beginning
31+
`<Example {...props1} {...props2} {...props3} className="foo" />`,
32+
],
33+
invalid: [
34+
// Named prop before spread
35+
{
36+
code: `<Example className="foo" {...rest} />`,
37+
output: `<Example {...rest} className="foo" />`,
38+
errors: [
39+
{
40+
messageId: 'spreadPropsFirst',
41+
data: {spreadProp: '{...rest}', namedProp: 'className'},
42+
},
43+
],
44+
},
45+
// Multiple named props before spread
46+
{
47+
code: `<Example className="foo" id="bar" {...rest} />`,
48+
output: `<Example {...rest} className="foo" id="bar" />`,
49+
errors: [
50+
{
51+
messageId: 'spreadPropsFirst',
52+
data: {spreadProp: '{...rest}', namedProp: 'id'},
53+
},
54+
],
55+
},
56+
// Named prop with expression before spread
57+
{
58+
code: `<Example onClick={handleClick} {...rest} />`,
59+
output: `<Example {...rest} onClick={handleClick} />`,
60+
errors: [
61+
{
62+
messageId: 'spreadPropsFirst',
63+
data: {spreadProp: '{...rest}', namedProp: 'onClick'},
64+
},
65+
],
66+
},
67+
// Mixed order with multiple spreads
68+
{
69+
code: `<Example className="foo" {...rest} id="bar" {...other} />`,
70+
output: `<Example {...rest} {...other} className="foo" id="bar" />`,
71+
errors: [
72+
{
73+
messageId: 'spreadPropsFirst',
74+
data: {spreadProp: '{...rest}', namedProp: 'id'},
75+
},
76+
],
77+
},
78+
// Named prop before multiple spreads
79+
{
80+
code: `<Example className="foo" {...rest} {...other} />`,
81+
output: `<Example {...rest} {...other} className="foo" />`,
82+
errors: [
83+
{
84+
messageId: 'spreadPropsFirst',
85+
data: {spreadProp: '{...rest}', namedProp: 'className'},
86+
},
87+
],
88+
},
89+
// Complex example with many props
90+
{
91+
code: `<Example className="foo" id="bar" onClick={handleClick} {...rest} disabled />`,
92+
output: `<Example {...rest} className="foo" id="bar" onClick={handleClick} disabled />`,
93+
errors: [
94+
{
95+
messageId: 'spreadPropsFirst',
96+
data: {spreadProp: '{...rest}', namedProp: 'disabled'},
97+
},
98+
],
99+
},
100+
// Boolean prop before spread
101+
{
102+
code: `<Example disabled {...rest} />`,
103+
output: `<Example {...rest} disabled />`,
104+
errors: [
105+
{
106+
messageId: 'spreadPropsFirst',
107+
data: {spreadProp: '{...rest}', namedProp: 'disabled'},
108+
},
109+
],
110+
},
111+
// Spread in the middle
112+
{
113+
code: `<Example className="foo" {...rest} id="bar" />`,
114+
output: `<Example {...rest} className="foo" id="bar" />`,
115+
errors: [
116+
{
117+
messageId: 'spreadPropsFirst',
118+
data: {spreadProp: '{...rest}', namedProp: 'id'},
119+
},
120+
],
121+
},
122+
],
123+
})

‎src/rules/spread-props-first.js‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
module.exports = {
2+
meta: {
3+
type: 'problem',
4+
fixable: 'code',
5+
schema: [],
6+
messages: {
7+
spreadPropsFirst:
8+
'Spread props should come before other props to avoid unintentional overrides. Move {{spreadProp}} before {{namedProp}}.',
9+
},
10+
},
11+
create(context) {
12+
return {
13+
JSXOpeningElement(node) {
14+
const attributes = node.attributes
15+
16+
// Track if we've seen a named prop before a spread
17+
let lastNamedPropIndex = -1
18+
let firstSpreadAfterNamedPropIndex = -1
19+
20+
for (let i = 0; i < attributes.length; i++) {
21+
const attr = attributes[i]
22+
23+
if (attr.type === 'JSXAttribute') {
24+
// This is a named prop
25+
lastNamedPropIndex = i
26+
} else if (attr.type === 'JSXSpreadAttribute' && lastNamedPropIndex !== -1) {
27+
// This is a spread prop that comes after a named prop
28+
if (firstSpreadAfterNamedPropIndex === -1) {
29+
firstSpreadAfterNamedPropIndex = i
30+
}
31+
}
32+
}
33+
34+
// If we found a spread after a named prop, report it
35+
if (firstSpreadAfterNamedPropIndex !== -1) {
36+
const sourceCode = context.sourceCode
37+
const spreadAttr = attributes[firstSpreadAfterNamedPropIndex]
38+
const namedAttr = attributes[lastNamedPropIndex]
39+
40+
context.report({
41+
node: spreadAttr,
42+
messageId: 'spreadPropsFirst',
43+
data: {
44+
spreadProp: sourceCode.getText(spreadAttr),
45+
namedProp: namedAttr.name.name,
46+
},
47+
fix(fixer) {
48+
// Collect all spreads and named props
49+
const spreads = []
50+
const namedProps = []
51+
52+
for (const attr of attributes) {
53+
if (attr.type === 'JSXSpreadAttribute') {
54+
spreads.push(attr)
55+
} else if (attr.type === 'JSXAttribute') {
56+
namedProps.push(attr)
57+
}
58+
}
59+
60+
// Generate the reordered attributes text
61+
const reorderedAttrs = [...spreads, ...namedProps]
62+
const fixes = []
63+
64+
// Replace each attribute with its new position
65+
for (let i = 0; i < attributes.length; i++) {
66+
const newAttr = reorderedAttrs[i]
67+
const oldAttr = attributes[i]
68+
69+
if (newAttr !== oldAttr) {
70+
fixes.push(fixer.replaceText(oldAttr, sourceCode.getText(newAttr)))
71+
}
72+
}
73+
74+
return fixes
75+
},
76+
})
77+
}
78+
},
79+
}
80+
},
81+
}

0 commit comments

Comments
 (0)