Skip to content
Closed
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/no-unmerged-classname.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# Ensure className is merged with spread props (no-unmerged-classname)

When using spread props (`{...rest}`, `{...props}`, etc.) along with a `className` prop, you should merge the className from the spread props with your custom className to avoid unintentionally overriding classes.

## Rule details

This rule warns when a component has spread props before a `className` prop, but the `className` doesn't appear to be merging values using a utility like `clsx()` or `classNames()`.

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

```jsx
/* eslint primer-react/no-unmerged-classname: "error" */

// ❌ className may override className from rest
<Example {...rest} className="custom-class" />

// ❌ className expression doesn't merge
<Example {...rest} className={myClassName} />

// ❌ Template literal doesn't merge with rest
<Example {...rest} className={`foo ${bar}`} />
```

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

```jsx
/* eslint primer-react/no-unmerged-classname: "error" */

// ✅ Using clsx to merge className from rest
<Example {...rest} className={clsx(className, "custom-class")} />

// ✅ Using classNames to merge
<Example {...rest} className={classNames(className, "custom-class")} />

// ✅ Using cn utility to merge
<Example {...rest} className={cn(className, "custom-class")} />

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

// ✅ No spread props
<Example className="custom-class" />
```

## Why this matters

When you spread props and then specify a className, you might be overriding a className that was passed in through the spread:

```jsx
// ❌ Bad: className from rest gets lost
function MyComponent({className, ...rest}) {
return <Button {...rest} className="my-custom-class" />
// If rest contains { className: "important-class" }
// Result: className="my-custom-class" (important-class is lost!)
}

// ✅ Good: className from rest is preserved and merged
function MyComponent({className, ...rest}) {
return <Button {...rest} className={clsx(className, 'my-custom-class')} />
// If rest contains { className: "important-class" }
// Result: className="important-class my-custom-class" (both preserved!)
}
```

## Options

This rule has no configuration options.

## When to use this rule

Use this rule when your components accept and spread props that might contain a `className`. This is common in component libraries and wrapper components.

## Related Rules

- [spread-props-first](./spread-props-first.md) - Ensures spread props come before named props
87 changes: 87 additions & 0 deletions docs/rules/no-unmerged-event-handler.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# Ensure event handlers are merged with spread props (no-unmerged-event-handler)

When using spread props (`{...rest}`, `{...props}`, etc.) along with event handler props (like `onClick`, `onChange`, etc.), you should merge the event handler from the spread props with your custom handler to avoid unintentionally overriding event handlers.

## Rule details

This rule warns when a component has spread props before an event handler prop, but the event handler doesn't appear to be merging handlers using a utility like `compose()` or `composeEventHandlers()`.

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

```jsx
/* eslint primer-react/no-unmerged-event-handler: "error" */

// ❌ onClick may override onClick from rest
<Example {...rest} onClick={handleClick} />

// ❌ Arrow function doesn't merge
<Example {...rest} onClick={() => {}} />

// ❌ onChange expression doesn't merge
<Example {...rest} onChange={handleChange} />
```

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

```jsx
/* eslint primer-react/no-unmerged-event-handler: "error" */

// ✅ Using compose to merge onClick from rest
<Example {...rest} onClick={compose(onClick, handleClick)} />

// ✅ Using composeEventHandlers to merge
<Example {...rest} onClick={composeEventHandlers(onClick, handleClick)} />

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

// ✅ No spread props
<Example onClick={handleClick} />
```

## Why this matters

When you spread props and then specify an event handler, you might be overriding an event handler that was passed in through the spread:

```jsx
// ❌ Bad: onClick from rest gets lost
function MyComponent({onClick, ...rest}) {
return <Button {...rest} onClick={() => console.log('clicked')} />
// If rest contains { onClick: importantHandler }
// Result: importantHandler never gets called!
}

// ✅ Good: onClick from rest is preserved and composed
function MyComponent({onClick, ...rest}) {
return <Button {...rest} onClick={compose(onClick, () => console.log('clicked'))} />
// If rest contains { onClick: importantHandler }
// Result: Both importantHandler and the log function get called!
}
```

## Detected Event Handlers

This rule checks for the following event handler props:

- Mouse events: `onClick`, `onDoubleClick`, `onContextMenu`, `onMouseDown`, `onMouseUp`, `onMouseEnter`, `onMouseLeave`, `onMouseMove`, `onMouseOver`, `onMouseOut`
- Touch events: `onTouchStart`, `onTouchEnd`, `onTouchMove`, `onTouchCancel`
- Keyboard events: `onKeyDown`, `onKeyUp`, `onKeyPress`
- Form events: `onChange`, `onSubmit`, `onInput`, `onInvalid`, `onSelect`
- Focus events: `onFocus`, `onBlur`
- Drag events: `onDrag`, `onDragEnd`, `onDragEnter`, `onDragExit`, `onDragLeave`, `onDragOver`, `onDragStart`, `onDrop`
- Other events: `onScroll`, `onWheel`, `onLoad`, `onError`, `onAbort`
- Media events: `onCanPlay`, `onCanPlayThrough`, `onDurationChange`, `onEmptied`, `onEncrypted`, `onEnded`, `onLoadedData`, `onLoadedMetadata`, `onLoadStart`, `onPause`, `onPlay`, `onPlaying`, `onProgress`, `onRateChange`, `onSeeked`, `onSeeking`, `onStalled`, `onSuspend`, `onTimeUpdate`, `onVolumeChange`, `onWaiting`
- Animation/Transition events: `onAnimationStart`, `onAnimationEnd`, `onAnimationIteration`, `onTransitionEnd`

