Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions docs/rules/merge-spread-props-classname.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# Ensure className is merged with spread props (merge-spread-props-classname)

When using spread props (`{...rest}`, `{...props}`, etc.) before a `className` prop, the className should be merged using a utility like `clsx` to avoid unintentionally overriding the className from the spread props.

## Rule details

This rule enforces that when a JSX element has both spread props and a `className` prop, and the spread props come before the `className`, the className should use `clsx` or a similar utility to merge both class names.

👎 Examples of **incorrect** code for this rule:

```jsx
/* eslint primer-react/merge-spread-props-classname: "error" */

// ❌ className after spread - not merged
<Example {...rest} className="custom-class" />

// ❌ className expression after spread - not merged
<Example {...rest} className={styles.button} />

// ❌ Multiple spreads with className - not merged
<Example {...rest} {...other} className="custom-class" />
```

👍 Examples of **correct** code for this rule:

```jsx
/* eslint primer-react/merge-spread-props-classname: "error" */

// ✅ className merged with clsx
<Example {...rest} className={clsx(rest.className, "custom-class")} />

// ✅ className merged with classnames
<Example {...rest} className={classnames(rest.className, "custom-class")} />

// ✅ className before spread (spread will override, which is expected)
<Example className="custom-class" {...rest} />

// ✅ Only spread props
<Example {...rest} />

// ✅ Only className
<Example className="custom-class" />
```

## Why this matters

When you have spread props before a className, the className from the spread props can be overridden if you don't merge them properly:

```jsx
// ❌ Bad: className from rest gets overridden
function MyComponent({className, ...rest}) {
return <Button {...rest} className="custom-class" />
}

// If called as: <MyComponent className="parent-class" />
// Result: className="custom-class" (parent-class is lost!)

// ✅ Good: Both classNames are merged
function MyComponent({className, ...rest}) {
return <Button {...rest} className={clsx(rest.className, 'custom-class')} />
}

// If called as: <MyComponent className="parent-class" />
// Result: className="parent-class custom-class" (both are applied!)
```

## Options

This rule has no configuration options.

## When to use autofix

This rule includes an autofix that will automatically wrap your className in a `clsx()` call with the spread prop's className. The autofix is safe to use and will preserve your className logic while adding the merging behavior.

Note: You'll need to import `clsx` in your file if it's not already imported.
91 changes: 91 additions & 0 deletions docs/rules/merge-spread-props-event-handlers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# Ensure event handlers are merged with spread props (merge-spread-props-event-handlers)

When using spread props (`{...rest}`, `{...props}`, etc.) before event handler props (like `onClick`, `onChange`, etc.), the event handlers should be merged using a utility like `compose` to avoid unintentionally overriding the event handler from the spread props.

## Rule details

This rule enforces that when a JSX element has both spread props and event handler props, and the spread props come before the event handlers, the event handlers should use `compose` or a similar utility to merge both handlers.

👎 Examples of **incorrect** code for this rule:

```jsx
/* eslint primer-react/merge-spread-props-event-handlers: "error" */

// ❌ onClick after spread - not merged
<Example {...rest} onClick={handleClick} />

// ❌ onChange after spread - not merged
<Example {...rest} onChange={() => {}} />

// ❌ Multiple event handlers after spread - not merged
<Example {...rest} onClick={handleClick} onChange={handleChange} />
```

👍 Examples of **correct** code for this rule:

```jsx
/* eslint primer-react/merge-spread-props-event-handlers: "error" */

// ✅ onClick merged with compose
<Example {...rest} onClick={compose(rest.onClick, handleClick)} />

// ✅ onChange merged with compose
<Example {...rest} onChange={compose(rest.onChange, handleChange)} />

// ✅ Event handler before spread (spread will override, which is expected)
<Example onClick={handleClick} {...rest} />

// ✅ Only spread props
<Example {...rest} />

// ✅ Only event handler
<Example onClick={handleClick} />
```

## Why this matters

When you have spread props before an event handler, the event handler from the spread props can be overridden if you don't merge them properly:

```jsx
// ❌ Bad: onClick from rest gets overridden
function MyComponent({onClick, ...rest}) {
return <Button {...rest} onClick={handleClick} />
}

// If called as: <MyComponent onClick={parentHandler} />
// Result: Only handleClick runs (parentHandler is lost!)

// ✅ Good: Both handlers are composed
function MyComponent({onClick, ...rest}) {
return <Button {...rest} onClick={compose(rest.onClick, handleClick)} />
}

// If called as: <MyComponent onClick={parentHandler} />
// Result: Both parentHandler and handleClick run in sequence!
```

