Skip to content

fix: PageHeader.ParentLink forwards rest props to support polymorphic as routing#7978

Open
Copilot wants to merge 2 commits into
mainfrom
copilot/fix-pageheader-parentlink-props
Open

fix: PageHeader.ParentLink forwards rest props to support polymorphic as routing#7978
Copilot wants to merge 2 commits into
mainfrom
copilot/fix-pageheader-parentlink-props

Conversation

Copilot AI commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

PageHeader.ParentLink destructured a fixed prop set and never spread the remainder onto the underlying Link, silently dropping any prop not in the allowlist — including to used by React Router. This broke the standard polymorphic pattern that works on Button, Link, and NavList.Item.

Changes

  • PageHeader.tsx — Add ...rest to ParentLink's destructuring and spread it onto <Link> after the explicit props, matching the pattern used by other polymorphic Primer components
  • PageHeader.test.tsx — Add two tests: one verifying arbitrary props (data-testid) reach the rendered element; one verifying a custom as component receives forwarded props (to)
  • .changeset/pageheader-parentlink-rest-props.md — Patch changeset
// Now works as expected — `to` reaches the RouterLink
<PageHeader.ParentLink as={RouterLink} to="/somewhere">
  Back
</PageHeader.ParentLink>

Changelog

New

  • None

Changed

  • PageHeader.ParentLink now forwards unknown/rest props to the underlying element, consistent with other polymorphic Primer components

Removed

  • None

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

Two new unit tests cover the fix:

  1. data-testid forwarded via default as="a"
  2. Custom as component (simulating a router Link) receives to and renders the correct href

Merge checklist

…c as routing

Co-authored-by: adierkens <13004162+adierkens@users.noreply.github.com>
@changeset-bot

changeset-bot Bot commented Jun 11, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 3ad321c

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 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

Copilot AI changed the title [WIP] Fix PageHeader.ParentLink to forward all props fix: PageHeader.ParentLink forwards rest props to support polymorphic as routing Jun 11, 2026
Copilot AI requested a review from adierkens June 11, 2026 21:38
@github-actions github-actions Bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Jun 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

To publish a canary release for integration testing, apply the Canary Release label to this PR.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes PageHeader.ParentLink prop forwarding so it behaves consistently with other polymorphic Primer components (e.g. allowing React Router-style to when using a custom as component), instead of silently dropping unknown props.

Changes:

  • Update PageHeader.ParentLink to collect ...rest props and spread them onto the underlying Link.
  • Add unit tests verifying arbitrary props (e.g. data-testid) and custom as props (e.g. to) are forwarded correctly.
  • Add a patch changeset for @primer/react.
Show a summary per file
File Description
packages/react/src/PageHeader/PageHeader.tsx Forwards remaining props from ParentLink onto Link, enabling polymorphic routing props like to.
packages/react/src/PageHeader/PageHeader.test.tsx Adds regression tests ensuring rest props and custom as props are forwarded to the rendered link.
.changeset/pageheader-parentlink-rest-props.md Declares a patch release note for the ParentLink prop-forwarding fix.

Copilot's findings

  • Files reviewed: 3/3 changed files
  • Comments generated: 0

@primer-integration

Copy link
Copy Markdown

Integration test results from github/github-ui PR:

Passed  CI   Passed
Passed  VRT   Passed
Passed  Projects   Passed

All checks passed!

@adierkens adierkens marked this pull request as ready for review June 12, 2026 13:11
@adierkens adierkens requested a review from a team as a code owner June 12, 2026 13:11
@adierkens adierkens enabled auto-merge June 12, 2026 14:01

@joshblack joshblack left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@adierkens just had one comment around the spread usage, let me know if it doesn't apply here 👀 LGTM otherwise

data-component="PageHeader.ParentLink"
{...getHiddenDataAttributes(hidden)}
href={href}
{...rest}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Only suggestion is to have this be added first and then merge things afterwards to avoid issues where this overrides things unintentionally 👀

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was a little less worried about it since it's typed as a generic a and not a Link component (so the component types from Link shouldn't show up). We could also put more into the rest spread (like href) to simplify too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Canary Release Apply this label when you want CI to create a canary release of the current PR integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PageHeader.ParentLink drops props (e.g. to) when using a polymorphic as, breaking client-side routing

5 participants