diff --git a/.changeset/friendly-chicken-cry.md b/.changeset/friendly-chicken-cry.md new file mode 100644 index 0000000000..3eadca8777 --- /dev/null +++ b/.changeset/friendly-chicken-cry.md @@ -0,0 +1,8 @@ +--- +'@backstage/frontend-test-utils': patch +'@backstage/frontend-app-api': patch +'@backstage/core-compat-api': patch +'@backstage/plugin-app-visualizer': patch +--- + +Updated usage of `useRouteRef`, which can now always return `undefined`. diff --git a/.changeset/small-ears-poke.md b/.changeset/small-ears-poke.md new file mode 100644 index 0000000000..de906a8643 --- /dev/null +++ b/.changeset/small-ears-poke.md @@ -0,0 +1,7 @@ +--- +'@backstage/frontend-plugin-api': minor +--- + +**BREAKING**: All types of route refs are always considered optional by `useRouteRef`, which means the caller must always handle a potential `undefined` return value. Related to this change, the `optional` option from `createExternalRouteRef` has been removed, since it is no longer necessary. + +This is released as an immediate breaking change as we expect the usage of the new route refs to be extremely low or zero, since plugins that support the new system will still use route refs and `useRouteRef` from `@backstage/core-plugin-api` in combination with `convertLegacyRouteRef` from `@backstage/core-compat-api`. diff --git a/docs/frontend-system/architecture/07-routes.md b/docs/frontend-system/architecture/07-routes.md index 9c6617a06f..9db4d007b6 100644 --- a/docs/frontend-system/architecture/07-routes.md +++ b/docs/frontend-system/architecture/07-routes.md @@ -101,23 +101,26 @@ export const IndexPage = () => { return (

Index Page

- - See "Foo" details - + {/* highlight-next-line */} + {getDetailsPath && ( + + See "Foo" details + + )}
); }; ``` -We use the `useRouteRef` hook to create a link generator function that returns the details page path. We then call the link generator, passing it an object with the kind, namespace, and name. These parameters are used to construct a concrete path to the "Foo" details page. +We use the `useRouteRef` hook to create a link generator function that returns the details page path. First we need to check whether the route is available, the link generator function will be `undefined` if it isn't. We then call the link generator, passing it an object with the kind, namespace, and name. These parameters are used to construct a concrete path to the "Foo" details page. Let's see how the details page can get the parameters from the URL: @@ -176,8 +179,11 @@ export const IndexPage = () => { return (

Index Page

- {/* highlight-next-line */} - Create Component + {/* highlight-start */} + {getCreateComponentPath && ( + Create Component + )} + {/* highlight-end */}
); }; @@ -289,42 +295,6 @@ export const createComponentExternalRouteRef = createExternalRouteRef({ }); ``` -### Optional External Route References - -It is possible to define an `ExternalRouteRef` as optional, so it is not required to bind it in the app. - -```tsx title="plugins/catalog/src/routes.ts" -import { createExternalRouteRef } from '@backstage/frontend-plugin-api'; - -export const createComponentExternalRouteRef = createExternalRouteRef({ - // highlight-next-line - optional: true, -}); -``` - -When calling `useRouteRef` with an optional external route, its return signature is changed to `RouteFunc | undefined`, and the returned value can be used to decide whether a certain link should be displayed or if an action should be taken: - -```tsx title="plugins/catalog/src/components/IndexPage.tsx" -import React from 'react'; -import { useRouteRef } from '@backstage/frontend-plugin-api'; -import { createComponentExternalRouteRef } from '../routes'; - -export const IndexPage = () => { - const getCreateComponentPath = useRouteRef(createComponentExternalRouteRef); - return ( -
-

Index Page