## Supported event handlers

This rule recognizes the following React event handler props:

- Mouse events: `onClick`, `onMouseEnter`, `onMouseLeave`, `onMouseDown`, `onMouseUp`
- Form events: `onChange`, `onSubmit`, `onInput`, `onSelect`
- Focus events: `onFocus`, `onBlur`
- Keyboard events: `onKeyDown`, `onKeyUp`, `onKeyPress`
- Touch events: `onTouchStart`, `onTouchEnd`, `onTouchMove`, `onTouchCancel`
- Pointer events: `onPointerDown`, `onPointerUp`, `onPointerMove`, etc.
- And many more...

## Options

This rule has no configuration options.

## When to use autofix

This rule includes an autofix that will automatically wrap your event handler in a `compose()` call with the spread prop's event handler. The autofix is safe to use and will preserve your event handler logic while adding the composing behavior.

Note: You'll need to import `compose` or a similar function in your file if it's not already imported. Common utilities include:

- `compose` from utility libraries
- `composeEventHandlers` from Radix UI
- Custom composition utilities from your codebase
2 changes: 2 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ module.exports = {
'enforce-css-module-default-import': require('./rules/enforce-css-module-default-import'),
'use-styled-react-import': require('./rules/use-styled-react-import'),
'spread-props-first': require('./rules/spread-props-first'),
'merge-spread-props-classname': require('./rules/merge-spread-props-classname'),
'merge-spread-props-event-handlers': require('./rules/merge-spread-props-event-handlers'),
},
configs: {
recommended: require('./configs/recommended'),
Expand Down
107 changes: 107 additions & 0 deletions src/rules/__tests__/merge-spread-props-classname.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
const rule = require('../merge-spread-props-classname')
const {RuleTester} = require('eslint')

const ruleTester = new RuleTester({
languageOptions: {
ecmaVersion: 'latest',
sourceType: 'module',
parserOptions: {
ecmaFeatures: {
jsx: true,
},
},
},
})

ruleTester.run('merge-spread-props-classname', rule, {
valid: [
// No spread props
`<Example className="foo" />`,
// Spread props but no className
`<Example {...rest} />`,
// className before spread props (spread will override, which is expected)
`<Example className="foo" {...rest} />`,
// className already using clsx
`<Example {...rest} className={clsx(rest.className, "foo")} />`,
// className already using classnames
`<Example {...rest} className={classnames(rest.className, "foo")} />`,
// className already using classNames (capital N)
`<Example {...rest} className={classNames(rest.className, "foo")} />`,
// className already using cn
`<Example {...rest} className={cn(rest.className, "foo")} />`,
// Multiple spreads but no className
`<Example {...rest} {...other} />`,
],
invalid: [
// className after spread with string literal
{
code: `<Example {...rest} className="foo" />`,
output: `<Example {...rest} className={clsx(rest.className, "foo")} />`,
errors: [
{
messageId: 'mergeClassName',
},
],
},
// className after spread with expression
{
code: `<Example {...rest} className={styles.button} />`,
output: `<Example {...rest} className={clsx(rest.className, styles.button)} />`,
errors: [
{
messageId: 'mergeClassName',
},
],
},
// className after spread with template literal
{
code: '<Example {...rest} className={`foo ${bar}`} />',
output: '<Example {...rest} className={clsx(rest.className, `foo ${bar}`)} />',
errors: [
{
messageId: 'mergeClassName',
},
],
},
// Multiple spreads with className after first spread
{
code: `<Example {...rest} {...other} className="foo" />`,
output: `<Example {...rest} {...other} className={clsx(rest.className, "foo")} />`,
errors: [
{
messageId: 'mergeClassName',
},
],
},
// className after spread with custom prop name
{
code: `<Example {...props} className="foo" />`,
output: `<Example {...props} className={clsx(props.className, "foo")} />`,
errors: [
{
messageId: 'mergeClassName',
},
],
},
// Complex className expression
{
code: `<Example {...rest} className={someCondition ? "foo" : "bar"} />`,
output: `<Example {...rest} className={clsx(rest.className, someCondition ? "foo" : "bar")} />`,
errors: [
{
messageId: 'mergeClassName',
},
],
},
// className in the middle of spreads
{
code: `<Example {...rest} className="foo" {...other} />`,
output: `<Example {...rest} className={clsx(rest.className, "foo")} {...other} />`,
errors: [
{
messageId: 'mergeClassName',
},
],
},
],
})
Loading