From 2840476b04e3b39acb2244e1395923d27f9f4d36 Mon Sep 17 00:00:00 2001 From: Johan Persson Date: Wed, 8 Apr 2026 15:58:20 +0200 Subject: [PATCH] feat(ui): resolve route-relative hrefs by default in useDefinition Made href resolution always-on in useDefinition instead of requiring each component to opt in via resolveHref: true. The hook now always calls useInRouterContext/useHref and only applies the resolved value when an href prop is actually provided, preventing href from leaking into components that don't accept it. Removed the resolveHref field from ComponentConfig, the ResolveHrefConstraint type, and resolveHref: true from all 14 component definitions. Also removed a now-unnecessary type cast in the Text component. Signed-off-by: Johan Persson --- .changeset/bumpy-boxes-chew.md | 5 ++++ packages/ui/report.api.md | 5 ---- .../src/components/ButtonLink/definition.ts | 1 - packages/ui/src/components/Card/definition.ts | 1 - .../components/Header/HeaderNavDefinition.ts | 1 - packages/ui/src/components/Link/definition.ts | 1 - packages/ui/src/components/List/definition.ts | 1 - packages/ui/src/components/Menu/definition.ts | 2 -- .../SearchAutocomplete/definition.ts | 1 - .../ui/src/components/Table/definition.ts | 3 --- packages/ui/src/components/Tabs/definition.ts | 1 - .../ui/src/components/TagGroup/definition.ts | 1 - packages/ui/src/components/Text/Text.tsx | 5 +--- packages/ui/src/hooks/useDefinition/types.ts | 26 ------------------- .../src/hooks/useDefinition/useDefinition.tsx | 21 +++++++-------- 15 files changed, 15 insertions(+), 60 deletions(-) create mode 100644 .changeset/bumpy-boxes-chew.md diff --git a/.changeset/bumpy-boxes-chew.md b/.changeset/bumpy-boxes-chew.md new file mode 100644 index 0000000000..71aa9011c7 --- /dev/null +++ b/.changeset/bumpy-boxes-chew.md @@ -0,0 +1,5 @@ +--- +'@backstage/ui': patch +--- + +Resolved route-relative `href` props to absolute paths by default in all components, removing the need for the `resolveHref` option in component definitions. diff --git a/packages/ui/report.api.md b/packages/ui/report.api.md index 0bce00e90e..3b79537f54 100644 --- a/packages/ui/report.api.md +++ b/packages/ui/report.api.md @@ -583,7 +583,6 @@ export const ButtonLinkDefinition: { }; readonly bg: 'consumer'; readonly analytics: true; - readonly resolveHref: true; readonly propDefs: { readonly noTrack: {}; readonly size: { @@ -690,7 +689,6 @@ export const CardDefinition: { readonly styles: { readonly [key: string]: string; }; - readonly resolveHref: true; readonly classNames: { readonly root: 'bui-Card'; readonly trigger: 'bui-CardTrigger'; @@ -1559,7 +1557,6 @@ export const HeaderNavItemDefinition: { readonly root: 'bui-HeaderNavItem'; }; readonly analytics: true; - readonly resolveHref: true; readonly propDefs: { readonly noTrack: {}; readonly id: {}; @@ -1682,7 +1679,6 @@ export const LinkDefinition: { readonly root: 'bui-Link'; }; readonly analytics: true; - readonly resolveHref: true; readonly propDefs: { readonly noTrack: {}; readonly variant: { @@ -1771,7 +1767,6 @@ export const ListRowDefinition: { readonly [key: string]: string; }; readonly bg: 'consumer'; - readonly resolveHref: true; readonly classNames: { readonly root: 'bui-ListRow'; readonly check: 'bui-ListRowCheck'; diff --git a/packages/ui/src/components/ButtonLink/definition.ts b/packages/ui/src/components/ButtonLink/definition.ts index e33b4aa390..a5f6a28489 100644 --- a/packages/ui/src/components/ButtonLink/definition.ts +++ b/packages/ui/src/components/ButtonLink/definition.ts @@ -30,7 +30,6 @@ export const ButtonLinkDefinition = defineComponent()({ }, bg: 'consumer', analytics: true, - resolveHref: true, propDefs: { noTrack: {}, size: { dataAttribute: true, default: 'small' }, diff --git a/packages/ui/src/components/Card/definition.ts b/packages/ui/src/components/Card/definition.ts index 691126067a..5d69005b4c 100644 --- a/packages/ui/src/components/Card/definition.ts +++ b/packages/ui/src/components/Card/definition.ts @@ -29,7 +29,6 @@ import styles from './Card.module.css'; */ export const CardDefinition = defineComponent()({ styles, - resolveHref: true, classNames: { root: 'bui-Card', trigger: 'bui-CardTrigger', diff --git a/packages/ui/src/components/Header/HeaderNavDefinition.ts b/packages/ui/src/components/Header/HeaderNavDefinition.ts index bbf3ce172f..d0bd3cda19 100644 --- a/packages/ui/src/components/Header/HeaderNavDefinition.ts +++ b/packages/ui/src/components/Header/HeaderNavDefinition.ts @@ -47,7 +47,6 @@ export const HeaderNavItemDefinition = defineComponent()({ root: 'bui-HeaderNavItem', }, analytics: true, - resolveHref: true, propDefs: { noTrack: {}, id: {}, diff --git a/packages/ui/src/components/Link/definition.ts b/packages/ui/src/components/Link/definition.ts index 7df8a0f256..15d0d35e12 100644 --- a/packages/ui/src/components/Link/definition.ts +++ b/packages/ui/src/components/Link/definition.ts @@ -28,7 +28,6 @@ export const LinkDefinition = defineComponent()({ root: 'bui-Link', }, analytics: true, - resolveHref: true, propDefs: { noTrack: {}, variant: { dataAttribute: true, default: 'body-medium' }, diff --git a/packages/ui/src/components/List/definition.ts b/packages/ui/src/components/List/definition.ts index e3f4b1b4f1..98b9e24fc6 100644 --- a/packages/ui/src/components/List/definition.ts +++ b/packages/ui/src/components/List/definition.ts @@ -42,7 +42,6 @@ export const ListDefinition = defineComponent()({ export const ListRowDefinition = defineComponent()({ styles, bg: 'consumer', - resolveHref: true, classNames: { root: 'bui-ListRow', check: 'bui-ListRowCheck', diff --git a/packages/ui/src/components/Menu/definition.ts b/packages/ui/src/components/Menu/definition.ts index 90d5266324..63c46a24a3 100644 --- a/packages/ui/src/components/Menu/definition.ts +++ b/packages/ui/src/components/Menu/definition.ts @@ -104,7 +104,6 @@ export const MenuItemDefinition = defineComponent()({ itemArrow: 'bui-MenuItemArrow', }, analytics: true, - resolveHref: true, propDefs: { iconStart: {}, children: {}, @@ -119,7 +118,6 @@ export const MenuItemDefinition = defineComponent()({ export const MenuListBoxItemDefinition = defineComponent()({ styles, - resolveHref: true, classNames: { root: 'bui-MenuItemListBox', itemContent: 'bui-MenuItemContent', diff --git a/packages/ui/src/components/SearchAutocomplete/definition.ts b/packages/ui/src/components/SearchAutocomplete/definition.ts index f773160297..d6204dad6a 100644 --- a/packages/ui/src/components/SearchAutocomplete/definition.ts +++ b/packages/ui/src/components/SearchAutocomplete/definition.ts @@ -61,7 +61,6 @@ export const SearchAutocompleteDefinition = export const SearchAutocompleteItemDefinition = defineComponent()({ styles, - resolveHref: true, classNames: { root: 'bui-SearchAutocompleteItem', itemContent: 'bui-SearchAutocompleteItemContent', diff --git a/packages/ui/src/components/Table/definition.ts b/packages/ui/src/components/Table/definition.ts index 5269b133f6..e2236e21b7 100644 --- a/packages/ui/src/components/Table/definition.ts +++ b/packages/ui/src/components/Table/definition.ts @@ -90,7 +90,6 @@ export const TableBodyDefinition = defineComponent()({ */ export const RowDefinition = defineComponent()({ styles, - resolveHref: true, analytics: true, bg: 'consumer', classNames: { @@ -144,7 +143,6 @@ export const CellDefinition = defineComponent()({ */ export const CellTextDefinition = defineComponent()({ styles, - resolveHref: true, classNames: { root: 'bui-TableCell', cellContentWrapper: 'bui-TableCellContentWrapper', @@ -167,7 +165,6 @@ export const CellTextDefinition = defineComponent()({ */ export const CellProfileDefinition = defineComponent()({ styles, - resolveHref: true, classNames: { root: 'bui-TableCell', cellContentWrapper: 'bui-TableCellContentWrapper', diff --git a/packages/ui/src/components/Tabs/definition.ts b/packages/ui/src/components/Tabs/definition.ts index 3dd0665abe..4563d5e8c8 100644 --- a/packages/ui/src/components/Tabs/definition.ts +++ b/packages/ui/src/components/Tabs/definition.ts @@ -55,7 +55,6 @@ export const TabListDefinition = defineComponent()({ /** @internal */ export const TabDefinition = defineComponent()({ styles, - resolveHref: true, classNames: { root: 'bui-Tab', }, diff --git a/packages/ui/src/components/TagGroup/definition.ts b/packages/ui/src/components/TagGroup/definition.ts index cb94c1da6f..38f6bef0ba 100644 --- a/packages/ui/src/components/TagGroup/definition.ts +++ b/packages/ui/src/components/TagGroup/definition.ts @@ -39,7 +39,6 @@ export const TagGroupDefinition = defineComponent()({ /** @internal */ export const TagDefinition = defineComponent()({ styles, - resolveHref: true, classNames: { root: 'bui-Tag', icon: 'bui-TagIcon', diff --git a/packages/ui/src/components/Text/Text.tsx b/packages/ui/src/components/Text/Text.tsx index c2ed20e38c..26ad63f2d0 100644 --- a/packages/ui/src/components/Text/Text.tsx +++ b/packages/ui/src/components/Text/Text.tsx @@ -24,12 +24,9 @@ function TextComponent( props: TextProps, ref: React.Ref, ) { - // Cast to default TextProps so TypeScript can evaluate the - // ResolveHrefConstraint. The generic ElementType is only used for - // the `as` prop which doesn't include 'a', so href is never present. const { ownProps, restProps, dataAttributes } = useDefinition( TextDefinition, - props as TextProps, + props, ); const { classes, as } = ownProps; diff --git a/packages/ui/src/hooks/useDefinition/types.ts b/packages/ui/src/hooks/useDefinition/types.ts index 900b7da71e..a819cb5d35 100644 --- a/packages/ui/src/hooks/useDefinition/types.ts +++ b/packages/ui/src/hooks/useDefinition/types.ts @@ -51,16 +51,6 @@ export interface ComponentConfig< * `noTrack?: boolean`. */ analytics?: boolean; - /** - * Whether this component accepts an href prop that should be turned - * into an absolute path before being passed to the underlying React - * Aria component. This is necessary because React Aria's navigate - * callback receives the raw href and cannot correctly turn relative - * paths into absolute ones from where it is called. When true, - * `useDefinition` will call `useHref()` to make the href absolute - * using the current route context. - */ - resolveHref?: boolean; } /** @@ -96,22 +86,6 @@ export type AnalyticsPropsConstraint = Analytics extends true } : {}; -/** - * Type constraint that ensures components whose props include `href` - * have `resolveHref: true` in their definition. This is necessary - * because React Aria's navigate callback cannot turn relative hrefs - * into absolute paths on its own in Backstage because of how routing - * is set up — the href must be made absolute before it reaches the - * React Aria layer. - */ -export type ResolveHrefConstraint = 'href' extends keyof P - ? ResolveHref extends true - ? {} - : { - __error: 'Components with href must set resolveHref: true in their definition to properly resolve relative paths.'; - } - : {}; - export interface UseDefinitionOptions> { utilityTarget?: keyof D['classNames'] | null; classNameTarget?: keyof D['classNames'] | null; diff --git a/packages/ui/src/hooks/useDefinition/useDefinition.tsx b/packages/ui/src/hooks/useDefinition/useDefinition.tsx index 067a5dfa48..a791343c15 100644 --- a/packages/ui/src/hooks/useDefinition/useDefinition.tsx +++ b/packages/ui/src/hooks/useDefinition/useDefinition.tsx @@ -24,7 +24,6 @@ import { noopTracker } from '../../analytics/useAnalytics'; import { useInRouterContext, useHref } from 'react-router-dom'; import type { ComponentConfig, - ResolveHrefConstraint, UseDefinitionOptions, UseDefinitionResult, UtilityKeys, @@ -34,7 +33,7 @@ export function useDefinition< D extends ComponentConfig, P extends Record, >( - definition: D & ResolveHrefConstraint, + definition: D, props: P, options?: UseDefinitionOptions, ): UseDefinitionResult { @@ -43,16 +42,14 @@ export function useDefinition< // Turn relative href into an absolute path using the current route // context, so that client-side navigation works correctly. let hrefResolvedProps = props; - if (definition.resolveHref) { - const hasRouter = useInRouterContext(); - // useHref throws outside a Router, so we guard with useInRouterContext. - // The guard is safe because a component's router context does not - // change during its lifetime, keeping the hook call count stable. - if (hasRouter) { - const absoluteHref = useHref((props as any).href ?? ''); - if ((props as any).href !== undefined) { - hrefResolvedProps = { ...props, href: absoluteHref } as P; - } + const hasRouter = useInRouterContext(); + // useHref throws outside a Router, so we guard with useInRouterContext. + // The guard is safe because a component's router context does not + // change during its lifetime, keeping the hook call count stable. + if (hasRouter) { + const absoluteHref = useHref((props as any).href ?? ''); + if ((props as any).href !== undefined) { + hrefResolvedProps = { ...props, href: absoluteHref } as P; } }