Skip to content

Conversation

@jonrohan
Copy link
Member

@jonrohan jonrohan commented Sep 11, 2025

🐛 Problem

The use-styled-react-import rule had two major issues when handling compound components (components with dot notation like ActionList.Item, FormControl.Label, etc.):

  1. Compound components were not recognized: When using <ActionList.Item sx={{...}}>, the rule failed to detect that ActionList should be imported from @primer/styled-react because it was looking for "ActionList.Item" in the styled components list instead of "ActionList".

  2. Incomplete aliasing of compound components: When a parent component required aliasing (e.g., FormControlStyledFormControl), compound components inside the same JSX element were not being updated. For example:

    // Before fix - compound component not updated
    <StyledFormControl sx={{ color: 'red' }}>
      <FormControl.Label>Label</FormControl.Label>  // ❌ Should be StyledFormControl.Label
    </StyledFormControl>

✅ Solution

1. Enhanced Compound Component Recognition

Modified the JSX element handler to extract the parent component name from compound components:

// For compound components like "ActionList.Item", we need to check the parent component
const parentComponentName = originalComponentName.includes('.')
  ? originalComponentName.split('.')[0]
  : originalComponentName

// Track all used components that are in our styled components list
if (styledComponents.has(parentComponentName)) {
  allUsedComponents.add(parentComponentName)
  // ... rest of logic uses parentComponentName
}

This ensures that <ActionList.Item sx={{...}}> correctly identifies that ActionList needs to be moved to @primer/styled-react.

2. Improved Aliasing with Regex-Based Fix

Replaced the individual tag replacement approach with a comprehensive regex-based solution that handles all component instances within a JSX element:

// Replace all instances of the component name (both main component and compound components)
const componentPattern = new RegExp(`\\b${componentName}(?=\\.|\\s|>)`, 'g')
const aliasedText = jsxText.replace(componentPattern, aliasName)
return fixer.replaceText(jsxNode, aliasedText)

This ensures that when FormControl gets aliased to StyledFormControl, all compound components within the same JSX element are also updated:

  • <FormControl sx={{...}}><StyledFormControl sx={{...}}>
  • <FormControl.Label><StyledFormControl.Label>
  • <FormControl.Caption><StyledFormControl.Caption>
  • etc.

📋 Test Cases Added

Added comprehensive test coverage for the new functionality:

  1. Simple compound component: <ActionList.Item sx={{...}}> properly moves ActionList import
  2. Complex aliasing scenario: FormControl used both with and without sx prop, requiring aliasing with proper compound component updates

🧪 Testing

  • ✅ All existing tests continue to pass (301/301)
  • ✅ New test cases verify compound component handling
  • ✅ Manual testing confirms multiple compound components work correctly
  • ✅ No linting errors
  • ✅ Backwards compatibility maintained

📝 Examples

Before Fix

// ❌ Rule failed to detect this case
import { ActionList } from '@primer/react'
const Component = () => <ActionList.Item sx={{ color: 'red' }}>Content</ActionList.Item>

// ❌ Compound components not properly aliased
import { FormControl } from '@primer/react'
import { FormControl as StyledFormControl } from '@primer/styled-react'
const Component = () => (
  <StyledFormControl sx={{ color: 'red' }}>
    <FormControl.Label>Label</FormControl.Label>  // Wrong!
  </StyledFormControl>
)

After Fix

// ✅ Correctly detects and fixes
import { ActionList } from '@primer/styled-react'  // Fixed!
const Component = () => <ActionList.Item sx={{ color: 'red' }}>Content</ActionList.Item>

// ✅ All compound components properly aliased
import { FormControl } from '@primer/react'
import { FormControl as StyledFormControl } from '@primer/styled-react'
const Component = () => (
  <StyledFormControl sx={{ color: 'red' }}>
    <StyledFormControl.Label>Label</StyledFormControl.Label>  // Fixed!
  </StyledFormControl>
)

🎯 Impact

This fix ensures that the use-styled-react-import rule now properly handles all Primer React compound components, making it more reliable for teams using components like:

  • ActionList.Item
  • FormControl.Label, FormControl.Caption, FormControl.Validation
  • PageLayout.Header, PageLayout.Content, PageLayout.Pane
  • And any other compound components that may be added in the future

The rule now provides complete and consistent enforcement of the styled-react import patterns across all component usage scenarios.

@jonrohan jonrohan requested a review from a team as a code owner September 11, 2025 20:55
@changeset-bot
Copy link

changeset-bot bot commented Sep 11, 2025

🦋 Changeset detected

Latest commit: 8b4f659

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
eslint-plugin-primer-react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes two critical issues in the use-styled-react-import ESLint rule when handling compound components (components with dot notation like ActionList.Item, FormControl.Label). The rule now correctly identifies parent components from compound notation and properly handles aliasing across all component instances within JSX elements.

  • Enhanced compound component recognition by extracting parent component names from dot notation
  • Improved aliasing logic using regex-based replacement for complete compound component updates
  • Added comprehensive test coverage for compound component scenarios

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/rules/use-styled-react-import.js Enhanced JSX element handler to recognize parent components in compound notation and improved aliasing with regex-based replacement
src/rules/tests/use-styled-react-import.test.js Added test cases for compound component handling and complex aliasing scenarios

fixes.push(fixer.replaceText(jsxNode.closingElement.name, aliasName))
}
// Replace all instances of the component name (both main component and compound components)
const componentPattern = new RegExp(`\\b${componentName}(?=\\.|\\s|>)`, 'g')
Copy link

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The componentName variable is being used directly in a RegExp constructor without escaping special regex characters. If componentName contains regex metacharacters like '.', '[', or '*', it could cause unexpected matching behavior or regex injection. Use a regex escape function or escape the componentName before constructing the pattern.

Copilot uses AI. Check for mistakes.
@jonrohan jonrohan requested a review from joshblack September 11, 2025 21:29
@jonrohan jonrohan enabled auto-merge (squash) September 11, 2025 21:31
@jonrohan jonrohan merged commit d72e8c4 into main Sep 11, 2025
9 checks passed
@jonrohan jonrohan deleted the component_child_fixes branch September 11, 2025 21:45
@primer-css primer-css mentioned this pull request Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants