Skip to content

Commit 7169487

Browse files
committed
Refine merge props migration reporting
Ignore pure prop-forwarding wrappers so the adoption report only tracks component roots with props that need intentional merging. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 00bfd295-6ed9-4119-8d63-103a0b3c63e8
1 parent 82dc2d4 commit 7169487

3 files changed

Lines changed: 42 additions & 14 deletions

File tree

packages/eslint-config/src/rules/preferMergeProps.test.ts

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,18 @@ ruleTester.run('prefer-merge-props', preferMergeProps as unknown as Parameters<R
4343
name: 'ignores elements without spread props',
4444
code: `function Example() { return <button type="button" /> }`,
4545
},
46+
{
47+
name: 'ignores a component that only forwards consumer props',
48+
code: `function Example(props) { return <button {...props} /> }`,
49+
},
50+
{
51+
name: 'ignores a component that forwards consumer props and composes a ref separately',
52+
code: `const Example = React.forwardRef((props, ref) => <button {...props} ref={ref} />)`,
53+
},
54+
{
55+
name: 'ignores a component that only forwards props to a custom component',
56+
code: `function Example(props) { return <Button {...props} /> }`,
57+
},
4658
{
4759
name: 'ignores spread props on nested elements',
4860
code: `function Example() { return <div><button {...props} /></div> }`,
@@ -132,27 +144,27 @@ ruleTester.run('prefer-merge-props', preferMergeProps as unknown as Parameters<R
132144
},
133145
{
134146
name: 'reports component roots in conditional expression branches',
135-
code: `function Example() { return condition ? <button {...props} /> : <a {...props} /> }`,
147+
code: `function Example() { return condition ? <button type="button" {...props} /> : <a href="#" {...props} /> }`,
136148
errors: [{messageId: 'preferMergeProps'}, {messageId: 'preferMergeProps'}],
137149
},
138150
{
139151
name: 'reports unmerged props on a root custom component',
140-
code: `function Example() { return <Button {...props} /> }`,
152+
code: `function Example() { return <Button variant="primary" {...props} /> }`,
141153
errors: [{messageId: 'preferMergeProps'}],
142154
},
143155
{
144156
name: 'reports unmerged props on a root member component',
145-
code: `function Example() { return <ActionList.Item {...props} /> }`,
157+
code: `function Example() { return <ActionList.Item role="menuitem" {...props} /> }`,
146158
errors: [{messageId: 'preferMergeProps'}],
147159
},
148160
{
149161
name: 'reports spread props returned from a function call',
150-
code: `function Example() { return <button {...getProps()} /> }`,
162+
code: `function Example() { return <button type="button" {...getProps()} /> }`,
151163
errors: [{messageId: 'preferMergeProps'}],
152164
},
153165
{
154166
name: 'reports props copied into an object expression',
155-
code: `function Example() { return <button {...{...props}} /> }`,
167+
code: `function Example() { return <button type="button" {...{...props}} /> }`,
156168
errors: [{messageId: 'preferMergeProps'}],
157169
},
158170
{
@@ -162,24 +174,24 @@ ruleTester.run('prefer-merge-props', preferMergeProps as unknown as Parameters<R
162174
},
163175
{
164176
name: 'reports props on component roots in logical expressions',
165-
code: `function Example() { return condition && <button {...props} /> }`,
177+
code: `function Example() { return condition && <button type="button" {...props} /> }`,
166178
errors: [{messageId: 'preferMergeProps'}],
167179
},
168180
{
169181
name: 'reports props on a concise arrow component root',
170-
code: `const Example = props => <button {...props} />`,
182+
code: `const Example = props => <button type="button" {...props} />`,
171183
errors: [{messageId: 'preferMergeProps'}],
172184
},
173185
{
174186
name: 'reports props on a forwardRef component root',
175-
code: `const Example = React.forwardRef((props, ref) => <button {...props} ref={ref} />)`,
187+
code: `const Example = React.forwardRef((props, ref) => <button type="button" {...props} ref={ref} />)`,
176188
errors: [{messageId: 'preferMergeProps'}],
177189
},
178190
{
179191
name: 'reports props on an asserted forwardRef component root',
180192
code: `
181193
const Example = React.forwardRef(
182-
(props, ref) => <button {...props} ref={ref} />
194+
(props, ref) => <button type="button" {...props} ref={ref} />
183195
) as PolymorphicForwardRefComponent
184196
`,
185197
errors: [{messageId: 'preferMergeProps'}],
@@ -188,14 +200,14 @@ ruleTester.run('prefer-merge-props', preferMergeProps as unknown as Parameters<R
188200
name: 'reports props on a forwardRef component root using satisfies',
189201
code: `
190202
const Example = React.forwardRef(
191-
(props, ref) => <button {...props} ref={ref} />
203+
(props, ref) => <button type="button" {...props} ref={ref} />
192204
) satisfies PolymorphicForwardRefComponent
193205
`,
194206
errors: [{messageId: 'preferMergeProps'}],
195207
},
196208
{
197209
name: 'reports props on a class component root',
198-
code: `class Example extends React.Component { render() { return <button {...this.props} /> } }`,
210+
code: `class Example extends React.Component { render() { return <button type="button" {...this.props} /> } }`,
199211
errors: [{messageId: 'preferMergeProps'}],
200212
},
201213
],

packages/eslint-config/src/rules/preferMergeProps.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ const preferMergeProps: TSESLint.RuleModule<MessageIds> = {
2323

2424
return {
2525
JSXOpeningElement(node) {
26-
if (!isComponentRoot(node.parent)) {
26+
if (!isComponentRoot(node.parent) || !hasPropsToMerge(node)) {
2727
return
2828
}
2929

@@ -43,6 +43,22 @@ const preferMergeProps: TSESLint.RuleModule<MessageIds> = {
4343
},
4444
}
4545

46+
function hasPropsToMerge(node: TSESTree.JSXOpeningElement): boolean {
47+
const spreadCount = node.attributes.filter(attribute => attribute.type === 'JSXSpreadAttribute').length
48+
if (spreadCount > 1) {
49+
return true
50+
}
51+
52+
return node.attributes.some(attribute => {
53+
return (
54+
attribute.type === 'JSXAttribute' &&
55+
attribute.name.type === 'JSXIdentifier' &&
56+
attribute.name.name !== 'key' &&
57+
attribute.name.name !== 'ref'
58+
)
59+
})
60+
}
61+
4662
function isTestOrStoryFile(filename: string): boolean {
4763
const normalizedFilename = filename.replaceAll('\\', '/')
4864
return (

script/merge-props-migration-status.mts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,11 @@ const violationCount = affectedResults.reduce((count, result) => {
3434
write(`
3535
# mergeProps Migration
3636
37-
This report tracks component root prop spreads that should migrate to \`mergeProps\`.
37+
This report tracks component roots that combine authored props with unmerged spread props.
3838
3939
## Status
4040
41-
**Component root prop spreads to migrate:** ${violationCount}
41+
**Unmerged component root prop spreads to migrate:** ${violationCount}
4242
4343
**Affected files:** ${affectedResults.length} of ${results.length}
4444
`)

0 commit comments

Comments
 (0)