Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
d95b2f9
rm Box and sx prop from components, add to styled-react
mperrotti Sep 10, 2025
778eca1
fixes NavList/ActionList type errors surfaced when creating wrappers …
mperrotti Sep 10, 2025
dac0887
adds PageHeader to styled-react and replaces its legacy polymorphic t…
mperrotti Sep 10, 2025
ea20922
fix type errors in styled-react UnderlineNav/UnderlineNav.Item
mperrotti Sep 10, 2025
3645c19
adds changeset
mperrotti Sep 10, 2025
58b207b
fix typescript issues in styled-react component ports
mperrotti Sep 11, 2025
73803e7
Merge branch 'main' into mp/rm-box-and-sx-from-components
mperrotti Sep 11, 2025
a0bbeb1
fixes mistake in UnderlineNavItem prop type
mperrotti Sep 11, 2025
487ae53
Merge branch 'mp/rm-box-and-sx-from-components' of github.com:primer/…
mperrotti Sep 11, 2025
c142b2e
Merge branch 'main' of github.com:primer/react into mp/rm-box-and-sx-…
mperrotti Sep 11, 2025
c8cf28b
Merge branch 'main' of github.com:primer/react into mp/rm-box-and-sx-…
mperrotti Sep 12, 2025
23b3fb1
revert Dialog changes (handled in separate PR)
mperrotti Sep 12, 2025
f742094
fixes type errors
mperrotti Sep 12, 2025
e6c77b6
fixes type error in PageLayout default story
mperrotti Sep 12, 2025
5683bc0
fixes failing VRT for PageLayout Dev Default story
mperrotti Sep 12, 2025
75b602a
replaces polymorphic2 with modern-polymorphic
mperrotti Sep 12, 2025
7c427b9
fixes type error in styled-react PageHeader
mperrotti Sep 12, 2025
f2efa0d
fixes silly typo causing type errors
mperrotti Sep 12, 2025
66d0ecb
fixes type errors in AriaAlert, and AriaStatus
mperrotti Sep 12, 2025
1801545
Merge branch 'main' of github.com:primer/react into mp/rm-box-and-sx-…
mperrotti Sep 15, 2025
cc36c5d
updates snapshots
mperrotti Sep 15, 2025
ecb1484
Merge branch 'main' into mp/rm-box-and-sx-from-components
mperrotti Sep 15, 2025
d7e66df
Merge branch 'main' of github.com:primer/react into mp/rm-box-and-sx-…
mperrotti Sep 16, 2025
634a103
adds SxProp type to styled-react
mperrotti Sep 16, 2025
6f04f57
attempts to fix type errors in integration tests
mperrotti Sep 16, 2025
a664932
more ActionList type fixes
mperrotti Sep 17, 2025
f6bcc88
UnderlineNavItem type fixes
mperrotti Sep 17, 2025
dd28ce5
get rid of ts-expect-error directives (woohoo)
mperrotti Sep 17, 2025
82ab9a4
still trying to fix ActionList type errors - ActionMenu.tsx having is…
mperrotti Sep 17, 2025
b8ffdbd
more ActionList.Item type fixes, patches new type errors surfaced in …
mperrotti Sep 17, 2025
41477d0
ugh linter fix
mperrotti Sep 17, 2025
06ae495
trying a dumb fix for new ActionList integration test error
mperrotti Sep 17, 2025
46952dd
trying an even dumber fix for ActionList.Item onSelect type errors
mperrotti Sep 17, 2025
90776ed
revert NavList and ActionList changes to shrink the PR
mperrotti Sep 17, 2025
2463320
reverts changes to AutocompleteMenu
mperrotti Sep 17, 2025
4c3f7e0
reverts changes to AutocompleteMenu
mperrotti Sep 17, 2025
b3cbc20
reverts styled-react tests for NavList
mperrotti Sep 17, 2025
e8e9312
reverts UnderlineNav changes
mperrotti Sep 17, 2025
b2913cb
reverts UnderlineNav changes in styled-react
mperrotti Sep 17, 2025
268ca7b
reverts UnderlineTabbedInterface changes
mperrotti Sep 17, 2025
267943e
reverts PageHeader changes
mperrotti Sep 17, 2025
2b47b69
reverts PageLayout changes
mperrotti Sep 17, 2025
207f275
reverts ActionList-related changes from SegmentedControl
mperrotti Sep 17, 2025
557f2e6
adds expect-error directive for something that's handled in a separat…
mperrotti Sep 17, 2025
58fc145
deal with failing tests
mperrotti Sep 18, 2025
2e6cbbf
reverts PageHeader subcomponent prop types export (moved to separate PR)
mperrotti Sep 18, 2025
9d7c05d
updates snapshots
mperrotti Sep 18, 2025
5d688d2
Merge branch 'main' of github.com:primer/react into mp/rm-box-and-sx-…
mperrotti Sep 24, 2025
5540417
rm dupe checkbox imports in styled-react
mperrotti Sep 24, 2025
5bcebe0
correct heading lineheight assertions
mperrotti Sep 24, 2025
7b1ef28
styled-react tweaks for int tests: exports LinkProps, converts Link t…
mperrotti Sep 24, 2025
c42c45b
Merge branch 'main' of github.com:primer/react into mp/rm-box-and-sx-…
mperrotti Sep 24, 2025
6ae78cc
moves styled-react Heading and Link to their own component files
mperrotti Sep 24, 2025
2b24955
reverts Heading changes - they're handled in a separate PR
mperrotti Sep 24, 2025
178e221
updates changelog
mperrotti Sep 24, 2025
bb77600
un-skip tests
mperrotti Sep 25, 2025
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/light-schools-wish.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': major
---

