Skip to content

Commit b83f467

Browse files
siddharthkpCopilot
siddharthkp
and
Copilot
authored
use-styled-react-import: add theme components (#426)
* add componentsToAlwaysImportFromStyledReact * progress does not look linear * skip complicated test * Update eslint-plugin-primer-react version to minor Add ThemeProvider, BaseStyles, and useTheme to allow theme components to be imported from styled-react without sx. * accept suggestion from copilot Co-authored-by: Copilot <[email protected]> * handle type import --------- Co-authored-by: Copilot <[email protected]>
1 parent eab870e commit b83f467

File tree

3 files changed

+164
-34
lines changed

3 files changed

+164
-34
lines changed

‎.changeset/short-pumas-kneel.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"eslint-plugin-primer-react": minor
3+
---
4+
5+
use-styled-react-import: Add ThemeProvider, BaseStyles and useTheme. Allow theme components to be imported from styled-react without sx

‎src/rules/__tests__/use-styled-react-import.test.js‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ const {RuleTester} = require('eslint')
33

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

2627
// Valid: Utilities imported from styled-react
2728
`import { sx } from '@primer/styled-react'`,
29+
`import { useTheme } from '@primer/styled-react'`,
30+
`import { sx, useTheme } from '@primer/styled-react'`,
2831

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

4144
// Valid: Component without sx prop imported from styled-react (when not used)
4245
`import { Button } from '@primer/styled-react'`,
46+
47+
// Valid: allowedComponents without sx prop imported from styled-react
48+
`import { ThemeProvider, BaseStyles } from '@primer/styled-react'
49+
const Component = ({children}) => <ThemeProvider><BaseStyles>{children}</BaseStyles></ThemeProvider>`,
50+
51+
// Valid: Component with sx prop AND allowedComponents
52+
`import { ThemeProvider, Button } from '@primer/styled-react'
53+
const Component = () => <ThemeProvider><Button sx={{ color: 'btn.bg'}}>Click me</Button></ThemeProvider>`,
4354
],
4455
invalid: [
4556
// Invalid: Box with sx prop imported from @primer/react
@@ -205,6 +216,43 @@ import { Box } from '@primer/styled-react'
205216
},
206217
],
207218
},
219+
{
220+
code: `import { useTheme } from '@primer/react'`,
221+
output: `import { useTheme } from '@primer/styled-react'`,
222+
errors: [
223+
{
224+
messageId: 'moveToStyledReact',
225+
data: {importName: 'useTheme'},
226+
},
227+
],
228+
},
229+
{
230+
code: `import { sx, useTheme } from '@primer/react'`,
231+
output: `import { sx, useTheme } from '@primer/styled-react'`,
232+
errors: [
233+
{
234+
messageId: 'moveToStyledReact',
235+
data: {importName: 'sx'},
236+
},
237+
{
238+
messageId: 'moveToStyledReact',
239+
data: {importName: 'useTheme'},
240+
},
241+
],
242+
},
243+
244+
// Invalid: Utility import from @primer/react that should be from styled-react, mixed with other imports
245+
{
246+
code: `import { sx, useAnchoredPosition } from '@primer/react'`,
247+
output: `import { useAnchoredPosition } from '@primer/react'
248+
import { sx } from '@primer/styled-react'`,
249+
errors: [
250+
{
251+
messageId: 'moveToStyledReact',
252+
data: {importName: 'sx'},
253+
},
254+
],
255+
},
208256

209257
// Invalid: Button and Link, only Button uses sx
210258
{
@@ -335,6 +383,61 @@ import { Button } from '@primer/react'
335383
},
336384
],
337385
},
386+
387+
// Invalid: ThemeProvider and BaseStyles - should move to styled-react
388+
{
389+
code: `
390+
import { ThemeProvider, BaseStyles } from '@primer/react'
391+
`,
392+
output: `
393+
import { ThemeProvider, BaseStyles } from '@primer/styled-react'
394+
`,
395+
errors: [
396+
{
397+
messageId: 'moveToStyledReact',
398+
data: {importName: 'ThemeProvider'},
399+
},
400+
{
401+
messageId: 'moveToStyledReact',
402+
data: {importName: 'BaseStyles'},
403+
},
404+
],
405+
},
406+
407+
// Invalid: ThemeProvider, Button without sx prop - only ThemeProvider should be from styled-react
408+
{
409+
code: `
410+
import { ThemeProvider, Button } from '@primer/react'
411+
const Component = () => <ThemeProvider><Button>Click me</Button></ThemeProvider>
412+
`,
413+
output: `
414+
import { Button } from '@primer/react'
415+
import { ThemeProvider } from '@primer/styled-react'
416+
const Component = () => <ThemeProvider><Button>Click me</Button></ThemeProvider>
417+
`,
418+
errors: [
419+
{
420+
messageId: 'moveToStyledReact',
421+
data: {importName: 'ThemeProvider'},
422+
},
423+
],
424+
},
425+
426+
// Invalid: Utility and type imports from @primer/react that should be from styled-react
427+
{
428+
code: `import { sx, type SxProp } from '@primer/react'`,
429+
output: `import { sx, type SxProp } from '@primer/styled-react'`,
430+
errors: [
431+
{
432+
messageId: 'moveToStyledReact',
433+
data: {importName: 'sx'},
434+
},
435+
{
436+
messageId: 'moveToStyledReact',
437+
data: {importName: 'SxProp'},
438+
},
439+
],
440+
},
338441
],
339442
})
340443

