Skip to content
Merged
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
5 changes: 5 additions & 0 deletions .changeset/short-pumas-kneel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-primer-react": minor
---

use-styled-react-import: Add ThemeProvider, BaseStyles and useTheme. Allow theme components to be imported from styled-react without sx
103 changes: 103 additions & 0 deletions src/rules/__tests__/use-styled-react-import.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const {RuleTester} = require('eslint')

const ruleTester = new RuleTester({
languageOptions: {
parser: require(require.resolve('@typescript-eslint/parser', {paths: [require.resolve('eslint-plugin-github')]})),
ecmaVersion: 'latest',
sourceType: 'module',
parserOptions: {
Expand All @@ -25,6 +26,8 @@ ruleTester.run('use-styled-react-import', rule, {

// Valid: Utilities imported from styled-react
`import { sx } from '@primer/styled-react'`,
`import { useTheme } from '@primer/styled-react'`,
`import { sx, useTheme } from '@primer/styled-react'`,

// Valid: Component not in the styled list
`import { Avatar } from '@primer/react'
Expand All @@ -40,6 +43,14 @@ ruleTester.run('use-styled-react-import', rule, {

// Valid: Component without sx prop imported from styled-react (when not used)
`import { Button } from '@primer/styled-react'`,

// Valid: allowedComponents without sx prop imported from styled-react
`import { ThemeProvider, BaseStyles } from '@primer/styled-react'
const Component = ({children}) => <ThemeProvider><BaseStyles>{children}</BaseStyles></ThemeProvider>`,

// Valid: Component with sx prop AND allowedComponents
`import { ThemeProvider, Button } from '@primer/styled-react'
const Component = () => <ThemeProvider><Button sx={{ color: 'btn.bg'}}>Click me</Button></ThemeProvider>`,
],
invalid: [
// Invalid: Box with sx prop imported from @primer/react
Expand Down Expand Up @@ -205,6 +216,43 @@ import { Box } from '@primer/styled-react'
},
],
},
{
code: `import { useTheme } from '@primer/react'`,
output: `import { useTheme } from '@primer/styled-react'`,
errors: [
{
messageId: 'moveToStyledReact',
data: {importName: 'useTheme'},
},
],
},
{
code: `import { sx, useTheme } from '@primer/react'`,
output: `import { sx, useTheme } from '@primer/styled-react'`,
errors: [
{
messageId: 'moveToStyledReact',
data: {importName: 'sx'},
},
{
messageId: 'moveToStyledReact',
data: {importName: 'useTheme'},
},
],
},

// Invalid: Utility import from @primer/react that should be from styled-react, mixed with other imports
{
code: `import { sx, useAnchoredPosition } from '@primer/react'`,
output: `import { useAnchoredPosition } from '@primer/react'
import { sx } from '@primer/styled-react'`,
errors: [
{
messageId: 'moveToStyledReact',
data: {importName: 'sx'},
},
],
},

// Invalid: Button and Link, only Button uses sx
{
Expand Down Expand Up @@ -335,6 +383,61 @@ import { Button } from '@primer/react'
},
],
},

// Invalid: ThemeProvider and BaseStyles - should move to styled-react
{
code: `
import { ThemeProvider, BaseStyles } from '@primer/react'
`,
output: `
import { ThemeProvider, BaseStyles } from '@primer/styled-react'
`,
errors: [
{
messageId: 'moveToStyledReact',
data: {importName: 'ThemeProvider'},
},
{
messageId: 'moveToStyledReact',
data: {importName: 'BaseStyles'},
},
],
},

// Invalid: ThemeProvider, Button without sx prop - only ThemeProvider should be from styled-react
{
code: `
import { ThemeProvider, Button } from '@primer/react'
const Component = () => <ThemeProvider><Button>Click me</Button></ThemeProvider>
`,
output: `
import { Button } from '@primer/react'
import { ThemeProvider } from '@primer/styled-react'
const Component = () => <ThemeProvider><Button>Click me</Button></ThemeProvider>
`,
errors: [
{
messageId: 'moveToStyledReact',
data: {importName: 'ThemeProvider'},
},
],
},

// Invalid: Utility and type imports from @primer/react that should be from styled-react
{
code: `import { sx, type SxProp } from '@primer/react'`,
output: `import { sx, type SxProp } from '@primer/styled-react'`,
errors: [
{
messageId: 'moveToStyledReact',
data: {importName: 'sx'},
},
{
messageId: 'moveToStyledReact',
data: {importName: 'SxProp'},
},
],
},
],
})

Expand Down
90 changes: 56 additions & 34 deletions src/rules/use-styled-react-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,17 @@ const defaultStyledComponents = [
'Truncate',
'Octicon',
'Dialog',
'ThemeProvider',
'BaseStyles',
]

const componentsToAlwaysImportFromStyledReact = new Set(['ThemeProvider', 'BaseStyles'])

// Default types that should be imported from @primer/styled-react
const defaultStyledTypes = ['BoxProps', 'SxProp', 'BetterSystemStyleObject']

// Default utilities that should be imported from @primer/styled-react
const defaultStyledUtilities = ['sx']
const defaultStyledUtilities = ['sx', 'useTheme']

/**
* @type {import('eslint').Rule.RuleModule}
Expand Down Expand Up @@ -246,7 +250,11 @@ module.exports = {
// Report errors for components used WITHOUT sx prop that are imported from @primer/styled-react
for (const componentName of allUsedComponents) {
// If component is used but NOT with sx prop, and it's imported from styled-react
if (!componentsWithSx.has(componentName) && styledReactImports.has(componentName)) {
if (
!componentsWithSx.has(componentName) &&
styledReactImports.has(componentName) &&
!componentsToAlwaysImportFromStyledReact.has(componentName)
) {
const importInfo = styledReactImports.get(componentName)
context.report({
node: importInfo.specifier,
Expand Down Expand Up @@ -337,53 +345,67 @@ module.exports = {
}
}

// Also report for types and utilities that should always be from styled-react
// Also report for types, utilities and components that should always be from styled-react
for (const [importName, importInfo] of primerReactImports) {
if ((styledTypes.has(importName) || styledUtilities.has(importName)) && !styledReactImports.has(importName)) {
if (
(styledTypes.has(importName) ||
styledUtilities.has(importName) ||
componentsToAlwaysImportFromStyledReact.has(importName)) &&
!styledReactImports.has(importName)
) {
context.report({
node: importInfo.specifier,
messageId: 'moveToStyledReact',
data: {importName},
fix(fixer) {
const {node: importNode, specifier, importSource} = importInfo
const otherSpecifiers = importNode.specifiers.filter(s => s !== specifier)

// Convert @primer/react path to @primer/styled-react path
const styledReactPath = importSource.replace('@primer/react', '@primer/styled-react')
const fixes = []

// If this is the only import, replace the whole import
if (otherSpecifiers.length === 0) {
const prefix = styledTypes.has(importName) ? 'type ' : ''
return fixer.replaceText(importNode, `import { ${prefix}${importName} } from '${styledReactPath}'`)
}
// we consolidate all the fixes for the import in the first specifier
const isFirst = importNode.specifiers[0] === specifier
if (!isFirst) return null

// Otherwise, remove from current import and add new import
const fixes = []
const specifiersToMove = importNode.specifiers.filter(specifier => {
const name = specifier.imported.name
return (
styledUtilities.has(name) ||
styledTypes.has(name) ||
componentsToAlwaysImportFromStyledReact.has(name)
)
})

const remainingSpecifiers = importNode.specifiers.filter(specifier => {
return !specifiersToMove.includes(specifier)
})

// Convert @primer/react path to @primer/styled-react path
const styledReactPath = importSource.replace('@primer/react', '@primer/styled-react')

// Remove the specifier from current import
if (importNode.specifiers.length === 1) {
if (remainingSpecifiers.length === 0) {
// if there are no remaining specifiers, we can remove the whole import
fixes.push(fixer.remove(importNode))
} else {
const isFirst = importNode.specifiers[0] === specifier
const isLast = importNode.specifiers[importNode.specifiers.length - 1] === specifier

if (isFirst) {
const nextSpecifier = importNode.specifiers[1]
fixes.push(fixer.removeRange([specifier.range[0], nextSpecifier.range[0]]))
} else if (isLast) {
const prevSpecifier = importNode.specifiers[importNode.specifiers.length - 2]
fixes.push(fixer.removeRange([prevSpecifier.range[1], specifier.range[1]]))
} else {
const nextSpecifier = importNode.specifiers[importNode.specifiers.indexOf(specifier) + 1]
fixes.push(fixer.removeRange([specifier.range[0], nextSpecifier.range[0]]))
}
const remainingNames = remainingSpecifiers.map(spec =>
spec.importKind === 'type' ? `type ${spec.imported.name}` : spec.imported.name,
)
fixes.push(
fixer.replaceText(importNode, `import { ${remainingNames.join(', ')} } from '${importSource}'`),
)
}

// Add new import
const prefix = styledTypes.has(importName) ? 'type ' : ''
fixes.push(
fixer.insertTextAfter(importNode, `\nimport { ${prefix}${importName} } from '${styledReactPath}'`),
)
if (specifiersToMove.length > 0) {
const movedComponents = specifiersToMove.map(spec =>
spec.importKind === 'type' ? `type ${spec.imported.name}` : spec.imported.name,
)
const onNewLine = remainingSpecifiers.length > 0
fixes.push(
fixer.insertTextAfter(
importNode,
`${onNewLine ? '\n' : ''}import { ${movedComponents.join(', ')} } from '${styledReactPath}'`,
),
)
}

return fixes
},
Expand Down