Removes `Box` component usage and `sx` prop from the `Link` component, Storybook stories, and a .figma.tsx file
2 changes: 1 addition & 1 deletion packages/react/src/Hidden/Hidden.examples.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export const PullRequestPage = () => (
<StateLabel status="pullOpened">Open</StateLabel>
<Hidden when={['narrow']}>
<Text sx={{fontSize: 1, color: 'fg.muted'}}>
<Link href="#" muted sx={{fontWeight: 'bold'}}>
<Link href="#" muted style={{fontWeight: 'var(--base-text-weight-semibold)'}}>
broccolinisoup
</Link>{' '}
wants to merge 3 commits into
Expand Down
5 changes: 0 additions & 5 deletions packages/react/src/Link/Link.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,6 @@
"name": "as",
"type": "React.ElementType",
"defaultValue": "\"a\""
},
{
"name": "sx",
"type": "SystemStyleObject",
"deprecated": true
}
],
"subcomponents": []
Expand Down
51 changes: 20 additions & 31 deletions packages/react/src/Link/Link.tsx
Original file line number Diff line number Diff line change
@@ -1,25 +1,28 @@
import {clsx} from 'clsx'
import React, {forwardRef, useEffect} from 'react'
import React, {useEffect, type ForwardedRef, type ElementRef} from 'react'
import {useRefObjectAsForwardedRef} from '../hooks'
import type {SxProp} from '../sx'
import classes from './Link.module.css'
import Box from '../Box'
import type {ComponentProps} from '../utils/types'
import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
import {type PolymorphicProps, fixedForwardRef} from '../utils/modern-polymorphic'

type StyledLinkProps = {
type StyledLinkProps<As extends React.ElementType = 'a'> = {
as?: As
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I can remove the as prop from these types since I added it to PolymorphicProps.

/** @deprecated use CSS modules to style hover color */
hoverColor?: string
muted?: boolean
/** @deprecated use `inline` to specify the type of link instead */
underline?: boolean
// Link inside a text block
inline?: boolean
} & SxProp
}