‎src/rules/use-styled-react-import.js‎

Lines changed: 56 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,17 @@ const defaultStyledComponents = [
2323
'Truncate',
2424
'Octicon',
2525
'Dialog',
26+
'ThemeProvider',
27+
'BaseStyles',
2628
]
2729

30+
const componentsToAlwaysImportFromStyledReact = new Set(['ThemeProvider', 'BaseStyles'])
31+
2832
// Default types that should be imported from @primer/styled-react
2933
const defaultStyledTypes = ['BoxProps', 'SxProp', 'BetterSystemStyleObject']
3034

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

3438
/**
3539
* @type {import('eslint').Rule.RuleModule}
@@ -246,7 +250,11 @@ module.exports = {
246250
// Report errors for components used WITHOUT sx prop that are imported from @primer/styled-react
247251
for (const componentName of allUsedComponents) {
248252
// If component is used but NOT with sx prop, and it's imported from styled-react
249-
if (!componentsWithSx.has(componentName) && styledReactImports.has(componentName)) {
253+
if (
254+
!componentsWithSx.has(componentName) &&
255+
styledReactImports.has(componentName) &&
256+
!componentsToAlwaysImportFromStyledReact.has(componentName)
257+
) {
250258
const importInfo = styledReactImports.get(componentName)
251259
context.report({
252260
node: importInfo.specifier,
@@ -337,53 +345,67 @@ module.exports = {
337345
}
338346
}
339347

340-
// Also report for types and utilities that should always be from styled-react
348+
// Also report for types, utilities and components that should always be from styled-react
341349
for (const [importName, importInfo] of primerReactImports) {
342-
if ((styledTypes.has(importName) || styledUtilities.has(importName)) && !styledReactImports.has(importName)) {
350+
if (
351+
(styledTypes.has(importName) ||
352+
styledUtilities.has(importName) ||
353+
componentsToAlwaysImportFromStyledReact.has(importName)) &&
354+
!styledReactImports.has(importName)
355+
) {
343356
context.report({
344357
node: importInfo.specifier,
345358
messageId: 'moveToStyledReact',
346359
data: {importName},
347360
fix(fixer) {
348361
const {node: importNode, specifier, importSource} = importInfo
349-
const otherSpecifiers = importNode.specifiers.filter(s => s !== specifier)
350362

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

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

360-
// Otherwise, remove from current import and add new import
361-
const fixes = []
369+
const specifiersToMove = importNode.specifiers.filter(specifier => {
370+
const name = specifier.imported.name
371+
return (
372+
styledUtilities.has(name) ||
373+
styledTypes.has(name) ||
374+
componentsToAlwaysImportFromStyledReact.has(name)
375+
)
376+
})
377+
378+
const remainingSpecifiers = importNode.specifiers.filter(specifier => {
379+
return !specifiersToMove.includes(specifier)
380+
})
381+
382+
// Convert @primer/react path to @primer/styled-react path
383+
const styledReactPath = importSource.replace('@primer/react', '@primer/styled-react')
362384

363-
// Remove the specifier from current import
364-
if (importNode.specifiers.length === 1) {
385+
if (remainingSpecifiers.length === 0) {
386+
// if there are no remaining specifiers, we can remove the whole import
365387
fixes.push(fixer.remove(importNode))
366388
} else {
367-
const isFirst = importNode.specifiers[0] === specifier
368-
const isLast = importNode.specifiers[importNode.specifiers.length - 1] === specifier
369-
370-
if (isFirst) {
371-
const nextSpecifier = importNode.specifiers[1]
372-
fixes.push(fixer.removeRange([specifier.range[0], nextSpecifier.range[0]]))
373-
} else if (isLast) {
374-
const prevSpecifier = importNode.specifiers[importNode.specifiers.length - 2]
375-
fixes.push(fixer.removeRange([prevSpecifier.range[1], specifier.range[1]]))
376-
} else {
377-
const nextSpecifier = importNode.specifiers[importNode.specifiers.indexOf(specifier) + 1]
378-
fixes.push(fixer.removeRange([specifier.range[0], nextSpecifier.range[0]]))
379-
}
389+
const remainingNames = remainingSpecifiers.map(spec =>
390+
spec.importKind === 'type' ? `type ${spec.imported.name}` : spec.imported.name,
391+
)
392+
fixes.push(
393+
fixer.replaceText(importNode, `import { ${remainingNames.join(', ')} } from '${importSource}'`),
394+
)
380395
}
381396

382-
// Add new import
383-
const prefix = styledTypes.has(importName) ? 'type ' : ''
384-
fixes.push(
385-
fixer.insertTextAfter(importNode, `\nimport { ${prefix}${importName} } from '${styledReactPath}'`),
386-
)
397+
if (specifiersToMove.length > 0) {
398+
const movedComponents = specifiersToMove.map(spec =>
399+
spec.importKind === 'type' ? `type ${spec.imported.name}` : spec.imported.name,
400+
)
401+
const onNewLine = remainingSpecifiers.length > 0
402+
fixes.push(
403+
fixer.insertTextAfter(
404+
importNode,
405+
`${onNewLine ? '\n' : ''}import { ${movedComponents.join(', ')} } from '${styledReactPath}'`,
406+
),
407+
)
408+
}
387409

388410
return fixes
389411
},

0 commit comments

Comments
 (0)