Skip to content

Commit

Permalink
Merge pull request #39 from primer/slots-rule
Browse files Browse the repository at this point in the history
  • Loading branch information
colebemis authored Mar 22, 2023
2 parents 191d35e + 0bda0ed commit 015237e
Show file tree
Hide file tree
Showing 8 changed files with 190 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/soft-rabbits-stare.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-primer-react": major
---

Add `direct-slot-children` rule
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,6 @@ ESLint rules for Primer React

## Rules

- [direct-slot-children](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/direct-slot-children.md)
- [no-deprecated-colors](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/no-deprecated-colors.md)
- [no-system-props](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/no-system-props.md)
39 changes: 39 additions & 0 deletions docs/rules/direct-slot-children.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Enforce direct parent-child relationship of slot components (direct-slot-children)

Some Primer React components use a slots pattern under the hood to render subcomponents in specific places. For example, the `PageLayout` component renders `PageLayout.Header` in the header area, and `PageLayout.Footer` in the footer area. These subcomponents must be direct children of the parent component, and cannot be nested inside other components.

## Rule details

This rule enforces that slot components are direct children of their parent component.

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

```jsx
/* eslint primer-react/direct-slot-children: "error" */
import {PageLayout} from '@primer/react'

const MyHeader = () => <PageLayout.Header>Header</PageLayout.Header>

const App = () => (
<PageLayout>
<MyHeader />
</PageLayout>
)
```

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

```jsx
/* eslint primer-react/direct-slot-children: "error" */
import {PageLayout} from '@primer/react'

const MyHeader = () => <div>Header</div>

const App = () => (
<PageLayout>
<PageLayout.Header>
<MyHeader />
</PageLayout.Header>
</PageLayout>
)
```
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
"jest": "^27.0.6"
},
"peerDependencies": {
"eslint": "^8.0.1",
"@primer/primitives": ">=4.6.2"
"@primer/primitives": ">=4.6.2",
"eslint": "^8.0.1"
},
"prettier": "@github/prettier-config",
"dependencies": {
Expand Down
67 changes: 67 additions & 0 deletions src/rules/__tests__/direct-slot-children.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
const rule = require('../direct-slot-children')
const {RuleTester} = require('eslint')

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

ruleTester.run('direct-slot-children', rule, {
valid: [
`import {PageLayout} from '@primer/react'; <PageLayout><PageLayout.Header>Header</PageLayout.Header><PageLayout.Footer>Footer</PageLayout.Footer></PageLayout>`,
`import {PageLayout} from '@primer/react'; <PageLayout><div><PageLayout.Pane>Header</PageLayout.Pane></div></PageLayout>`,
`import {PageLayout} from 'some-library'; <PageLayout.Header>Header</PageLayout.Header>`
],
invalid: [
{
code: `import {PageLayout} from '@primer/react'; <PageLayout.Header>Header</PageLayout.Header>`,
errors: [
{
messageId: 'directSlotChildren',
data: {childName: 'PageLayout.Header', parentName: 'PageLayout'}
}
]
},
{
code: `import {PageLayout} from '@primer/react/drafts'; <PageLayout.Header>Header</PageLayout.Header>`,
errors: [
{
messageId: 'directSlotChildren',
data: {childName: 'PageLayout.Header', parentName: 'PageLayout'}
}
]
},
{
code: `import {PageLayout} from '@primer/react'; <div><PageLayout.Header>Header</PageLayout.Header></div>`,
errors: [
{
messageId: 'directSlotChildren',
data: {childName: 'PageLayout.Header', parentName: 'PageLayout'}
}
]
},
{
code: `import {PageLayout} from '@primer/react'; <PageLayout><div><PageLayout.Header>Header</PageLayout.Header></div></PageLayout>`,
errors: [
{
messageId: 'directSlotChildren',
data: {childName: 'PageLayout.Header', parentName: 'PageLayout'}
}
]
},
{
code: `import {TreeView} from '@primer/react'; <TreeView><TreeView.Item><div><TreeView.LeadingVisual>Visual</TreeView.LeadingVisual></div></TreeView.Item></TreeView>`,
errors: [
{
messageId: 'directSlotChildren',
data: {childName: 'TreeView.LeadingVisual', parentName: 'TreeView.Item'}
}
]
}
]
})
60 changes: 60 additions & 0 deletions src/rules/direct-slot-children.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
const {isPrimerComponent} = require('../utils/is-primer-component')

const slotParentToChildMap = {
PageLayout: ['PageLayout.Header', 'PageLayout.Footer'],
FormControl: ['FormControl.Label', 'FormControl.Caption', 'FormControl.LeadingVisual', 'FormControl.TrailingVisual'],
MarkdownEditor: ['MarkdownEditor.Toolbar', 'MarkdownEditor.Actions', 'MarkdownEditor.Label'],
'ActionList.Item': ['ActionList.LeadingVisual', 'ActionList.TrailingVisual', 'ActionList.Description'],
'TreeView.Item': ['TreeView.LeadingVisual', 'TreeView.TrailingVisual'],
RadioGroup: ['RadioGroup.Label', 'RadioGroup.Caption', 'RadioGroup.Validation'],
CheckboxGroup: ['CheckboxGroup.Label', 'CheckboxGroup.Caption', 'CheckboxGroup.Validation']
}

const slotChildToParentMap = Object.entries(slotParentToChildMap).reduce((acc, [parent, children]) => {
for (const child of children) {
acc[child] = parent
}
return acc
}, {})

module.exports = {
meta: {
type: 'problem',
schema: [],
messages: {
directSlotChildren: '{{childName}} must be a direct child of {{parentName}}.'
}
},
create(context) {
return {
JSXOpeningElement(jsxNode) {
const name = getJSXOpeningElementName(jsxNode)

// If component is a Primer component and a slot child,
// check if it's a direct child of the slot parent
if (isPrimerComponent(jsxNode.name, context.getScope(jsxNode)) && slotChildToParentMap[name]) {
const JSXElement = jsxNode.parent
const parent = JSXElement.parent

const expectedParentName = slotChildToParentMap[name]
if (parent.type !== 'JSXElement' || getJSXOpeningElementName(parent.openingElement) !== expectedParentName) {
context.report({
node: jsxNode,
messageId: 'directSlotChildren',
data: {childName: name, parentName: expectedParentName}
})
}
}
}
}
}
}

// Convert JSXOpeningElement name to string
function getJSXOpeningElementName(jsxNode) {
if (jsxNode.name.type === 'JSXIdentifier') {
return jsxNode.name.name
} else if (jsxNode.name.type === 'JSXMemberExpression') {
return `${jsxNode.name.object.name}.${jsxNode.name.property.name}`
}
}
15 changes: 14 additions & 1 deletion src/utils/is-primer-component.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,19 @@
const {isImportedFrom} = require('./is-imported-from')

function isPrimerComponent(identifier, scope) {
function isPrimerComponent(name, scope) {
let identifier

switch (name.type) {
case 'JSXIdentifier':
identifier = name
break
case 'JSXMemberExpression':
identifier = name.object
break
default:
return false
}

return isImportedFrom(/^@primer\/react/, identifier, scope)
}
exports.isPrimerComponent = isPrimerComponent

0 comments on commit 015237e

Please sign in to comment.