1573 accessibility request primer react link enforce what as can be set to with a runtime validation rule 1572

.changeset/tidy-hornets-act.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+warn user if link `as` prop is not <a> or <button>

src/Link.tsx

@@ -1,8 +1,11 @@
+import React, {forwardRef, useEffect} from 'react'
 import styled from 'styled-components'
 import {system} from 'styled-system'
 import {get} from './constants'
+import {useRefObjectAsForwardedRef} from './hooks'
 import sx, {SxProp} from './sx'
 import {ComponentProps} from './utils/types'
+import {ForwardRefComponent as PolymorphicForwardRefComponent} from './utils/polymorphic'
 
 type StyledLinkProps = {
   hoverColor?: string
@@ -17,7 +20,7 @@ const hoverColor = system({
   },
 })
 
-const Link = styled.a<StyledLinkProps>`
+const StyledLink = styled.a<StyledLinkProps>`
   color: ${props => (props.muted ? get('colors.fg.muted')(props) : get('colors.accent.fg')(props))};
   text-decoration: ${props => (props.underline ? 'underline' : 'none')};
   &:hover {
@@ -38,5 +41,45 @@ const Link = styled.a<StyledLinkProps>`
   ${sx};
 `
 
+const Link = forwardRef(({as: Component = 'a', ...props}, forwardedRef) => {
+  const innerRef = React.useRef<HTMLAnchorElement>(null)
+  useRefObjectAsForwardedRef(forwardedRef, innerRef)
+
+  if (__DEV__) {
+    /**
+     * The Linter yells because it thinks this conditionally calls an effect,
+     * but since this is a compile-time flag and not a runtime conditional
+     * this is safe, and ensures the entire effect is kept out of prod builds
+     * shaving precious bytes from the output, and avoiding mounting a noop effect
+     */
+    // eslint-disable-next-line react-hooks/rules-of-hooks
+    useEffect(() => {
+      if (
+        innerRef.current &&
+        !(innerRef.current instanceof HTMLButtonElement) &&
+        !(innerRef.current instanceof HTMLAnchorElement)
+      ) {
+        // eslint-disable-next-line no-console
+        console.error(
+          'Error: Found `Link` component that renders an inaccessible element',
+          innerRef.current,
+          'Please ensure `Link` always renders as <a> or <button>',
+        )
+      }
+    }, [innerRef])
+  }
+
+  return (
+    <StyledLink
+      as={Component}
+      {...props}
+      // @ts-ignore shh
+      ref={innerRef}
+    />
+  )
+}) as PolymorphicForwardRefComponent<'a', StyledLinkProps>
+
+Link.displayName = 'Link'
+
 export type LinkProps = ComponentProps<typeof Link>
 export default Link

src/PageHeader/PageHeader.tsx

@@ -4,7 +4,8 @@ import {useResponsiveValue, ResponsiveValue} from '../hooks/useResponsiveValue'
 import {SxProp, merge, BetterSystemStyleObject} from '../sx'
 import Heading from '../Heading'
 import {ArrowLeftIcon} from '@primer/octicons-react'
-import Link from '../Link'
+import Link, {LinkProps as BaseLinkProps} from '../Link'
+
 import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
 import {getBreakpointDeclarations} from '../utils/getBreakpointDeclarations'
 const REGION_ORDER = {
@@ -92,8 +93,8 @@ const ContextArea: React.FC<React.PropsWithChildren<PageHeaderProps>> = ({
   return <Box sx={merge<BetterSystemStyleObject>(contentNavStyles, sx)}>{children}</Box>
 }
 type LinkProps = Pick<
-  React.AnchorHTMLAttributes<HTMLAnchorElement>,
-  'download' | 'href' | 'hrefLang' | 'media' | 'ping' | 'rel' | 'target' | 'type' | 'referrerPolicy'
+  React.AnchorHTMLAttributes<HTMLAnchorElement> & BaseLinkProps,
+  'download' | 'href' | 'hrefLang' | 'media' | 'ping' | 'rel' | 'target' | 'type' | 'referrerPolicy' | 'as'
 >
 export type ParentLinkProps = React.PropsWithChildren<PageHeaderProps & LinkProps>
 

src/__tests__/Link.test.tsx

@@ -29,7 +29,6 @@ describe('Link', () => {
 
   it('respects the "sx" prop', () => {
     expect(render(<Link sx={{fontStyle: 'italic'}} />)).toHaveStyleRule('font-style', 'italic')
-    expect(render(<Link as="i" sx={{fontStyle: 'normal'}} />)).toHaveStyleRule('font-style', 'normal')
   })
 
   it('applies button styles when rendering a button element', () => {
@@ -43,4 +42,13 @@ describe('Link', () => {
   it('respects the  "sx" prop when "muted" prop is also passed', () => {
     expect(render(<Link muted sx={{color: 'fg.onEmphasis'}} />)).toMatchSnapshot()
   })
+
+  it('logs a warning when trying to render invalid "as" prop', () => {
+    const consoleSpy = jest.spyOn(global.console, 'error').mockImplementation()
+
+    HTMLRender(<Link as="i" />)
+    expect(consoleSpy).toHaveBeenCalled()
+
+    consoleSpy.mockRestore()
+  })
 })

src/__tests__/__snapshots__/SideNav.test.tsx.snap

@@ -5,11 +5,6 @@ exports[`SideNav SideNav.Link renders consistently 1`] = `
   color: #0969da;
   -webkit-text-decoration: none;
   text-decoration: none;
-  position: relative;
-  display: block;
-  width: 100%;
-  text-align: left;
-  font-size: 14px;
 }
 
 .c0:hover {
@@ -34,29 +29,37 @@ exports[`SideNav SideNav.Link renders consistently 1`] = `
   appearance: none;
 }
 
-.c0 > .c1 {
+.c1 {
+  position: relative;
+  display: block;
+  width: 100%;
+  text-align: left;
+  font-size: 14px;
+}
+
+.c1 > .c2 {
   border-bottom: none;
 }
 
-.c1.variant-normal > .c0 {
+.c2.variant-normal > .c1 {
   color: #24292f;
   padding: 16px;
   border: 0;
   border-top: 1px solid hsla(210,18%,87%,1);
 }
 
-.c1.variant-normal > .c0:first-child {
+.c2.variant-normal > .c1:first-child {
   border-top: 0;
   border-top-right-radius: 6px;
   border-top-left-radius: 6px;
 }
 
-.c1.variant-normal > .c0:last-child {
+.c2.variant-normal > .c1:last-child {
   border-bottom-right-radius: 6px;
   border-bottom-left-radius: 6px;
 }
 
-.c1.variant-normal > .c0::before {
+.c2.variant-normal > .c1::before {
   position: absolute;
   top: 0;
   bottom: 0;
@@ -67,57 +70,57 @@ exports[`SideNav SideNav.Link renders consistently 1`] = `
   content: '';
 }
 
-.c1.variant-normal > .c0:hover {
+.c2.variant-normal > .c1:hover {
   background-color: rgba(234,238,242,0.5);
   -webkit-text-decoration: none;
   text-decoration: none;
 }
 
-.c1.variant-normal > .c0:focus {
+.c2.variant-normal > .c1:focus {
   background-color: rgba(234,238,242,0.5);
   -webkit-text-decoration: none;
   text-decoration: none;
   outline: solid 2px #0969da;
   z-index: 1;
 }
 
-.c1.variant-normal > .c0[aria-current='page'],
-.c1.variant-normal > .c0[aria-selected='true'] {
+.c2.variant-normal > .c1[aria-current='page'],
+.c2.variant-normal > .c1[aria-selected='true'] {
   background-color: #ffffff;
 }
 
-.c1.variant-normal > .c0[aria-current='page']::before,
-.c1.variant-normal > .c0[aria-selected='true']::before {
+.c2.variant-normal > .c1[aria-current='page']::before,
+.c2.variant-normal > .c1[aria-selected='true']::before {
   background-color: #fd8c73;
 }
 
-.c1.variant-lightweight > .c0 {
+.c2.variant-lightweight > .c1 {
   padding: 4px 0;
   color: #0969da;
 }
 
-.c1.variant-lightweight > .c0:hover {
+.c2.variant-lightweight > .c1:hover {
   color: #24292f;
   -webkit-text-decoration: none;
   text-decoration: none;
 }
 
-.c1.variant-lightweight > .c0:focus {
+.c2.variant-lightweight > .c1:focus {
   color: #24292f;
   -webkit-text-decoration: none;
   text-decoration: none;
   outline: solid 1px #0969da;
   z-index: 1;
 }
 
-.c1.variant-lightweight > .c0[aria-current='page'],
-.c1.variant-lightweight > .c0[aria-selected='true'] {
+.c2.variant-lightweight > .c1[aria-current='page'],
+.c2.variant-lightweight > .c1[aria-selected='true'] {
   color: #24292f;
   font-weight: 500;
 }
 
 <a
-  className="c0"
+  className="c0 c1"
 />
 `;
 

src/stories/Link.stories.tsx

@@ -0,0 +1,22 @@
+import Link from '../Link'
+import {Meta} from '@storybook/react'
+import React from 'react'
+import {ThemeProvider} from '..'
+
+const meta: Meta = {
+  title: 'Components/Link',
+  component: Link,
+  decorators: [
+    (Story: React.ComponentType<React.PropsWithChildren<unknown>>): JSX.Element => (
+      <ThemeProvider>
+        <Story />
+      </ThemeProvider>
+    ),
+  ],
+}
+export default meta
+
+export function LinkStory(): JSX.Element {
+  return <Link as="i">{`Link with <i> as prop`}</Link>
+}
+LinkStory.storyName = 'Link'