diff --git a/.changeset/fix-bui-relative-href-resolution.md b/.changeset/fix-bui-relative-href-resolution.md new file mode 100644 index 0000000000..62d5b4ada2 --- /dev/null +++ b/.changeset/fix-bui-relative-href-resolution.md @@ -0,0 +1,7 @@ +--- +'@backstage/ui': patch +--- + +Fixed relative `href` resolution for BUI link components. Relative paths like `../other` are now correctly turned into absolute paths before reaching the React Aria layer, ensuring client-side navigation goes to the right place. + +**Affected components:** ButtonLink, Card, CellProfile, CellText, Link, ListRow, MenuItem, MenuListBoxItem, Row, SearchAutocompleteItem, Tab, Tag diff --git a/.patches/pr-33597.txt b/.patches/pr-33597.txt new file mode 100644 index 0000000000..f95fc325b4 --- /dev/null +++ b/.patches/pr-33597.txt @@ -0,0 +1 @@ +Fix relative href resolution for BUI link components \ No newline at end of file diff --git a/packages/ui/report.api.md b/packages/ui/report.api.md index b8a0f121de..f8aa8a024f 100644 --- a/packages/ui/report.api.md +++ b/packages/ui/report.api.md @@ -543,6 +543,7 @@ export const ButtonLinkDefinition: { }; readonly bg: 'consumer'; readonly analytics: true; + readonly resolveHref: true; readonly propDefs: { readonly noTrack: {}; readonly size: { @@ -649,6 +650,7 @@ export const CardDefinition: { readonly styles: { readonly [key: string]: string; }; + readonly resolveHref: true; readonly classNames: { readonly root: 'bui-Card'; readonly trigger: 'bui-CardTrigger'; @@ -1586,6 +1588,7 @@ export const LinkDefinition: { readonly root: 'bui-Link'; }; readonly analytics: true; + readonly resolveHref: true; readonly propDefs: { readonly noTrack: {}; readonly variant: { @@ -1674,6 +1677,7 @@ 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 a5f6a28489..e33b4aa390 100644 --- a/packages/ui/src/components/ButtonLink/definition.ts +++ b/packages/ui/src/components/ButtonLink/definition.ts @@ -30,6 +30,7 @@ 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 5d69005b4c..691126067a 100644 --- a/packages/ui/src/components/Card/definition.ts +++ b/packages/ui/src/components/Card/definition.ts @@ -29,6 +29,7 @@ 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/Link/definition.ts b/packages/ui/src/components/Link/definition.ts index 15d0d35e12..7df8a0f256 100644 --- a/packages/ui/src/components/Link/definition.ts +++ b/packages/ui/src/components/Link/definition.ts @@ -28,6 +28,7 @@ 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 98b9e24fc6..e3f4b1b4f1 100644 --- a/packages/ui/src/components/List/definition.ts +++ b/packages/ui/src/components/List/definition.ts @@ -42,6 +42,7 @@ 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 63c46a24a3..90d5266324 100644 --- a/packages/ui/src/components/Menu/definition.ts +++ b/packages/ui/src/components/Menu/definition.ts @@ -104,6 +104,7 @@ export const MenuItemDefinition = defineComponent()({ itemArrow: 'bui-MenuItemArrow', }, analytics: true, + resolveHref: true, propDefs: { iconStart: {}, children: {}, @@ -118,6 +119,7 @@ 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 d6204dad6a..f773160297 100644 --- a/packages/ui/src/components/SearchAutocomplete/definition.ts +++ b/packages/ui/src/components/SearchAutocomplete/definition.ts @@ -61,6 +61,7 @@ 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 e2236e21b7..5269b133f6 100644 --- a/packages/ui/src/components/Table/definition.ts +++ b/packages/ui/src/components/Table/definition.ts @@ -90,6 +90,7 @@ export const TableBodyDefinition = defineComponent()({ */ export const RowDefinition = defineComponent()({ styles, + resolveHref: true, analytics: true, bg: 'consumer', classNames: { @@ -143,6 +144,7 @@ export const CellDefinition = defineComponent()({ */ export const CellTextDefinition = defineComponent()({ styles, + resolveHref: true, classNames: { root: 'bui-TableCell', cellContentWrapper: 'bui-TableCellContentWrapper', @@ -165,6 +167,7 @@ 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 4563d5e8c8..3dd0665abe 100644 --- a/packages/ui/src/components/Tabs/definition.ts +++ b/packages/ui/src/components/Tabs/definition.ts @@ -55,6 +55,7 @@ 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 38f6bef0ba..cb94c1da6f 100644 --- a/packages/ui/src/components/TagGroup/definition.ts +++ b/packages/ui/src/components/TagGroup/definition.ts @@ -39,6 +39,7 @@ 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 294e129abf..0839b144c3 100644 --- a/packages/ui/src/components/Text/Text.tsx +++ b/packages/ui/src/components/Text/Text.tsx @@ -24,9 +24,12 @@ 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, + props as TextProps, ); const { classes, as } = ownProps; diff --git a/packages/ui/src/hooks/useDefinition/types.ts b/packages/ui/src/hooks/useDefinition/types.ts index a819cb5d35..900b7da71e 100644 --- a/packages/ui/src/hooks/useDefinition/types.ts +++ b/packages/ui/src/hooks/useDefinition/types.ts @@ -51,6 +51,16 @@ 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; } /** @@ -86,6 +96,22 @@ 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 20ce94e123..067a5dfa48 100644 --- a/packages/ui/src/hooks/useDefinition/useDefinition.tsx +++ b/packages/ui/src/hooks/useDefinition/useDefinition.tsx @@ -21,8 +21,10 @@ import { useBgProvider, useBgConsumer, BgProvider } from '../useBg'; import { resolveDefinitionProps, processUtilityProps } from './helpers'; import { useAnalytics } from '../../analytics/useAnalytics'; import { noopTracker } from '../../analytics/useAnalytics'; +import { useInRouterContext, useHref } from 'react-router-dom'; import type { ComponentConfig, + ResolveHrefConstraint, UseDefinitionOptions, UseDefinitionResult, UtilityKeys, @@ -32,16 +34,32 @@ export function useDefinition< D extends ComponentConfig, P extends Record, >( - definition: D, + definition: D & ResolveHrefConstraint, props: P, options?: UseDefinitionOptions, ): UseDefinitionResult { const { breakpoint } = useBreakpoint(); + // 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; + } + } + } + // Resolve all props centrally — applies responsive values and defaults const { ownPropsResolved, restProps } = resolveDefinitionProps( definition, - props, + hrefResolvedProps, breakpoint, ); @@ -85,7 +103,6 @@ export function useDefinition< ); // Analytics: conditionally call useAnalytics based on definition flag - // Safe: definition is a module-level constant, condition never changes at runtime let analytics = noopTracker; if (definition.analytics) { const tracker = useAnalytics();