chore(Table): Remove the CSS modules feature flag from the Table component

[jonrohan]

This PR removes the CSS modules feature flag from the Table component. The component Table and sub components are used 19 times in dotcom.

Changelog

New

Changed

Removed

Removes the CSS modules feature flag from the Table components.

Rollout strategy

• Patch release
• Minor release
• Major release; if selected, include a written rollout or migration plan
• None; if selected, include a brief description as to why

Testing & Reviewing

Using Integration testing.

Merge checklist

• Added/updated tests
• Added/updated documentation
• Added/updated previews (Storybook)
• Changes are SSR compatible
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge
• (GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

[changeset-bot]

🦋 Changeset detected

Latest commit: 7071d53

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

This PR includes changesets to release 1 package

Name	Type
@primer/react	Minor

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

[github-actions]

👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks!

[github-actions]

size-limit report 📦

Path	Size
packages/react/dist/browser.esm.js	104.57 KB (+0.1% 🔺)
packages/react/dist/browser.umd.js	104.99 KB (+0.09% 🔺)

[jonrohan]

It seems like there should be a simpler way of doing this? Can I avoid Box as= when not using sx? @joshblack What do I need to change here?

[joshblack]

I bet you could just render whatever as is directly, specifically:

const TableTitle = React.forwardRef<HTMLElement, TableTitleProps>(function TableTitle({as: BaseComponent = 'h2', children, id}, ref) {

  return (
    <BaseComponent className={clsx('TableTitle', classes.TableTitle)} id={id} ref={ref}>

[jonrohan]

Typescript not happy about that

src/DataTable/Table.tsx:268:6 - error TS2322: Type '{ children: ReactNode; className: string; id: string; ref: ForwardedRef<HTMLElement>; }' is not assignable to type 'SVGProps<SVGSymbolElement>'.
  Types of property 'ref' are incompatible.
    Type 'ForwardedRef<HTMLElement>' is not assignable to type 'LegacyRef<SVGSymbolElement> | undefined'.
      Type '(instance: HTMLElement | null) => void' is not assignable to type 'LegacyRef<SVGSymbolElement> | undefined'.
        Type '(instance: HTMLElement | null) => void' is not assignable to type '(instance: SVGSymbolElement | null) => void'.
          Types of parameters 'instance' and 'instance' are incompatible.
            Type 'SVGSymbolElement | null' is not assignable to type 'HTMLElement | null'.
              Type 'SVGSymbolElement' is missing the following properties from type 'HTMLElement': accessKey, accessKeyLabel, autocapitalize, dir, and 25 more.

268     <BaseComponent className={clsx('TableTitle', classes.TableTitle)} id={id} ref={ref}>
         ~~~~~~~~~~~~~

src/DataTable/Table.tsx:268:20 - error TS2590: Expression produces a union type that is too complex to represent.

268     <BaseComponent className={clsx('TableTitle', classes.TableTitle)} id={id} ref={ref}>
                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[joshblack]

Ah, I think as?: keyof JSX.IntrinsicElements | React.ComponentType is including SVG but the forwardRef is saying HTMLElement

We could extend that for now if we want:

const TableTitle = React.forwardRef<HTMLElement | SVGSymbolElement, TableTitleProps>

Or we could restrict the as? prop to omit things. Let me know what you think 👀

[primer-integration]

👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/371527

[primer-integration]

🔴 golden-jobs completed with status failure.