Merge pull request #33597 from backstage/bui-fix-relative-links
fix(ui): resolve relative hrefs to absolute paths before navigation
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
|
||||
@@ -0,0 +1 @@
|
||||
Fix relative href resolution for BUI link components
|
||||
@@ -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';
|
||||
|
||||
@@ -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