const Link = forwardRef(({as: Component = 'a', className, inline, underline, hoverColor, ...props}, forwardedRef) => {
const innerRef = React.useRef<HTMLAnchorElement>(null)
useRefObjectAsForwardedRef(forwardedRef, innerRef)
export const UnwrappedLink = <As extends React.ElementType = 'a'>(
props: PolymorphicProps<As, 'a', StyledLinkProps>,
ref: ForwardedRef<unknown>,
) => {
const {as: Component = 'a', className, inline, underline, hoverColor, ...restProps} = props
const innerRef = React.useRef<ElementRef<As>>(null)
useRefObjectAsForwardedRef(ref, innerRef)

if (__DEV__) {
/**
Expand All @@ -46,37 +49,23 @@ const Link = forwardRef(({as: Component = 'a', className, inline, underline, hov
}, [innerRef])
}

if (props.sx) {
return (
<Box
as={Component}
className={clsx(className, classes.Link)}
data-muted={props.muted}
data-inline={inline}
data-underline={underline}
data-hover-color={hoverColor}
{...props}
// @ts-ignore shh
ref={innerRef}
/>
)
}

return (
<Component
className={clsx(className, classes.Link)}
data-muted={props.muted}
data-muted={restProps.muted}
data-inline={inline}
data-underline={underline}
data-hover-color={hoverColor}
{...props}
// @ts-ignore shh
ref={innerRef}
{...restProps}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
ref={innerRef as any}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I can get rid of this cast if I just use fixForwardedRef directly instead of wrapping it below.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is old though. so i guess its an improvement to have it

/>
)
}) as PolymorphicForwardRefComponent<'a', StyledLinkProps>
}

const LinkComponent = fixedForwardRef(UnwrappedLink)

Link.displayName = 'Link'
const Link = Object.assign(LinkComponent, {displayName: 'Link'})

export type LinkProps = ComponentProps<typeof Link>
export default Link
11 changes: 0 additions & 11 deletions packages/react/src/Link/__tests__/Link.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ describe('Link', () => {
expect(container.firstChild).toHaveStyle('--fgColor-accent: #0969da')
})

it('respects the "sx" prop', () => {
const {container} = render(<Link sx={{fontStyle: 'italic'}} />)
expect(container.firstChild).toHaveStyle('font-style: italic')
})

it('applies button styles when rendering a button element', () => {
const {container} = render(<Link as="button" />)
expect((container.firstChild as Element).tagName).toBe('BUTTON')
Expand All @@ -33,12 +28,6 @@ describe('Link', () => {
expect(container.firstChild).toHaveAttribute('data-muted', 'true')
})

it('respects the "sx" prop when "muted" prop is also passed', () => {
const {container} = render(<Link muted sx={{color: 'fg.onEmphasis'}} />)
expect(container.firstChild).toHaveAttribute('data-muted', 'true')
expect(container.firstChild).toHaveStyle('color: rgb(89, 99, 110)')
})

it('logs a warning when trying to render invalid "as" prop', () => {
const consoleSpy = vi.spyOn(globalThis.console, 'error').mockImplementation(() => {})

Expand Down
11 changes: 11 additions & 0 deletions packages/react/src/Overlay/Overlay.features.stories.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,14 @@
right: var(--base-size-4);
top: var(--base-size-4);
}

.IssueLink {
display: block;
border: var(--borderWidth-thin) solid var(--borderColor-default);
padding: var(--base-size-8);
transition: background 0.2s;
}

.IssueLink:hover {
background-color: var(--bgColor-muted);
}
10 changes: 1 addition & 9 deletions packages/react/src/Overlay/Overlay.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -429,15 +429,7 @@ export const MemexIssueOverlay = ({role, open}: Args) => {
event.preventDefault()
setOverlayOpen(true)
}}
sx={{
display: 'block',
border: '1px solid',
borderColor: 'border.default',
p: 2,
':hover': {
backgroundColor: 'canvas.subtle',
},
}}
className={classes.IssueLink}
>
<IssueDraftIcon /> {title}
</Link>
Expand Down
8 changes: 1 addition & 7 deletions packages/react/src/Popover/Popover.figma.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,7 @@ figma.connect(Popover, 'https://www.figma.com/design/GCvY3Qv8czRgZgvl1dG6lp/Prim
example: ({caret, heading, body, action}) => (
<Popover caret={caret}>
<Popover.Content>
<Heading
sx={{
fontSize: 2,
}}
>
{heading}
</Heading>
<Heading style={{fontSize: 'var(--text-title-size-small)'}}>{heading}</Heading>
<Text as="p">{body}</Text>
{action}
</Popover.Content>
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/live-region/Announce.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import type React from 'react'
import {useEffect, useRef, useState, type ElementRef} from 'react'
import {useEffectOnce} from '../internal/hooks/useEffectOnce'
import {useEffectCallback} from '../internal/hooks/useEffectCallback'
import type {PolymorphicProps} from '../utils/polymorphic2'
import type {PolymorphicProps} from '../utils/modern-polymorphic'

export type AnnounceProps<As extends React.ElementType> = PolymorphicProps<
'div',
As,
'div',
{
/**
* Specify if the content of the element should be announced when this
Expand Down
16 changes: 4 additions & 12 deletions packages/react/src/live-region/AriaAlert.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import type React from 'react'
import {type ElementType} from 'react'
import {Announce} from './Announce'
import type {PolymorphicProps} from '../utils/polymorphic2'
import type {PolymorphicProps} from '../utils/modern-polymorphic'

export type AriaAlertProps<As extends ElementType> = PolymorphicProps<
'div',
As,
'div',
{
/**
* Specify if the content of the element should be announced when this
Expand All @@ -22,14 +22,6 @@ export type AriaAlertProps<As extends ElementType> = PolymorphicProps<
}
>

export function AriaAlert<As extends ElementType = 'div'>({
announceOnShow = true,
children,
...rest
}: AriaAlertProps<As>) {
return (
<Announce {...rest} announceOnShow={announceOnShow} politeness="assertive">
{children}
</Announce>
)
export function AriaAlert<As extends ElementType = 'div'>(props: AriaAlertProps<As>) {
return <Announce {...props} announceOnShow={props.announceOnShow ?? true} politeness="assertive" />
}
18 changes: 5 additions & 13 deletions packages/react/src/live-region/AriaStatus.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import type React from 'react'
import {type ElementType} from 'react'
import {Announce} from './Announce'
import type {PolymorphicProps} from '../utils/polymorphic2'
import type {PolymorphicProps} from '../utils/modern-polymorphic'

export type AriaStatusProps<As extends ElementType> = PolymorphicProps<
'div',
export type AriaStatusProps<As extends ElementType = 'div'> = PolymorphicProps<
As,
'div',
{
/**
* Specify if the content of the element should be announced when this
Expand All @@ -27,14 +27,6 @@ export type AriaStatusProps<As extends ElementType> = PolymorphicProps<
}
>

export function AriaStatus<As extends ElementType = 'div'>({
announceOnShow = false,
children,
...rest
}: AriaStatusProps<As>) {
return (
<Announce {...rest} announceOnShow={announceOnShow} politeness="polite">
{children}
</Announce>
)
export function AriaStatus<As extends ElementType = 'div'>(props: AriaStatusProps<As>) {
return <Announce {...props} announceOnShow={props.announceOnShow ?? false} politeness="polite" />
}
35 changes: 35 additions & 0 deletions packages/react/src/utils/modern-polymorphic.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Mostly taken from https://github.com/total-typescript/react-typescript-tutorial/blob/main/src/08-advanced-patterns/72-as-prop-with-forward-ref.solution.tsx
Copy link
Member

Choose a reason for hiding this comment

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

can you elaborate on the need for this over the regular polymorphic we've been using?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our current approach causes the following problems:

  • I don't think there's a good way to use ForwardRefComponent without casting (as ForwardRefComponent)
  • We can't access the prop types to automatically generate component API docs (project @adierkens is working on) when we cast with as ForwardRefComponent
  • The ForwardRefComponent type isn't portable, so I get an error when I try to import a component that uses ForwardRefComponent into styled-react
  • The DistributiveOmit (usually) handles type collisions better (for example: we specify an onClick, but the native HTML element type has a conflicting onClick)
  • I think the ForwardRefComponent approach is less type-safe, but I don't remember the specifics

@adierkens or somebody who is actually good at TypeScript can keep me correct on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All that said, I'm fine with going back to ForwardRefComponent for now 🙂

This just felt like a good opportunity to level-up our component types.


import {forwardRef} from 'react'
import type {ComponentPropsWithRef, ElementType} from 'react'

/**
* Distributive Omit utility type that works correctly with union types
*/
type DistributiveOmit<T, TOmitted extends PropertyKey> = T extends unknown ? Omit<T, TOmitted> : never

/**
* Fixed version of forwardRef that provides better type inference for polymorphic components
*/
// TODO: figure out how to change this type so we can set displayName
// like this: `ComponentName.displayName = 'DisplayName' instead of using workarounds
type FixedForwardRef = <T, P extends Record<string, unknown> = Record<string, unknown>>(
render: (props: P, ref: React.Ref<T>) => React.ReactNode,
) => (props: P & React.RefAttributes<T>) => React.ReactNode

/**
* Cast forwardRef to the fixed version with better type inference
*/
export const fixedForwardRef = forwardRef as FixedForwardRef

/**
* Simplified polymorphic props type that handles the common pattern of
* `DistributiveOmit<ComponentPropsWithRef<ElementType extends As ? DefaultElement : As>, 'as'>`
*/
export type PolymorphicProps<
TAs extends ElementType,
TDefaultElement extends ElementType = 'div',
Props extends Record<string, unknown> = Record<string, unknown>,
> = DistributiveOmit<ComponentPropsWithRef<ElementType extends TAs ? TDefaultElement : TAs> & Props, 'as'> & {
as?: TAs
}
23 changes: 0 additions & 23 deletions packages/react/src/utils/polymorphic2.ts

This file was deleted.

12 changes: 12 additions & 0 deletions packages/styled-react/src/components/Link.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import {Link as PrimerLink, type LinkProps as PrimerLinkProps} from '@primer/react'
import styled from 'styled-components'
import {sx, type SxProp} from '../sx'

type LinkProps = PrimerLinkProps & SxProp

const Link = styled(PrimerLink).withConfig<LinkProps>({
shouldForwardProp: prop => prop !== 'sx',
})`
${sx}
`
export {Link, type LinkProps}
2 changes: 1 addition & 1 deletion packages/styled-react/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ export {FormControl} from '@primer/react'
export {Heading} from '@primer/react'
export {IconButton} from '@primer/react'
export {Label} from '@primer/react'
export {Link} from '@primer/react'
export {NavList} from '@primer/react'
export {Overlay} from '@primer/react'
export {PageLayout} from '@primer/react'
Expand All @@ -39,6 +38,7 @@ export {Checkbox, type CheckboxProps} from './components/Checkbox'
export {CounterLabel, type CounterLabelProps} from './components/CounterLabel'
export {Flash} from './components/Flash'
export {Header, type HeaderProps} from './components/Header'
export {Link, type LinkProps} from './components/Link'
export {LinkButton, type LinkButtonProps} from './components/LinkButton'
export {
PageHeader,
Expand Down
Loading