feat(app-shell): add render prop to AppShellRailItem

[dennisofficial]

Closes ENG-379.

The problem

AppShellRailItem hard-renders a <button>. Consumers that need the rail item to be a navigation link have to wrap it in an anchor:

<Link href={href}>
  <AppShellRailItem isActive={isActive} icon={icon} label={label} />
</Link>

which produces invalid HTML — verified in jsdom:

<a href="/org_1/products/compliance"><button data-slot="app-shell-rail-item" …></button></a>

An <a> may not contain interactive content. It yields two tab stops and inconsistent screen-reader announcement. The workaround consumers reach for instead — onClick={() => router.push(href)} — is valid markup but loses cmd-click, middle-click and prefetch, which is exactly what cross-application navigation needs (ENG-350's acceptance criterion "no bare anchors or plain Next Link across the boundary" is unachievable while the rail hard-renders a button).

The change

AppShellRailItem takes an optional render prop:

<AppShellRailItem
  isActive={isActive}
  icon={<Icon />}
  label={label}
  render={<Link href={href} />}
/>

After:

<a href="/org_1/products/compliance" data-slot="app-shell-rail-item" class="flex size-10 …" aria-label="Compliance" id="base-ui-_r_5_"><span class="size-5 [&>svg]:size-5">…</span></a>

One element. No nested button, one tab stop, the rail item's classes / data attributes / aria-label all land on the anchor.

Why render and not asChild

render is the convention already established throughout this codebase — it is Base UI's API, and sidebar.tsx, badge.tsx, breadcrumb.tsx, item.tsx and button-group.tsx all expose it via useRender. AppShellRailItem itself already consumes it (<TooltipTrigger render={button} />). Introducing asChild would mean two competing composition idioms in one library. The props type is copied verbatim from SidebarMenuButton:

Omit<useRender.ComponentProps<'button'>, 'className'> & Omit<React.ComponentProps<'button'>, 'className'> & { … }

Using useRender rather than a hand-rolled React.cloneElement also gets the render-function overload ((props, state) => ReactElement) and Base UI's ref merging for free.

This is additive and backwards compatible

Omitting render produces byte-identical markup to main. Not asserted — measured. I rendered a rail containing an active item, an inactive item, an item with no isActive, and an item with id / onClick / disabled / title passthrough; dumped container.innerHTML; swapped in git show HEAD:…/app-shell.tsx; dumped again. The two strings are equal, including attribute order.

Two regressions that a reading of the diff would have missed, and which that check caught:

1. Base UI's renderTag injects type="button" for the default button tag. The rail item has always rendered a bare <button>. Adding type="button" would change submit behaviour inside a form. Suppressed by passing type: undefined in the default props — a caller-supplied type still wins.
2. Base UI's default state→attribute mapping emits data-active="" when true and omits the attribute entirely when false. Today it is data-active="false". Any downstream [data-active] selector would have silently stopped matching. Fixed with an explicit stateAttributesMapping that emits the "true" / "false" string, and null (attribute absent) when isActive is undefined.

Also preserved: the RailIndicatorContext registration (registerItem / setActiveId), the tooltip wrapping when label is set, and data-slot="app-shell-rail-item". CS-773 is preserved — React.useId() is untouched, and there is a test asserting all four rail-item copies (desktop rail plus the always-mounted mobile drawer) get unique ids.

One deliberate behavioural difference

The internal ref changed from useRef<HTMLButtonElement> to useRef<HTMLElement>, since render can produce an anchor. As a side effect, useRender now merges a caller-supplied ref with the internal one, whereas previously a caller passing ref would silently clobber the rail's registerItem registration and break the active-indicator animation for that item. This is a fix, but it is a behavioural change in that one edge case and is called out here so nobody later mistakes it for an accident.

Tests and story

• apps/storybook/tests/AppShell.test.tsx (new) — 8 tests. Default renders a <button> with no type attribute; render={<a href="…" />} renders a single anchor with no nested <button>; tooltip and aria-label survive both modes; data-active is "true" / "false" / absent as appropriate; props forward in both modes; rail item ids are unique across the desktop and drawer copies. All pass, and they fail against main as they should.
• apps/storybook/stories/AppShell.stories.tsx — new WithRailLinks story showing a rail whose items are anchors.

Second commit: prettier config (103 files) — please read

80647ca is separate from the API change so the render work stays reviewable on its own. It touches 103 files. None of it is hand-written.

The repo had no prettier config at all, so pnpm lint (prettier --check) fell back to prettier's defaults — double quotes at 80 columns — while the source is single-quoted at 100. Essentially every file failed, and the CI Lint job has been red on every recent run, including on main and on every dependabot PR.

The two settings that differ from prettier's defaults were inferred by measuring the existing source rather than guessed:

printWidth	files still failing in design-system
80 (default)	67
90	58
100	35
120	62

semi, trailingComma: all, arrowParens and jsxSingleQuote already match prettier's defaults and are left unset. Lockfiles are added to .prettierignore so the formatter does not rewrite pnpm-lock.yaml.

Only packages/design-system and apps/mcp have prettier-based lint scripts, so strictly 43 files needed reformatting to turn CI green. I formatted all 103 instead so the repo is internally consistent and the existing root pnpm format script is a no-op — otherwise the next person to run it produces a surprise 60-file diff. Say the word if you'd rather I narrow it to the 43.

One non-formatting change rides along in that commit: apps/example lints with eslint, not prettier, and had a single react/no-unescaped-entities error (an apostrophe in the string "That vendor doesn't exist"). It was the last error between pnpm lint and green, so it is escaped as '.

Two more commits, both needed to get CI to actually run

Neither is part of the API change; both are called out here rather than left for a reviewer to find.

94cb82c — ci: stop pinning pnpm in both the workflow and packageManager. Every CI job on this repo was dying after six seconds at the setup step:

Error: Multiple versions of pnpm specified:
  - version 9 in the GitHub Action config with the key "version"
  - version pnpm@9.0.0+sha512... in the package.json with the key "packageManager"

pnpm/action-setup refuses to guess between the two. Dropping the workflow pin lets it read packageManager, which is the version used locally. Until this landed, no CI job on this repo had run its actual command in months — including on main and on every dependabot PR.

2a35d55 — fix(ai-chat): guard the navigator access so prerendering works on node 20. With CI setup unblocked, the Build job ran for the first time and failed:

ReferenceError: navigator is not defined
Export encountered an error on /(app)/design/loading/page

AIChatTrigger read navigator?.platform during render. Optional chaining does not protect an undeclared identifier — it throws ReferenceError rather than yielding undefined. Node 21 added a global navigator, which is why this never reproduced locally; CI runs Node 20. I reproduced it under Node 20 against main, and confirmed the guard fixes it. Pre-existing bug, unrelated to render, one line.

Fifth commit: the six failing tests

aa5cca5 — fix(spinner): expose the loading state as a live region

Four of the six failures shared one root cause, and it was a real accessibility bug rather
than a stale test.

Spinner passed role="status" to a Carbon icon. getAttributes in
@carbon/icon-helpers does iconAttributes.role = 'img' unconditionally whenever an
aria-label is present, so the role was silently discarded:

Unable to find an accessible element with the role "status" and name "Loading"
  img:
  Name "Loading":
  <svg aria-label="Loading" class="size-4 shrink-0 animate-spin" />

A loading spinner announced as a static image never tells assistive technology that state
changed. The tests were right; the component was wrong. The role now lives on a wrapper with
display: contents, which keeps the icon participating directly in its parent's flex layout.
That fixed both tests/Button.test.tsx > shows spinner when loading and
stories/Button.stories.tsx > Loading.

The remaining three were genuinely stale assertions, against components redesigned without
their tests being updated:

Assertion	Actual
Badge secondary → bg-secondary	bg-muted
Badge outline → border-border	border-border/50
Button size="lg" → h-9	h-8

h-9 appears zero times in button.tsx on main — that test asserted against a class
the source never had.

Dialog.stories.tsx > With Textarea was an animation race. DialogContent fades in
(data-open:fade-in-0, duration-100) and jest-dom counts zero opacity as not visible, so
the assertion sampled the textarea mid-fade. Wrapped in waitFor so it outlasts the
animation rather than sampling it once.

Verification

Every job green — the first time this repo's CI has passed, and the first time most of these
jobs have run their command at all rather than dying at setup.

Check	Result	Previously on main
Lint	pass	never ran (setup failure)
Type Check	pass	never ran
Build	pass	never ran; then navigator ReferenceError
Unit Tests	pass — 138/138	never ran; then 4 failures
Storybook Tests	pass — 361/361	never ran; then 2 failures
CodeQL	pass

The render API change itself remains verified by the byte-identical markup comparison
described above, re-run after prettier reformatted app-shell.tsx.

Version

packages/design-system/package.json 1.1.16 → 1.1.20. The field had drifted behind npm's published 1.1.19; app-shell.tsx is byte-identical between git HEAD and the published 1.1.19, so no unreleased source is being skipped. Not published — publishing is manual.

[linear-code]

ENG-379

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
design-system-storybook	Ready	Preview	Aug 18, 2026 7:56pm

[cubic-dev-ai]

2 issues found across 4 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/design-system/src/components/atoms/spinner.tsx">

<violation number="1" location="packages/design-system/src/components/atoms/spinner.tsx:16">
P2: The role="status" and aria-label="Loading" live on a <span> using display:contents (className="contents"), so they are unlikely to be exposed to assistive technology. Browsers implementing display:contents remove the element from the accessibility tree (documented MDN accessibility concern, active in most engines), so the span's role/aria attributes are ignored and its only child Renew is aria-hidden. This defeats the change's stated purpose of exposing the loading state as a live region. Put role="status" on an element that produces a real box instead of a display:contents wrapper.</violation>

<violation number="2" location="packages/design-system/src/components/atoms/spinner.tsx:16">
P2: The `role="status"` live region is placed on a `display: contents` span whose only child (`Renew`) is `aria-hidden`. In browsers that strip `display: contents` elements from the accessibility tree (Safari, and Chrome prior to the 115 fix), the role and aria-label are dropped while the icon is hidden, so the spinner is never announced — defeating the fix's purpose. Keep the icon as a direct participant in the flex layout, but expose the live region through a visually-hidden (sr-only) status element instead of `display: contents`.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: The role="status" and aria-label="Loading" live on a using display:contents (className="contents"), so they are unlikely to be exposed to assistive technology. Browsers implementing display:contents remove the element from the accessibility tree (documented MDN accessibility concern, active in most engines), so the span's role/aria attributes are ignored and its only child Renew is aria-hidden. This defeats the change's stated purpose of exposing the loading state as a live region. Put role="status" on an element that produces a real box instead of a display:contents wrapper.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/design-system/src/components/atoms/spinner.tsx, line 16:

<comment>The role="status" and aria-label="Loading" live on a <span> using display:contents (className="contents"), so they are unlikely to be exposed to assistive technology. Browsers implementing display:contents remove the element from the accessibility tree (documented MDN accessibility concern, active in most engines), so the span's role/aria attributes are ignored and its only child Renew is aria-hidden. This defeats the change's stated purpose of exposing the loading state as a live region. Put role="status" on an element that produces a real box instead of a display:contents wrapper.</comment>

<file context>
@@ -1,8 +1,21 @@
 function Spinner({ ...props }: Omit<React.ComponentProps<typeof Renew>, 'className'>) {
   return (
-    <Renew role="status" aria-label="Loading" className="size-4 shrink-0 animate-spin" {...props} />
+    <span role="status" aria-label="Loading" className="contents">
+      <Renew aria-hidden className="size-4 shrink-0 animate-spin" {...props} />
+    </span>
</file context>

Suggested change

	<span role="status" aria-label="Loading" className="contents">
	<span role="status" aria-label="Loading">

[cubic-dev-ai]

P2: The role="status" live region is placed on a display: contents span whose only child (Renew) is aria-hidden. In browsers that strip display: contents elements from the accessibility tree (Safari, and Chrome prior to the 115 fix), the role and aria-label are dropped while the icon is hidden, so the spinner is never announced — defeating the fix's purpose. Keep the icon as a direct participant in the flex layout, but expose the live region through a visually-hidden (sr-only) status element instead of display: contents.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/design-system/src/components/atoms/spinner.tsx, line 16:

<comment>The `role="status"` live region is placed on a `display: contents` span whose only child (`Renew`) is `aria-hidden`. In browsers that strip `display: contents` elements from the accessibility tree (Safari, and Chrome prior to the 115 fix), the role and aria-label are dropped while the icon is hidden, so the spinner is never announced — defeating the fix's purpose. Keep the icon as a direct participant in the flex layout, but expose the live region through a visually-hidden (sr-only) status element instead of `display: contents`.</comment>

<file context>
@@ -1,8 +1,21 @@
 function Spinner({ ...props }: Omit<React.ComponentProps<typeof Renew>, 'className'>) {
   return (
-    <Renew role="status" aria-label="Loading" className="size-4 shrink-0 animate-spin" {...props} />
+    <span role="status" aria-label="Loading" className="contents">
+      <Renew aria-hidden className="size-4 shrink-0 animate-spin" {...props} />
+    </span>
</file context>

Suggested change

	<span role="status" aria-label="Loading" className="contents">
	<Renew aria-hidden className="size-4 shrink-0 animate-spin" {...props} />
	</span>
	);
	function Spinner({ ...props }: Omit<React.ComponentProps<typeof Renew>, 'className'>) {
	return (
	<>
	<Renew aria-hidden className="size-4 shrink-0 animate-spin" {...props} />
	<span role="status" aria-label="Loading" className="sr-only" />
	</>
	);
	}

[dennisofficial]

Relationship to comp-v3, and a scope correction

This PR was opened while building @trycompai/shell in trycompai/comp-v3#44 (Linear
ENG-337), where the product rail wraps AppShellRailItem in a next/link and produces
invalid markup:

<a href="/org_1/products/compliance"><button data-slot="app-shell-rail-item">…</button></a>

That comp-v3 PR has since merged, documenting the nesting as a known issue. It is not
blocked on this one.

Correcting the framing: comp-v3 is a private, in-progress re-architecture, and it should
not be the thing driving changes to a package that five repos and ~5k monthly npm installs
depend on. So this PR should stand or fall on whether it is right for the design system on
its own terms — not on whether comp-v3 wants it.

On that basis, most of what is here is independently justified and has nothing to do with
comp-v3:

• CI had not run in months. pnpm/action-setup refuses to start when both with.version
  and packageManager are set, so every job on main and on every Dependabot PR died at
  setup in ~6s. Nothing was being checked.
• ReferenceError: navigator is not defined on any Node 20 prerender — optional chaining
  does not protect an undeclared identifier. Node 21 added a global navigator, so it never
  reproduced locally.
• Every loading spinner was invisible to screen readers. Spinner set role="status",
  but @carbon/icon-helpers overwrites it with role="img" whenever an aria-label is
  present. Two tests had been failing on this and nobody could see them.
• Three stale assertions against components redesigned without their tests updated.

Those benefit comp, comp-v2, comp-private and gtm-dashboard equally.

The render prop is the one piece comp-v3 asked for. It is still defensible on its own —
apps/app in comp-v2 ships the same invalid nesting today, via
ShellRailNavItem.tsx — but it is the part to scrutinise under the rule above, and the part
to drop if you would rather this PR carried only the fixes the design system needs for
itself.