- {/* Rendering the link only if the getCreateComponentPath is defined */} - {/* highlight-start */} - {getCreateComponentPath && ( - Create Component - )} - {/* highlight-end */} -
- ); -}; -``` - ## Sub Route References The last kind of route ref that can be created is a `SubRouteRef`, which can be used to create a route ref with a fixed path relative to an absolute `RouteRef`. They are useful if you have a page that internally is mounted at a sub route of a page extension component, and you want other plugins to be able to route to that page. And they can be a useful utility to handle routing within a plugin itself as well. @@ -368,17 +338,21 @@ import { DetailsPage } from './DetailsPage'; export const IndexPage = () => { const { pathname } = useLocation(); + + // highlight-start const getIndexPath = useRouteRef(indexRouteRef); const getDetailsPath = useRouteRef(detailsSubRouteRef); + // highlight-end + return (

Index Page

{/* Linking to the details sub route */} - {pathname === getIndexPath() ? ( - // highlight-start + {/* highlight-start */} + {pathname === getIndexPath?.() ? ( { > Show details - // highlight-end + {/* highlight-end */} ) : ( - // highlight-next-line - Hide details + {/* highlight-next-line */} + Hide details )} {/* Registering the details sub route */} + {/* highlight-next-line */} } />
diff --git a/packages/app-next/src/examples/pagesPlugin.tsx b/packages/app-next/src/examples/pagesPlugin.tsx index 826040a6cb..095c992e18 100644 --- a/packages/app-next/src/examples/pagesPlugin.tsx +++ b/packages/app-next/src/examples/pagesPlugin.tsx @@ -47,9 +47,11 @@ const IndexPage = createPageExtension({ return (
op -
- Page 1 -
+ {page1Link && ( +
+ Page 1 +
+ )}
Home
@@ -82,10 +84,10 @@ const Page1 = createPageExtension({ return (

This is page 1

- Go back + {indexLink && Go back} Page 2 {/* Page 2 */} - Page X + {xLink && Page X}
Sub-page content: @@ -115,7 +117,7 @@ const ExternalPage = createPageExtension({ return (

This is page X

- Go back + {indexLink && Go back}
); }; diff --git a/packages/core-compat-api/api-report.md b/packages/core-compat-api/api-report.md index 3cd78e6f30..8d177b4dab 100644 --- a/packages/core-compat-api/api-report.md +++ b/packages/core-compat-api/api-report.md @@ -37,12 +37,9 @@ export function convertLegacyRouteRef( ): SubRouteRef_2; // @public -export function convertLegacyRouteRef< - TParams extends AnyRouteRefParams, - TOptional extends boolean, ->( - ref: ExternalRouteRef, -): ExternalRouteRef_2; +export function convertLegacyRouteRef( + ref: ExternalRouteRef, +): ExternalRouteRef_2; // @public export function convertLegacyRouteRef( @@ -55,12 +52,9 @@ export function convertLegacyRouteRef( ): SubRouteRef; // @public -export function convertLegacyRouteRef< - TParams extends AnyRouteRefParams, - TOptional extends boolean, ->( - ref: ExternalRouteRef_2, -): ExternalRouteRef; +export function convertLegacyRouteRef( + ref: ExternalRouteRef_2, +): ExternalRouteRef; // @public export function convertLegacyRouteRefs< @@ -93,8 +87,8 @@ export type ToNewRouteRef = ? RouteRef_2 : T extends SubRouteRef ? SubRouteRef_2 - : T extends ExternalRouteRef - ? ExternalRouteRef_2 + : T extends ExternalRouteRef + ? ExternalRouteRef_2 : never; // (No @packageDocumentation comment for this package) diff --git a/packages/core-compat-api/src/compatWrapper/ForwardsCompatProvider.tsx b/packages/core-compat-api/src/compatWrapper/ForwardsCompatProvider.tsx index b2200387cc..9c646d5b00 100644 --- a/packages/core-compat-api/src/compatWrapper/ForwardsCompatProvider.tsx +++ b/packages/core-compat-api/src/compatWrapper/ForwardsCompatProvider.tsx @@ -111,7 +111,7 @@ class CompatRouteResolutionApi implements RouteResolutionApi { anyRouteRef: | RouteRef | SubRouteRef - | ExternalRouteRef, + | ExternalRouteRef, options?: RouteResolutionApiResolveOptions | undefined, ): RouteFunc | undefined { const legacyRef = convertLegacyRouteRef(anyRouteRef as RouteRef); diff --git a/packages/core-compat-api/src/compatWrapper/compatWrapper.test.tsx b/packages/core-compat-api/src/compatWrapper/compatWrapper.test.tsx index 6a27bf19e8..04e2361087 100644 --- a/packages/core-compat-api/src/compatWrapper/compatWrapper.test.tsx +++ b/packages/core-compat-api/src/compatWrapper/compatWrapper.test.tsx @@ -129,7 +129,7 @@ describe('ForwardsCompatProvider', () => { function Component() { const link = useNewRouteRef(routeRef); - return
link: {link()}
; + return
link: {link?.()}
; } await renderInOldTestApp(compatWrapper(), { diff --git a/packages/core-compat-api/src/convertLegacyRouteRef.test.ts b/packages/core-compat-api/src/convertLegacyRouteRef.test.ts index bf82d1e034..f09060212c 100644 --- a/packages/core-compat-api/src/convertLegacyRouteRef.test.ts +++ b/packages/core-compat-api/src/convertLegacyRouteRef.test.ts @@ -123,13 +123,11 @@ describe('convertLegacyRouteRef', () => { 'routeRef{type=external,id=ref3}', ); expect(ref3Internal.getParams()).toEqual([]); - expect(ref3Internal.optional).toBe(false); expect(ref4Internal.getDefaultTarget()).toBe('ref2'); expect(ref4Internal.getDescription()).toBe( 'routeRef{type=external,id=ref4}', ); expect(ref4Internal.getParams()).toEqual(['p1', 'p2']); - expect(ref4Internal.optional).toBe(true); }); it('converts new to old', () => { @@ -149,7 +147,6 @@ describe('convertLegacyRouteRef', () => { }); const ref3 = createNewExternalRouteRef(); const ref4 = createNewExternalRouteRef({ - optional: true, defaultTarget: 'ref2', params: ['p1', 'p2'], }); @@ -196,7 +193,7 @@ describe('convertLegacyRouteRef', () => { /^ExternalRouteRef\{created at '.*'\}$/, ); expect(ref3Converted.params).toEqual([]); - expect(ref3Converted.optional).toBe(false); + expect(ref3Converted.optional).toBe(true); expect(String(ref4Converted)).toMatch( /^ExternalRouteRef\{created at '.*'\}$/, ); diff --git a/packages/core-compat-api/src/convertLegacyRouteRef.ts b/packages/core-compat-api/src/convertLegacyRouteRef.ts index 24c589bb87..40a95a81ff 100644 --- a/packages/core-compat-api/src/convertLegacyRouteRef.ts +++ b/packages/core-compat-api/src/convertLegacyRouteRef.ts @@ -51,8 +51,8 @@ export type ToNewRouteRef< ? RouteRef : T extends LegacySubRouteRef ? SubRouteRef - : T extends LegacyExternalRouteRef - ? ExternalRouteRef + : T extends LegacyExternalRouteRef + ? ExternalRouteRef : never; /** @@ -109,12 +109,9 @@ export function convertLegacyRouteRef( * * In the future the legacy createExternalRouteRef will instead create refs compatible with both systems. */ -export function convertLegacyRouteRef< - TParams extends AnyRouteRefParams, - TOptional extends boolean, ->( - ref: LegacyExternalRouteRef, -): ExternalRouteRef; +export function convertLegacyRouteRef( + ref: LegacyExternalRouteRef, +): ExternalRouteRef; /** * A temporary helper to convert a new route ref to the legacy system. @@ -148,12 +145,9 @@ export function convertLegacyRouteRef( * * In the future the legacy createExternalRouteRef will instead create refs compatible with both systems. */ -export function convertLegacyRouteRef< - TParams extends AnyRouteRefParams, - TOptional extends boolean, ->( - ref: ExternalRouteRef, -): LegacyExternalRouteRef; +export function convertLegacyRouteRef( + ref: ExternalRouteRef, +): LegacyExternalRouteRef; export function convertLegacyRouteRef( ref: | LegacyRouteRef @@ -209,6 +203,7 @@ function convertNewToOld( const newRef = toInternalExternalRouteRef(ref); return Object.assign(ref, { [routeRefType]: 'external', + optional: true, params: newRef.getParams(), defaultTarget: newRef.getDefaultTarget(), } as Omit) as unknown as LegacyExternalRouteRef; @@ -282,7 +277,6 @@ function convertOldToNew( const newRef = toInternalExternalRouteRef( createExternalRouteRef<{ [key in string]: string }>({ params: legacyRef.params as string[], - optional: legacyRef.optional, defaultTarget: 'getDefaultTarget' in legacyRef ? (legacyRef.getDefaultTarget as () => string | undefined)() @@ -293,7 +287,6 @@ function convertOldToNew( $$type: '@backstage/ExternalRouteRef' as const, version: 'v1', T: newRef.T, - optional: newRef.optional, getParams() { return newRef.getParams(); }, diff --git a/packages/frontend-app-api/api-report.md b/packages/frontend-app-api/api-report.md index 0375a67482..27d486b6a8 100644 --- a/packages/frontend-app-api/api-report.md +++ b/packages/frontend-app-api/api-report.md @@ -44,7 +44,7 @@ export type CreateAppRouteBinder = < externalRoutes: TExternalRoutes, targetRoutes: PartialKeys< TargetRouteMap, - KeysWithType> + KeysWithType> >, ) => void; diff --git a/packages/frontend-app-api/src/extensions/AppNav.tsx b/packages/frontend-app-api/src/extensions/AppNav.tsx index 761c6d5a24..ffef1c7c61 100644 --- a/packages/frontend-app-api/src/extensions/AppNav.tsx +++ b/packages/frontend-app-api/src/extensions/AppNav.tsx @@ -73,9 +73,12 @@ const SidebarNavItem = ( props: (typeof createNavItemExtension.targetDataRef)['T'], ) => { const { icon: Icon, title, routeRef } = props; - const to = useRouteRef(routeRef)(); + const link = useRouteRef(routeRef); + if (!link) { + return null; + } // TODO: Support opening modal, for example, the search one - return ; + return ; }; export const AppNav = createExtension({ diff --git a/packages/frontend-app-api/src/routing/RouteResolver.test.ts b/packages/frontend-app-api/src/routing/RouteResolver.test.ts index ba0a66a380..497e7397cb 100644 --- a/packages/frontend-app-api/src/routing/RouteResolver.test.ts +++ b/packages/frontend-app-api/src/routing/RouteResolver.test.ts @@ -41,9 +41,7 @@ const subRef2 = createSubRouteRef({ parent: ref1, path: '/foo/:a' }); const subRef3 = createSubRouteRef({ parent: ref2, path: '/bar' }); const subRef4 = createSubRouteRef({ parent: ref2, path: '/bar/:a' }); const externalRef1 = createExternalRouteRef(); -const externalRef2 = createExternalRouteRef({ optional: true }); -const externalRef3 = createExternalRouteRef({ params: ['x'] }); -const externalRef4 = createExternalRouteRef({ optional: true, params: ['x'] }); +const externalRef2 = createExternalRouteRef({ params: ['x'] }); function src(sourcePath: string) { return { sourcePath }; @@ -62,9 +60,7 @@ describe('RouteResolver', () => { undefined, ); expect(r.resolve(externalRef1, src('/'))?.()).toBe(undefined); - expect(r.resolve(externalRef2, src('/'))?.()).toBe(undefined); - expect(r.resolve(externalRef3, src('/'))?.({ x: '5x' })).toBe(undefined); - expect(r.resolve(externalRef4, src('/'))?.({ x: '6x' })).toBe(undefined); + expect(r.resolve(externalRef2, src('/'))?.({ x: '5x' })).toBe(undefined); }); it('should resolve an absolute route', () => { @@ -87,9 +83,7 @@ describe('RouteResolver', () => { undefined, ); expect(r.resolve(externalRef1, src('/'))?.()).toBe(undefined); - expect(r.resolve(externalRef2, src('/'))?.()).toBe(undefined); - expect(r.resolve(externalRef3, src('/'))?.({ x: '5x' })).toBe(undefined); - expect(r.resolve(externalRef4, src('/'))?.({ x: '6x' })).toBe(undefined); + expect(r.resolve(externalRef2, src('/'))?.({ x: '5x' })).toBe(undefined); }); it('should resolve an absolute route with a param and with a parent', () => { @@ -112,8 +106,7 @@ describe('RouteResolver', () => { ], new Map([ [externalRef1, ref1], - [externalRef3, ref2], - [externalRef4, subRef3], + [externalRef2, subRef3], ]), '', ); @@ -133,11 +126,7 @@ describe('RouteResolver', () => { '/my-route/my-parent/4x/bar/4a', ); expect(r.resolve(externalRef1, src('/'))?.()).toBe('/my-route'); - expect(r.resolve(externalRef2, src('/'))?.()).toBe(undefined); - expect(r.resolve(externalRef3, src('/'))?.({ x: '5x' })).toBe( - '/my-route/my-parent/5x', - ); - expect(r.resolve(externalRef4, src('/'))?.({ x: '6x' })).toBe( + expect(r.resolve(externalRef2, src('/'))?.({ x: '6x' })).toBe( '/my-route/my-parent/6x/bar', ); }); @@ -230,8 +219,7 @@ describe('RouteResolver', () => { ], new Map([ [externalRef1, ref1], - [externalRef3, ref2], - [externalRef4, subRef3], + [externalRef2, subRef3], ]), '', ); @@ -282,17 +270,10 @@ describe('RouteResolver', () => { expect(() => r.resolve(externalRef1, src('/'))?.()).toThrow( /^Cannot route.*with parent.*as it has parameters$/, ); - expect(r.resolve(externalRef2, src(l))?.()).toBe(undefined); - expect(r.resolve(externalRef3, src(l))?.({ x: '5x' })).toBe( - '/my-grandparent/my-y/my-parent/5x', + expect(r.resolve(externalRef2, src(l))?.({ x: '5x' })).toBe( + '/my-grandparent/my-y/my-parent/5x/bar', ); - expect(() => r.resolve(externalRef3, src('/'))?.({ x: '5x' })).toThrow( - /^Cannot route.*with parent.*as it has parameters$/, - ); - expect(r.resolve(externalRef4, src(l))?.({ x: '6x' })).toBe( - '/my-grandparent/my-y/my-parent/6x/bar', - ); - expect(() => r.resolve(externalRef4, src('/'))?.({ x: '6x' })).toThrow( + expect(() => r.resolve(externalRef2, src('/'))?.({ x: '5x' })).toThrow( /^Cannot route.*with parent.*as it has parameters$/, ); }); diff --git a/packages/frontend-app-api/src/routing/RouteResolver.ts b/packages/frontend-app-api/src/routing/RouteResolver.ts index 1ec1822fe9..ff7966c4db 100644 --- a/packages/frontend-app-api/src/routing/RouteResolver.ts +++ b/packages/frontend-app-api/src/routing/RouteResolver.ts @@ -195,7 +195,7 @@ export class RouteResolver implements RouteResolutionApi { anyRouteRef: | RouteRef | SubRouteRef - | ExternalRouteRef, + | ExternalRouteRef, options?: RouteResolutionApiResolveOptions, ): RouteFunc | undefined { // First figure out what our target absolute ref is, as well as our target path. diff --git a/packages/frontend-app-api/src/routing/resolveRouteBindings.ts b/packages/frontend-app-api/src/routing/resolveRouteBindings.ts index 81d9eddef3..a8f641daff 100644 --- a/packages/frontend-app-api/src/routing/resolveRouteBindings.ts +++ b/packages/frontend-app-api/src/routing/resolveRouteBindings.ts @@ -53,8 +53,7 @@ type TargetRouteMap< ExternalRoutes extends { [name: string]: ExternalRouteRef }, > = { [name in keyof ExternalRoutes]: ExternalRoutes[name] extends ExternalRouteRef< - infer Params, - any + infer Params > ? RouteRef | SubRouteRef : never; @@ -72,7 +71,7 @@ export type CreateAppRouteBinder = < externalRoutes: TExternalRoutes, targetRoutes: PartialKeys< TargetRouteMap, - KeysWithType> + KeysWithType> >, ) => void; @@ -95,11 +94,6 @@ export function resolveRouteBindings( if (!externalRoute) { throw new Error(`Key ${key} is not an existing external route`); } - if (!value && !externalRoute.optional) { - throw new Error( - `External route ${key} is required but was undefined`, - ); - } if (value) { result.set(externalRoute, value); } diff --git a/packages/frontend-plugin-api/api-report.md b/packages/frontend-plugin-api/api-report.md index d37f0a3a56..93f300c591 100644 --- a/packages/frontend-plugin-api/api-report.md +++ b/packages/frontend-plugin-api/api-report.md @@ -799,13 +799,11 @@ export function createExternalRouteRef< [param in TParamKeys]: string; } | undefined = undefined, - TOptional extends boolean = false, TParamKeys extends string = string, >(options?: { readonly params?: string extends TParamKeys ? (keyof TParams)[] : TParamKeys[]; - optional?: TOptional; defaultTarget?: string; }): ExternalRouteRef< keyof TParams extends never @@ -814,8 +812,7 @@ export function createExternalRouteRef< ? TParams : { [param in TParamKeys]: string; - }, - TOptional + } >; // @public @@ -1300,13 +1297,10 @@ export interface ExtensionOverridesOptions { // @public export interface ExternalRouteRef< TParams extends AnyRouteRefParams = AnyRouteRefParams, - TOptional extends boolean = boolean, > { // (undocumented) readonly $$type: '@backstage/ExternalRouteRef'; // (undocumented) - readonly optional: TOptional; - // (undocumented) readonly T: TParams; } @@ -1570,7 +1564,7 @@ export interface RouteResolutionApi { anyRouteRef: | RouteRef | SubRouteRef - | ExternalRouteRef, + | ExternalRouteRef, options?: RouteResolutionApiResolveOptions, ): RouteFunc | undefined; } @@ -1631,18 +1625,13 @@ export function useComponentRef( ref: ComponentRef, ): ComponentType; -// @public -export function useRouteRef< - TOptional extends boolean, - TParams extends AnyRouteRefParams, ->( - routeRef: ExternalRouteRef, -): TOptional extends true ? RouteFunc | undefined : RouteFunc; - // @public export function useRouteRef( - routeRef: RouteRef | SubRouteRef, -): RouteFunc; + routeRef: + | RouteRef + | SubRouteRef + | ExternalRouteRef, +): RouteFunc | undefined; // @public export function useRouteRefParams( diff --git a/packages/frontend-plugin-api/src/apis/definitions/RouteResolutionApi.ts b/packages/frontend-plugin-api/src/apis/definitions/RouteResolutionApi.ts index 06388cc966..30569bde15 100644 --- a/packages/frontend-plugin-api/src/apis/definitions/RouteResolutionApi.ts +++ b/packages/frontend-plugin-api/src/apis/definitions/RouteResolutionApi.ts @@ -60,7 +60,7 @@ export interface RouteResolutionApi { anyRouteRef: | RouteRef | SubRouteRef - | ExternalRouteRef, + | ExternalRouteRef, options?: RouteResolutionApiResolveOptions, ): RouteFunc | undefined; } diff --git a/packages/frontend-plugin-api/src/routing/ExternalRouteRef.test.ts b/packages/frontend-plugin-api/src/routing/ExternalRouteRef.test.ts index 727dc1f53e..97d3dd2264 100644 --- a/packages/frontend-plugin-api/src/routing/ExternalRouteRef.test.ts +++ b/packages/frontend-plugin-api/src/routing/ExternalRouteRef.test.ts @@ -26,7 +26,6 @@ describe('ExternalRouteRef', () => { const routeRef: ExternalRouteRef = createExternalRouteRef(); const internal = toInternalExternalRouteRef(routeRef); expect(internal.getParams()).toEqual([]); - expect(internal.optional).toBe(false); expect(String(internal)).toMatch( /^ExternalRouteRef\{created at '.*ExternalRouteRef\.test\.ts.*'\}$/, @@ -35,16 +34,6 @@ describe('ExternalRouteRef', () => { expect(String(internal)).toBe('ExternalRouteRef{some-id}'); }); - it('should be created as optional', () => { - const routeRef: ExternalRouteRef = createExternalRouteRef({ - params: [], - optional: true, - }); - const internal = toInternalExternalRouteRef(routeRef); - expect(internal.getParams()).toEqual([]); - expect(internal.optional).toEqual(true); - }); - it('should be created with params', () => { const routeRef: ExternalRouteRef<{ x: string; @@ -52,63 +41,39 @@ describe('ExternalRouteRef', () => { }> = createExternalRouteRef({ params: ['x', 'y'] }); const internal = toInternalExternalRouteRef(routeRef); expect(internal.getParams()).toEqual(['x', 'y']); - expect(internal.optional).toEqual(false); - }); - - it('should be created as optional with params', () => { - const routeRef: ExternalRouteRef<{ - x: string; - y: string; - }> = createExternalRouteRef({ params: ['x', 'y'], optional: true }); - const internal = toInternalExternalRouteRef(routeRef); - expect(internal.getParams()).toEqual(['x', 'y']); - expect(internal.optional).toEqual(true); }); it('should properly infer and validate parameter types and assignments', () => { - function checkRouteRef< - T extends AnyRouteRefParams, - TOptional extends boolean, - TCheck extends TOptional, - >( - _ref: ExternalRouteRef, + function checkRouteRef( + _ref: ExternalRouteRef, _params: T extends undefined ? undefined : T, - _optional: TCheck, ) {} - const _1 = createExternalRouteRef({ params: ['notX'] }); - checkRouteRef(_1, { notX: '' }, false); + const _1 = createExternalRouteRef(); + checkRouteRef(_1, undefined); // @ts-expect-error - checkRouteRef(_1, { x: '' }, false); + checkRouteRef(_1, { x: '' }); - const _2 = createExternalRouteRef({ params: ['x'], optional: true }); - checkRouteRef(_2, { x: '' }, true); + const _2 = createExternalRouteRef({ params: ['x'] }); + checkRouteRef(_2, { x: '' }); // @ts-expect-error - checkRouteRef(_2, undefined, false); + checkRouteRef(_2, { notX: '' }); + // @ts-expect-error + checkRouteRef(_2, undefined); const _3 = createExternalRouteRef({ params: ['x', 'y'] }); - checkRouteRef(_3, { x: '', y: '' }, false); + checkRouteRef(_3, { x: '', y: '' }); // @ts-expect-error - checkRouteRef(_3, { x: '' }, false); + checkRouteRef(_3, { x: '' }); // @ts-expect-error - checkRouteRef(_3, { x: '', y: '', z: '' }, false); + checkRouteRef(_3, { x: '', y: '', z: '' }); const _4 = createExternalRouteRef({ params: [] }); - checkRouteRef(_4, undefined, false); + checkRouteRef(_4, undefined); // @ts-expect-error - checkRouteRef(_4, { x: '' }); - - const _5 = createExternalRouteRef(); - checkRouteRef(_5, undefined, false); - // @ts-expect-error - checkRouteRef(_5, { x: '' }); - - const _6 = createExternalRouteRef({ optional: true }); - checkRouteRef(_6, undefined, true); - // @ts-expect-error - checkRouteRef(_6, undefined, false); + checkRouteRef(_4, { x: '' }); // To avoid complains about missing expectations and unused vars - expect([_1, _2, _3, _4, _5, _6].join('')).toEqual(expect.any(String)); + expect([_1, _2, _3, _4].join('')).toEqual(expect.any(String)); }); }); diff --git a/packages/frontend-plugin-api/src/routing/ExternalRouteRef.ts b/packages/frontend-plugin-api/src/routing/ExternalRouteRef.ts index 7a754ffa7b..6ce36707c0 100644 --- a/packages/frontend-plugin-api/src/routing/ExternalRouteRef.ts +++ b/packages/frontend-plugin-api/src/routing/ExternalRouteRef.ts @@ -29,18 +29,15 @@ import { AnyRouteRefParams } from './types'; */ export interface ExternalRouteRef< TParams extends AnyRouteRefParams = AnyRouteRefParams, - TOptional extends boolean = boolean, > { readonly $$type: '@backstage/ExternalRouteRef'; readonly T: TParams; - readonly optional: TOptional; } /** @internal */ export interface InternalExternalRouteRef< TParams extends AnyRouteRefParams = AnyRouteRefParams, - TOptional extends boolean = boolean, -> extends ExternalRouteRef { +> extends ExternalRouteRef { readonly version: 'v1'; getParams(): string[]; getDescription(): string; @@ -52,11 +49,8 @@ export interface InternalExternalRouteRef< /** @internal */ export function toInternalExternalRouteRef< TParams extends AnyRouteRefParams = AnyRouteRefParams, - TOptional extends boolean = boolean, ->( - resource: ExternalRouteRef, -): InternalExternalRouteRef { - const r = resource as InternalExternalRouteRef; +>(resource: ExternalRouteRef): InternalExternalRouteRef { + const r = resource as InternalExternalRouteRef; if (r.$$type !== '@backstage/ExternalRouteRef') { throw new Error(`Invalid ExternalRouteRef, bad type '${r.$$type}'`); } @@ -79,7 +73,6 @@ class ExternalRouteRefImpl readonly $$type = '@backstage/ExternalRouteRef' as any; constructor( - readonly optional: boolean, readonly params: string[] = [], readonly defaultTarget: string | undefined, creationSite: string, @@ -104,7 +97,6 @@ class ExternalRouteRefImpl */ export function createExternalRouteRef< TParams extends { [param in TParamKeys]: string } | undefined = undefined, - TOptional extends boolean = false, TParamKeys extends string = string, >(options?: { /** @@ -114,14 +106,6 @@ export function createExternalRouteRef< ? (keyof TParams)[] : TParamKeys[]; - /** - * Whether or not this route is optional, defaults to false. - * - * Optional external routes are not required to be bound in the app, and - * if they aren't, `useExternalRouteRef` will return `undefined`. - */ - optional?: TOptional; - /** * The route (typically in another plugin) that this should map to by default. * @@ -134,13 +118,11 @@ export function createExternalRouteRef< ? undefined : string extends TParamKeys ? TParams - : { [param in TParamKeys]: string }, - TOptional + : { [param in TParamKeys]: string } > { return new ExternalRouteRefImpl( - Boolean(options?.optional), options?.params as string[] | undefined, options?.defaultTarget, describeParentCallSite(), - ) as ExternalRouteRef; + ); } diff --git a/packages/frontend-plugin-api/src/routing/useRouteRef.test.tsx b/packages/frontend-plugin-api/src/routing/useRouteRef.test.tsx index 4f43cf8d01..bdac95a5a0 100644 --- a/packages/frontend-plugin-api/src/routing/useRouteRef.test.tsx +++ b/packages/frontend-plugin-api/src/routing/useRouteRef.test.tsx @@ -45,7 +45,7 @@ describe('v1 consumer', () => { }); const routeFunc = renderedHook.result.current; - expect(routeFunc()).toBe('/hello'); + expect(routeFunc?.()).toBe('/hello'); expect(resolve).toHaveBeenCalledWith( routeRef, expect.objectContaining({ @@ -54,6 +54,23 @@ describe('v1 consumer', () => { ); }); + it('should ignore missing routes', () => { + const routeRef = createRouteRef(); + + const renderedHook = renderHook(() => useRouteRef(routeRef), { + wrapper: ({ children }: React.PropsWithChildren<{}>) => ( + undefined }]]} + > + + + ), + }); + + const routeFunc = renderedHook.result.current; + expect(routeFunc).toBeUndefined(); + }); + it('re-resolves the routeFunc when the search parameters change', () => { const resolve = jest.fn(() => () => '/hello'); diff --git a/packages/frontend-plugin-api/src/routing/useRouteRef.tsx b/packages/frontend-plugin-api/src/routing/useRouteRef.tsx index fdf831953d..2d2b3a5458 100644 --- a/packages/frontend-plugin-api/src/routing/useRouteRef.tsx +++ b/packages/frontend-plugin-api/src/routing/useRouteRef.tsx @@ -30,47 +30,14 @@ import { RouteFunc, routeResolutionApiRef, useApi } from '../apis'; * See {@link https://backstage.io/docs/plugins/composability#routing-system} * * @param routeRef - The ref to route that should be converted to URL. - * @returns A function that will in turn return the concrete URL of the `routeRef`. - * @public - */ -export function useRouteRef< - TOptional extends boolean, - TParams extends AnyRouteRefParams, ->( - routeRef: ExternalRouteRef, -): TOptional extends true ? RouteFunc | undefined : RouteFunc; - -/** - * React hook for constructing URLs to routes. - * - * @remarks - * - * See {@link https://backstage.io/docs/plugins/composability#routing-system} - * - * @param routeRef - The ref to route that should be converted to URL. - * @returns A function that will in turn return the concrete URL of the `routeRef`. - * @public - */ -export function useRouteRef( - routeRef: RouteRef | SubRouteRef, -): RouteFunc; - -/** - * React hook for constructing URLs to routes. - * - * @remarks - * - * See {@link https://backstage.io/docs/plugins/composability#routing-system} - * - * @param routeRef - The ref to route that should be converted to URL. - * @returns A function that will in turn return the concrete URL of the `routeRef`. + * @returns A function that will in turn return the concrete URL of the `routeRef`, or `undefined` if the route is not available. * @public */ export function useRouteRef( routeRef: | RouteRef | SubRouteRef - | ExternalRouteRef, + | ExternalRouteRef, ): RouteFunc | undefined { const { pathname } = useLocation(); const routeResolutionApi = useApi(routeResolutionApiRef); @@ -80,10 +47,5 @@ export function useRouteRef( [routeResolutionApi, routeRef, pathname], ); - const isOptional = 'optional' in routeRef && routeRef.optional; - if (!routeFunc && !isOptional) { - throw new Error(`No path for ${routeRef}`); - } - return routeFunc; } diff --git a/packages/frontend-test-utils/src/app/createExtensionTester.tsx b/packages/frontend-test-utils/src/app/createExtensionTester.tsx index 1d41958a09..bbe52c3fbc 100644 --- a/packages/frontend-test-utils/src/app/createExtensionTester.tsx +++ b/packages/frontend-test-utils/src/app/createExtensionTester.tsx @@ -56,10 +56,13 @@ const NavItem = (props: { icon: IconComponent; }) => { const { routeRef, title, icon: Icon } = props; - const to = useRouteRef(routeRef)(); + const link = useRouteRef(routeRef); + if (!link) { + return null; + } return (
  • - + {title}
  • diff --git a/plugins/api-docs/api-report-alpha.md b/plugins/api-docs/api-report-alpha.md index 1d22283884..f17ba0fa3c 100644 --- a/plugins/api-docs/api-report-alpha.md +++ b/plugins/api-docs/api-report-alpha.md @@ -13,7 +13,7 @@ const _default: BackstagePlugin< root: RouteRef; }, { - registerApi: ExternalRouteRef; + registerApi: ExternalRouteRef; } >; export default _default; diff --git a/plugins/app-visualizer/src/components/AppVisualizerPage/DetailedVisualizer.tsx b/plugins/app-visualizer/src/components/AppVisualizerPage/DetailedVisualizer.tsx index 2356f38cfa..b2c9685bab 100644 --- a/plugins/app-visualizer/src/components/AppVisualizerPage/DetailedVisualizer.tsx +++ b/plugins/app-visualizer/src/components/AppVisualizerPage/DetailedVisualizer.tsx @@ -189,18 +189,12 @@ function OutputLink(props: { }) { const routeRef = props.node?.instance?.getData(coreExtensionData.routeRef); - let link: string | undefined = undefined; - try { - // eslint-disable-next-line react-hooks/rules-of-hooks - link = useRouteRef(routeRef as RouteRef)(); - } catch { - /* ignore */ - } + const link = useRouteRef(routeRef as RouteRef); return ( {props.dataRef.id}}> - {link ? link : null} + {link ? link : null} ); diff --git a/plugins/catalog-graph/api-report-alpha.md b/plugins/catalog-graph/api-report-alpha.md index 3895f12fcf..88b3a38b76 100644 --- a/plugins/catalog-graph/api-report-alpha.md +++ b/plugins/catalog-graph/api-report-alpha.md @@ -13,14 +13,11 @@ const _default: BackstagePlugin< catalogGraph: RouteRef; }, { - catalogEntity: ExternalRouteRef< - { - name: string; - kind: string; - namespace: string; - }, - true - >; + catalogEntity: ExternalRouteRef<{ + name: string; + kind: string; + namespace: string; + }>; } >; export default _default; diff --git a/plugins/catalog/api-report-alpha.md b/plugins/catalog/api-report-alpha.md index f79d65394a..336bf4bc2b 100644 --- a/plugins/catalog/api-report-alpha.md +++ b/plugins/catalog/api-report-alpha.md @@ -121,23 +121,17 @@ const _default: BackstagePlugin< }>; }, { - viewTechDoc: ExternalRouteRef< - { - name: string; - kind: string; - namespace: string; - }, - true - >; - createComponent: ExternalRouteRef; - createFromTemplate: ExternalRouteRef< - { - namespace: string; - templateName: string; - }, - true - >; - unregisterRedirect: ExternalRouteRef; + viewTechDoc: ExternalRouteRef<{ + name: string; + kind: string; + namespace: string; + }>; + createComponent: ExternalRouteRef; + createFromTemplate: ExternalRouteRef<{ + namespace: string; + templateName: string; + }>; + unregisterRedirect: ExternalRouteRef; } >; export default _default; diff --git a/plugins/org/api-report-alpha.md b/plugins/org/api-report-alpha.md index 8c5391d857..e7c4db2c6d 100644 --- a/plugins/org/api-report-alpha.md +++ b/plugins/org/api-report-alpha.md @@ -10,7 +10,7 @@ import { ExternalRouteRef } from '@backstage/frontend-plugin-api'; const _default: BackstagePlugin< {}, { - catalogIndex: ExternalRouteRef; + catalogIndex: ExternalRouteRef; } >; export default _default; diff --git a/plugins/scaffolder/api-report-alpha.md b/plugins/scaffolder/api-report-alpha.md index 3f0b60cd95..4628563be2 100644 --- a/plugins/scaffolder/api-report-alpha.md +++ b/plugins/scaffolder/api-report-alpha.md @@ -30,15 +30,12 @@ const _default: BackstagePlugin< edit: SubRouteRef; }, { - registerComponent: ExternalRouteRef; - viewTechDoc: ExternalRouteRef< - { - name: string; - kind: string; - namespace: string; - }, - true - >; + registerComponent: ExternalRouteRef; + viewTechDoc: ExternalRouteRef<{ + name: string; + kind: string; + namespace: string; + }>; } >; export default _default;