Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 27, 2025

Follow-up to #437. When spread props appear before className or event handlers, the spread values get silently overridden unless explicitly merged. This implements detection for these cases.

Changes

New Rules

  • merge-spread-props-classname: Detects className after spread props without merging

    • Recognizes existing merge utilities: clsx, classnames, classNames, cn
    • Autofix: wraps in clsx(rest.className, value)
  • merge-spread-props-event-handlers: Detects event handlers after spread props without merging

    • Supports 60+ React event handlers (onClick, onChange, etc.)
    • Recognizes existing composition utilities: compose, composeEventHandlers, composeHandlers
    • Autofix: wraps in compose(rest.handlerName, handler)

Smart Detection

Both rules only trigger when:

  • Spread props AND the specific prop are both present
  • The specific prop comes after the spread
  • No merge utility is already in use

Example

// ❌ Detected - parent's className lost
<Button {...rest} className="custom" />

// ✅ Fixed
<Button {...rest} className={clsx(rest.className, "custom")} />

// ❌ Detected - parent's onClick lost  
<Button {...rest} onClick={handleClick} />

// ✅ Fixed
<Button {...rest} onClick={compose(rest.onClick, handleClick)} />
Original prompt

This section details on the original issue you should resolve

<issue_title>Add rule to ensure className and event handlers are merged properly</issue_title>
<issue_description>- Follow up to #437

From @joshblack:

Would it be possible to have the rule (or a follow up rule?) that helps people understand when something may not be merged correctly. For example:

-<Example {...rest} className="..." />
+<Example {...rest} className={clsx(className, '...')} />

-<Example {...rest} onClick={() => {}} />
+<Example {...rest} onClick={compose(onClick, () => {})} />

It would be helpful to know in these situations that we have not merged rest correctly. My guess is that it would be too hard in eslint world to know what rest is but figured I'd ask


Added by @siddharthkp:

We probably need to check if there is className or event handlers expected in the props and only then suggest to merge them


Implementation plan:

  • Create 2 new rules in src/rules in the same style as other files in that directory
  • Write tests for these in src/rules/__tests__
  • Add files for documentation in docs/rules
  • Don't blindly add this in every file, first check if there is className or event handlers expected from the props </issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@changeset-bot
Copy link

changeset-bot bot commented Oct 27, 2025

⚠️ No Changeset found

Latest commit: 8c82c53

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copilot AI and others added 3 commits October 27, 2025 11:59
Co-authored-by: siddharthkp <1863771+siddharthkp@users.noreply.github.com>
Co-authored-by: siddharthkp <1863771+siddharthkp@users.noreply.github.com>
Co-authored-by: siddharthkp <1863771+siddharthkp@users.noreply.github.com>
Copilot AI changed the title [WIP] Add rule to ensure className and event handlers are merged properly Add rules to detect unmerged className and event handlers with spread props Oct 27, 2025
Copilot finished work on behalf of siddharthkp October 27, 2025 12:09
Copilot AI requested a review from siddharthkp October 27, 2025 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add rule to ensure className and event handlers are merged properly

2 participants