## Options

This rule has no configuration options.

## When to use this rule

Use this rule when your components accept and spread props that might contain event handlers. This is common in component libraries and wrapper components.

## Related Rules

- [spread-props-first](./spread-props-first.md) - Ensures spread props come before named props
- [no-unmerged-classname](./no-unmerged-classname.md) - Ensures className is merged with spread props
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'),
'no-unmerged-classname': require('./rules/no-unmerged-classname'),
'no-unmerged-event-handler': require('./rules/no-unmerged-event-handler'),
},
configs: {
recommended: require('./configs/recommended'),
Expand Down
71 changes: 71 additions & 0 deletions src/rules/__tests__/no-unmerged-classname.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
const rule = require('../no-unmerged-classname')
const {RuleTester} = require('eslint')

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

ruleTester.run('no-unmerged-classname', rule, {
valid: [
// No spread props - OK
`<Example className="foo" />`,
// Spread but no className - OK
`<Example {...rest} />`,
// className before spread (spread overrides) - OK
`<Example className="foo" {...rest} />`,
// className after spread with clsx - OK
`<Example {...rest} className={clsx(className, "foo")} />`,
// className after spread with classNames - OK
`<Example {...rest} className={classNames(className, "foo")} />`,
// className after spread with cn - OK
`<Example {...rest} className={cn(className, "foo")} />`,
// Multiple spreads but className with clsx - OK
`<Example {...rest} {...other} className={clsx(className, "foo")} />`,
],
invalid: [
// className after spread without merging - PROBLEM
{
code: `<Example {...rest} className="foo" />`,
errors: [
{
messageId: 'noUnmergedClassName',
},
],
},
// className after spread with expression but not merging function - PROBLEM
{
code: `<Example {...rest} className={myClassName} />`,
errors: [
{
messageId: 'noUnmergedClassName',
},
],
},
// className after spread with template literal - PROBLEM
{
code: `<Example {...rest} className={\`foo \${bar}\`} />`,
errors: [
{
messageId: 'noUnmergedClassName',
},
],
},
// Multiple spreads with className not merged - PROBLEM
{
code: `<Example {...rest} {...other} className="foo" />`,
errors: [
{
messageId: 'noUnmergedClassName',
},
],
},
],
})
99 changes: 99 additions & 0 deletions src/rules/__tests__/no-unmerged-event-handler.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
const rule = require('../no-unmerged-event-handler')
const {RuleTester} = require('eslint')

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

ruleTester.run('no-unmerged-event-handler', rule, {
valid: [
// No spread props - OK
`<Example onClick={handleClick} />`,
// Spread but no event handler - OK
`<Example {...rest} />`,
// Event handler before spread (spread overrides) - OK
`<Example onClick={handleClick} {...rest} />`,
// Event handler after spread with compose - OK
`<Example {...rest} onClick={compose(onClick, handleClick)} />`,
// Event handler after spread with composeEventHandlers - OK
`<Example {...rest} onClick={composeEventHandlers(onClick, handleClick)} />`,
// Multiple spreads but event handler with compose - OK
`<Example {...rest} {...other} onClick={compose(onClick, handleClick)} />`,
// Different event handlers with compose - OK
`<Example {...rest} onFocus={compose(onFocus, handleFocus)} onBlur={compose(onBlur, handleBlur)} />`,
],
invalid: [
// onClick after spread without merging - PROBLEM
{
code: `<Example {...rest} onClick={handleClick} />`,
errors: [
{
messageId: 'noUnmergedEventHandler',
data: {handlerName: 'onClick'},
},
],
},
// onClick after spread with arrow function - PROBLEM
{
code: `<Example {...rest} onClick={() => {}} />`,
errors: [
{
messageId: 'noUnmergedEventHandler',
data: {handlerName: 'onClick'},
},
],
},
// onChange after spread - PROBLEM
{
code: `<Example {...rest} onChange={handleChange} />`,
errors: [
{
messageId: 'noUnmergedEventHandler',
data: {handlerName: 'onChange'},
},
],
},
// Multiple event handlers not merged - PROBLEM
{
code: `<Example {...rest} onClick={handleClick} onFocus={handleFocus} />`,
errors: [
{
messageId: 'noUnmergedEventHandler',
data: {handlerName: 'onClick'},
},
{
messageId: 'noUnmergedEventHandler',
data: {handlerName: 'onFocus'},
},
],
},
// onSubmit after spread - PROBLEM
{
code: `<Example {...rest} onSubmit={handleSubmit} />`,
errors: [
{
messageId: 'noUnmergedEventHandler',
data: {handlerName: 'onSubmit'},
},
],
},
// Multiple spreads with event handler not merged - PROBLEM
{
code: `<Example {...rest} {...other} onClick={handleClick} />`,
errors: [
{
messageId: 'noUnmergedEventHandler',
data: {handlerName: 'onClick'},
},
],
},
],
})
Loading