fix(ui): resolve relative hrefs to absolute paths before navigation
BUI link components with relative hrefs (e.g. `../other`) would navigate to the wrong URL because React Aria's navigate callback receives the raw href string and cannot resolve it correctly from where it is called in Backstage's routing setup. This adds a `resolveHref` flag to the component definition system. When enabled, `useDefinition` calls `useHref()` to turn relative hrefs into absolute paths before they reach the React Aria layer. A compile-time type guard ensures components with `href` in their props cannot omit the flag. Signed-off-by: Johan Persson <johanopersson@gmail.com>
This commit is contained in:
@@ -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
|
||||
@@ -542,6 +542,7 @@ export const ButtonLinkDefinition: {
|
||||
};
|
||||
readonly bg: 'consumer';
|
||||
readonly analytics: true;
|
||||
readonly resolveHref: true;
|
||||
readonly propDefs: {
|
||||
readonly noTrack: {};
|
||||
readonly size: {
|
||||
@@ -648,6 +649,7 @@ export const CardDefinition: {
|
||||
readonly styles: {
|
||||
readonly [key: string]: string;
|
||||
};
|
||||
readonly resolveHref: true;
|
||||
readonly classNames: {
|
||||
readonly root: 'bui-Card';
|
||||
readonly trigger: 'bui-CardTrigger';
|
||||
@@ -1583,6 +1585,7 @@ export const LinkDefinition: {
|
||||
readonly root: 'bui-Link';
|
||||
};
|
||||
readonly analytics: true;
|
||||
readonly resolveHref: true;
|
||||
readonly propDefs: {
|
||||
readonly noTrack: {};
|
||||
readonly variant: {
|
||||
@@ -1671,6 +1674,7 @@ export const ListRowDefinition: {
|
||||
readonly [key: string]: string;
|
||||
};
|
||||
readonly bg: 'consumer';
|
||||
readonly resolveHref: true;
|
||||
readonly classNames: {
|
||||
readonly root: 'bui-ListRow';
|
||||
readonly check: 'bui-ListRowCheck';
|
||||
|
||||
@@ -30,6 +30,7 @@ export const ButtonLinkDefinition = defineComponent<ButtonLinkOwnProps>()({
|
||||
},
|
||||
bg: 'consumer',
|
||||
analytics: true,
|
||||
resolveHref: true,
|
||||
propDefs: {
|
||||
noTrack: {},
|
||||
size: { dataAttribute: true, default: 'small' },
|
||||
|
||||
@@ -29,6 +29,7 @@ import styles from './Card.module.css';
|
||||
*/
|
||||
export const CardDefinition = defineComponent<CardOwnProps>()({
|
||||
styles,
|
||||
resolveHref: true,
|
||||
classNames: {
|
||||
root: 'bui-Card',
|
||||
trigger: 'bui-CardTrigger',
|
||||
|
||||
@@ -28,6 +28,7 @@ export const LinkDefinition = defineComponent<LinkOwnProps>()({
|
||||
root: 'bui-Link',
|
||||
},
|
||||
analytics: true,
|
||||
resolveHref: true,
|
||||
propDefs: {
|
||||
noTrack: {},
|
||||
variant: { dataAttribute: true, default: 'body-medium' },
|
||||
|
||||
@@ -42,6 +42,7 @@ export const ListDefinition = defineComponent<ListOwnProps>()({
|
||||
export const ListRowDefinition = defineComponent<ListRowOwnProps>()({
|
||||
styles,
|
||||
bg: 'consumer',
|
||||
resolveHref: true,
|
||||
classNames: {
|
||||
root: 'bui-ListRow',
|
||||
check: 'bui-ListRowCheck',
|
||||
|
||||
@@ -104,6 +104,7 @@ export const MenuItemDefinition = defineComponent<MenuItemOwnProps>()({
|
||||
itemArrow: 'bui-MenuItemArrow',
|
||||
},
|
||||
analytics: true,
|
||||
resolveHref: true,
|
||||
propDefs: {
|
||||
iconStart: {},
|
||||
children: {},
|
||||
@@ -118,6 +119,7 @@ export const MenuItemDefinition = defineComponent<MenuItemOwnProps>()({
|
||||
export const MenuListBoxItemDefinition =
|
||||
defineComponent<MenuListBoxItemOwnProps>()({
|
||||
styles,
|
||||
resolveHref: true,
|
||||
classNames: {
|
||||
root: 'bui-MenuItemListBox',
|
||||
itemContent: 'bui-MenuItemContent',
|
||||
|
||||
@@ -61,6 +61,7 @@ export const SearchAutocompleteDefinition =
|
||||
export const SearchAutocompleteItemDefinition =
|
||||
defineComponent<SearchAutocompleteItemOwnProps>()({
|
||||
styles,
|
||||
resolveHref: true,
|
||||
classNames: {
|
||||
root: 'bui-SearchAutocompleteItem',
|
||||
itemContent: 'bui-SearchAutocompleteItemContent',
|
||||
|
||||
@@ -90,6 +90,7 @@ export const TableBodyDefinition = defineComponent<TableBodyOwnProps>()({
|
||||
*/
|
||||
export const RowDefinition = defineComponent<RowOwnProps>()({
|
||||
styles,
|
||||
resolveHref: true,
|
||||
analytics: true,
|
||||
bg: 'consumer',
|
||||
classNames: {
|
||||
@@ -143,6 +144,7 @@ export const CellDefinition = defineComponent<CellOwnProps>()({
|
||||
*/
|
||||
export const CellTextDefinition = defineComponent<CellTextOwnProps>()({
|
||||
styles,
|
||||
resolveHref: true,
|
||||
classNames: {
|
||||
root: 'bui-TableCell',
|
||||
cellContentWrapper: 'bui-TableCellContentWrapper',
|
||||
@@ -165,6 +167,7 @@ export const CellTextDefinition = defineComponent<CellTextOwnProps>()({
|
||||
*/
|
||||
export const CellProfileDefinition = defineComponent<CellProfileOwnProps>()({
|
||||
styles,
|
||||
resolveHref: true,
|
||||
classNames: {
|
||||
root: 'bui-TableCell',
|
||||
cellContentWrapper: 'bui-TableCellContentWrapper',
|
||||
|
||||
@@ -55,6 +55,7 @@ export const TabListDefinition = defineComponent<TabListOwnProps>()({
|
||||
/** @internal */
|
||||
export const TabDefinition = defineComponent<TabOwnProps>()({
|
||||
styles,
|
||||
resolveHref: true,
|
||||
classNames: {
|
||||
root: 'bui-Tab',
|
||||
},
|
||||
|
||||
@@ -39,6 +39,7 @@ export const TagGroupDefinition = defineComponent<TagGroupOwnProps>()({
|
||||
/** @internal */
|
||||
export const TagDefinition = defineComponent<TagOwnProps>()({
|
||||
styles,
|
||||
resolveHref: true,
|
||||
classNames: {
|
||||
root: 'bui-Tag',
|
||||
icon: 'bui-TagIcon',
|
||||
|
||||
@@ -24,9 +24,12 @@ function TextComponent<T extends ElementType = 'span'>(
|
||||
props: TextProps<T>,
|
||||
ref: React.Ref<any>,
|
||||
) {
|
||||
// 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;
|
||||
|
||||
|
||||
@@ -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<P, Analytics> = 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<P, ResolveHref> = '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<D extends ComponentConfig<any, any>> {
|
||||
utilityTarget?: keyof D['classNames'] | null;
|
||||
classNameTarget?: keyof D['classNames'] | null;
|
||||
|
||||
@@ -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<any, any>,
|
||||
P extends Record<string, any>,
|
||||
>(
|
||||
definition: D,
|
||||
definition: D & ResolveHrefConstraint<P, D['resolveHref']>,
|
||||
props: P,
|
||||
options?: UseDefinitionOptions<D>,
|
||||
): UseDefinitionResult<D, P> {
|
||||
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();
|
||||
|
||||
Reference in New Issue
Block a user