From e63bf3111699126d54363193d39f3143573ba35b Mon Sep 17 00:00:00 2001 From: Lukas Oppermann Date: Tue, 11 Nov 2025 09:35:21 +0100 Subject: [PATCH 01/10] update type for branchname --- packages/react/src/BranchName/BranchName.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/react/src/BranchName/BranchName.tsx b/packages/react/src/BranchName/BranchName.tsx index aeb7bf4ff20..1c0d45940a0 100644 --- a/packages/react/src/BranchName/BranchName.tsx +++ b/packages/react/src/BranchName/BranchName.tsx @@ -4,16 +4,17 @@ import {clsx} from 'clsx' import classes from './BranchName.module.css' import {fixedForwardRef, type PolymorphicProps} from '../utils/modern-polymorphic' -export type BranchNameProps = PolymorphicProps< +export type BranchNameProps = PolymorphicProps< As, 'a', { className?: string } -> +> & + (As extends 'a' ? {href: string} : {href?: never}) // eslint-disable-next-line @typescript-eslint/no-explicit-any -function BranchName(props: BranchNameProps, ref: ForwardedRef) { +function BranchName(props: BranchNameProps, ref: ForwardedRef) { const {as: Component = 'a', className, children, ...rest} = props return ( From 2f5cdccb73c9d371bb07b726e9067cc6e51efe67 Mon Sep 17 00:00:00 2001 From: Lukas Oppermann Date: Tue, 11 Nov 2025 09:40:46 +0100 Subject: [PATCH 02/10] add test --- .../BranchName/__tests__/BranchName.test.tsx | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/react/src/BranchName/__tests__/BranchName.test.tsx b/packages/react/src/BranchName/__tests__/BranchName.test.tsx index a66bcdcf4bf..c72893779f7 100644 --- a/packages/react/src/BranchName/__tests__/BranchName.test.tsx +++ b/packages/react/src/BranchName/__tests__/BranchName.test.tsx @@ -4,12 +4,30 @@ import {describe, expect, it} from 'vitest' describe('BranchName', () => { it('renders an by default', () => { - const {container} = HTMLRender() + const {container} = HTMLRender() expect(container.firstChild?.nodeName).toEqual('A') }) it('should support `className` on the outermost element', () => { - const Element = () => + const Element = () => expect(HTMLRender().container.firstChild).toHaveClass('test-class-name') }) + + it('requires href when no as property is provided', () => { + // @ts-expect-error - href is required when as property is not provided + const Element = () => + expect(HTMLRender().container.firstChild?.nodeName).toEqual('A') + }) + + it('requires href when as="a"', () => { + // @ts-expect-error - href is required when as="a" + const Element = () => + expect(HTMLRender().container.firstChild?.nodeName).toEqual('A') + }) + + it('does not allow href when as="span"', () => { + // @ts-expect-error - href is required when as="a" + const Element = () => + expect(HTMLRender().container.firstChild?.nodeName).toEqual('SPAN') + }) }) From 3dd042135cc7dde58bfcdea86721a667fd3a0407 Mon Sep 17 00:00:00 2001 From: Lukas Oppermann Date: Tue, 11 Nov 2025 09:42:38 +0100 Subject: [PATCH 03/10] added changeset --- .changeset/plenty-cloths-dance.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/plenty-cloths-dance.md diff --git a/.changeset/plenty-cloths-dance.md b/.changeset/plenty-cloths-dance.md new file mode 100644 index 00000000000..341f82a06aa --- /dev/null +++ b/.changeset/plenty-cloths-dance.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Strictier type for BranchName: as="a" without href and neither as nor href are not allowed anymore. From b7c5d58f3a9209de03dd5e5db59a997af7d06a97 Mon Sep 17 00:00:00 2001 From: Lukas Oppermann Date: Tue, 11 Nov 2025 11:35:39 +0100 Subject: [PATCH 04/10] trying to fix issue with no props --- .../BranchName/BranchName.features.stories.tsx | 2 ++ packages/react/src/BranchName/BranchName.tsx | 17 ++++++++++++----- .../BranchName/__tests__/BranchName.test.tsx | 7 +++---- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/packages/react/src/BranchName/BranchName.features.stories.tsx b/packages/react/src/BranchName/BranchName.features.stories.tsx index 3459b60bfa1..21c229b519c 100644 --- a/packages/react/src/BranchName/BranchName.features.stories.tsx +++ b/packages/react/src/BranchName/BranchName.features.stories.tsx @@ -19,3 +19,5 @@ export const WithBranchIcon = () => ( ) export const NotALink = () => branch_name_as_span + +export const NoProps = () => branch_name_no_props diff --git a/packages/react/src/BranchName/BranchName.tsx b/packages/react/src/BranchName/BranchName.tsx index 1c0d45940a0..34a4853c156 100644 --- a/packages/react/src/BranchName/BranchName.tsx +++ b/packages/react/src/BranchName/BranchName.tsx @@ -4,7 +4,7 @@ import {clsx} from 'clsx' import classes from './BranchName.module.css' import {fixedForwardRef, type PolymorphicProps} from '../utils/modern-polymorphic' -export type BranchNameProps = PolymorphicProps< +export type BranchNameProps = PolymorphicProps< As, 'a', { @@ -14,12 +14,19 @@ export type BranchNameProps = PolymorphicPro (As extends 'a' ? {href: string} : {href?: never}) // eslint-disable-next-line @typescript-eslint/no-explicit-any -function BranchName(props: BranchNameProps, ref: ForwardedRef) { - const {as: Component = 'a', className, children, ...rest} = props +function BranchName(props: BranchNameProps, ref: ForwardedRef) { + const {as: Component, className, children, ...rest} = props + + let InferredComponent = (Component || 'span') as As + + if (!Component && 'href' in props) { + InferredComponent = 'a' as As + } + return ( - + {children} - + ) } diff --git a/packages/react/src/BranchName/__tests__/BranchName.test.tsx b/packages/react/src/BranchName/__tests__/BranchName.test.tsx index c72893779f7..204aa1066d3 100644 --- a/packages/react/src/BranchName/__tests__/BranchName.test.tsx +++ b/packages/react/src/BranchName/__tests__/BranchName.test.tsx @@ -13,10 +13,9 @@ describe('BranchName', () => { expect(HTMLRender().container.firstChild).toHaveClass('test-class-name') }) - it('requires href when no as property is provided', () => { - // @ts-expect-error - href is required when as property is not provided - const Element = () => - expect(HTMLRender().container.firstChild?.nodeName).toEqual('A') + it('renders an by default', () => { + const {container} = HTMLRender() + expect(container.firstChild?.nodeName).toEqual('SPAN') }) it('requires href when as="a"', () => { From b5a6f4242a61710c3d8d607c8134acc1084bdbd0 Mon Sep 17 00:00:00 2001 From: Lukas Oppermann Date: Tue, 11 Nov 2025 11:41:08 +0100 Subject: [PATCH 05/10] Update .changeset/plenty-cloths-dance.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .changeset/plenty-cloths-dance.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/plenty-cloths-dance.md b/.changeset/plenty-cloths-dance.md index 341f82a06aa..3b78b5fda1f 100644 --- a/.changeset/plenty-cloths-dance.md +++ b/.changeset/plenty-cloths-dance.md @@ -2,4 +2,4 @@ '@primer/react': patch --- -Strictier type for BranchName: as="a" without href and neither as nor href are not allowed anymore. +Stricter type for BranchName: as="a" without href and neither as nor href are not allowed anymore. From 45e42f8a67decf02b30f367b21e5880c61d1a8d2 Mon Sep 17 00:00:00 2001 From: Lukas Oppermann Date: Tue, 11 Nov 2025 12:52:09 +0100 Subject: [PATCH 06/10] rm type test --- .../src/BranchName/__tests__/BranchName.test.tsx | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/packages/react/src/BranchName/__tests__/BranchName.test.tsx b/packages/react/src/BranchName/__tests__/BranchName.test.tsx index 204aa1066d3..f6e92b15220 100644 --- a/packages/react/src/BranchName/__tests__/BranchName.test.tsx +++ b/packages/react/src/BranchName/__tests__/BranchName.test.tsx @@ -17,16 +17,4 @@ describe('BranchName', () => { const {container} = HTMLRender() expect(container.firstChild?.nodeName).toEqual('SPAN') }) - - it('requires href when as="a"', () => { - // @ts-expect-error - href is required when as="a" - const Element = () => - expect(HTMLRender().container.firstChild?.nodeName).toEqual('A') - }) - - it('does not allow href when as="span"', () => { - // @ts-expect-error - href is required when as="a" - const Element = () => - expect(HTMLRender().container.firstChild?.nodeName).toEqual('SPAN') - }) }) From d1a66988d07e9b92a032ff76e73b3e6c7b1c7f29 Mon Sep 17 00:00:00 2001 From: Lukas Oppermann Date: Tue, 11 Nov 2025 13:20:02 +0100 Subject: [PATCH 07/10] fix --- packages/react/src/BranchName/BranchName.tsx | 3 +-- packages/react/src/BranchName/__tests__/BranchName.test.tsx | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/react/src/BranchName/BranchName.tsx b/packages/react/src/BranchName/BranchName.tsx index 34a4853c156..ce21cfb18f3 100644 --- a/packages/react/src/BranchName/BranchName.tsx +++ b/packages/react/src/BranchName/BranchName.tsx @@ -10,8 +10,7 @@ export type BranchNameProps = PolymorphicProps< { className?: string } -> & - (As extends 'a' ? {href: string} : {href?: never}) +> // eslint-disable-next-line @typescript-eslint/no-explicit-any function BranchName(props: BranchNameProps, ref: ForwardedRef) { diff --git a/packages/react/src/BranchName/__tests__/BranchName.test.tsx b/packages/react/src/BranchName/__tests__/BranchName.test.tsx index f6e92b15220..5434a50652c 100644 --- a/packages/react/src/BranchName/__tests__/BranchName.test.tsx +++ b/packages/react/src/BranchName/__tests__/BranchName.test.tsx @@ -9,7 +9,7 @@ describe('BranchName', () => { }) it('should support `className` on the outermost element', () => { - const Element = () => + const Element = () => expect(HTMLRender().container.firstChild).toHaveClass('test-class-name') }) From e14861d8f61d340af151539b378e3246fa4c45cf Mon Sep 17 00:00:00 2001 From: Lukas Oppermann Date: Tue, 11 Nov 2025 13:31:20 +0100 Subject: [PATCH 08/10] update storybook --- .changeset/plenty-cloths-dance.md | 2 +- .../src/BranchName/BranchName.features.stories.tsx | 7 +++++++ packages/react/src/BranchName/BranchName.tsx | 12 +++--------- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/.changeset/plenty-cloths-dance.md b/.changeset/plenty-cloths-dance.md index 3b78b5fda1f..ee03c5c2ea6 100644 --- a/.changeset/plenty-cloths-dance.md +++ b/.changeset/plenty-cloths-dance.md @@ -2,4 +2,4 @@ '@primer/react': patch --- -Stricter type for BranchName: as="a" without href and neither as nor href are not allowed anymore. +BranchName if no as prop is provided, set BranchName to "span" or to "a" if a "href" prop exists. diff --git a/packages/react/src/BranchName/BranchName.features.stories.tsx b/packages/react/src/BranchName/BranchName.features.stories.tsx index 21c229b519c..85db4bb601c 100644 --- a/packages/react/src/BranchName/BranchName.features.stories.tsx +++ b/packages/react/src/BranchName/BranchName.features.stories.tsx @@ -20,4 +20,11 @@ export const WithBranchIcon = () => ( export const NotALink = () => branch_name_as_span +export const LinkWithoutHref = () => ( +
+ branch_name_as_a + branch_name_no_as +
+) + export const NoProps = () => branch_name_no_props diff --git a/packages/react/src/BranchName/BranchName.tsx b/packages/react/src/BranchName/BranchName.tsx index ce21cfb18f3..71ad35472b3 100644 --- a/packages/react/src/BranchName/BranchName.tsx +++ b/packages/react/src/BranchName/BranchName.tsx @@ -14,18 +14,12 @@ export type BranchNameProps = PolymorphicProps< // eslint-disable-next-line @typescript-eslint/no-explicit-any function BranchName(props: BranchNameProps, ref: ForwardedRef) { - const {as: Component, className, children, ...rest} = props - - let InferredComponent = (Component || 'span') as As - - if (!Component && 'href' in props) { - InferredComponent = 'a' as As - } + const {as: Component = 'a', className, children, ...rest} = props return ( - + {children} - + ) } From 730ab065c181659733bc576165c6ec02f62b2c35 Mon Sep 17 00:00:00 2001 From: Lukas Oppermann Date: Tue, 11 Nov 2025 13:39:31 +0100 Subject: [PATCH 09/10] only add storybook --- .changeset/plenty-cloths-dance.md | 2 +- packages/react/src/BranchName/BranchName.tsx | 1 - .../react/src/BranchName/__tests__/BranchName.test.tsx | 7 +------ 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/.changeset/plenty-cloths-dance.md b/.changeset/plenty-cloths-dance.md index ee03c5c2ea6..9f7ccefe6b1 100644 --- a/.changeset/plenty-cloths-dance.md +++ b/.changeset/plenty-cloths-dance.md @@ -2,4 +2,4 @@ '@primer/react': patch --- -BranchName if no as prop is provided, set BranchName to "span" or to "a" if a "href" prop exists. +BranchName added more storybook stories to make sure the component is visually correct. diff --git a/packages/react/src/BranchName/BranchName.tsx b/packages/react/src/BranchName/BranchName.tsx index 71ad35472b3..aeb7bf4ff20 100644 --- a/packages/react/src/BranchName/BranchName.tsx +++ b/packages/react/src/BranchName/BranchName.tsx @@ -15,7 +15,6 @@ export type BranchNameProps = PolymorphicProps< // eslint-disable-next-line @typescript-eslint/no-explicit-any function BranchName(props: BranchNameProps, ref: ForwardedRef) { const {as: Component = 'a', className, children, ...rest} = props - return ( {children} diff --git a/packages/react/src/BranchName/__tests__/BranchName.test.tsx b/packages/react/src/BranchName/__tests__/BranchName.test.tsx index 5434a50652c..a66bcdcf4bf 100644 --- a/packages/react/src/BranchName/__tests__/BranchName.test.tsx +++ b/packages/react/src/BranchName/__tests__/BranchName.test.tsx @@ -4,7 +4,7 @@ import {describe, expect, it} from 'vitest' describe('BranchName', () => { it('renders an by default', () => { - const {container} = HTMLRender() + const {container} = HTMLRender() expect(container.firstChild?.nodeName).toEqual('A') }) @@ -12,9 +12,4 @@ describe('BranchName', () => { const Element = () => expect(HTMLRender().container.firstChild).toHaveClass('test-class-name') }) - - it('renders an by default', () => { - const {container} = HTMLRender() - expect(container.firstChild?.nodeName).toEqual('SPAN') - }) }) From 93a040cefe0039d89341c2dec11a2249943843db Mon Sep 17 00:00:00 2001 From: Lukas Oppermann Date: Wed, 12 Nov 2025 08:51:31 +0100 Subject: [PATCH 10/10] Delete .changeset/plenty-cloths-dance.md --- .changeset/plenty-cloths-dance.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .changeset/plenty-cloths-dance.md diff --git a/.changeset/plenty-cloths-dance.md b/.changeset/plenty-cloths-dance.md deleted file mode 100644 index 9f7ccefe6b1..00000000000 --- a/.changeset/plenty-cloths-dance.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@primer/react': patch ---- - -BranchName added more storybook stories to make sure the component is visually correct.