diff --git a/.changeset/swift-ideas-fry.md b/.changeset/swift-ideas-fry.md new file mode 100644 index 0000000000..d0e308fce7 --- /dev/null +++ b/.changeset/swift-ideas-fry.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-plugin-api': minor +--- + +All route references are now forwards compatible with the new frontend system, i.e. `@backstage/frontend-plugin-api`. This means they no longer need to be converted with `convertLegacyRouteRef` or `convertLegacyRouteRefs` from `@backstage/core-compat-api`. diff --git a/docs/frontend-system/building-apps/08-migrating.md b/docs/frontend-system/building-apps/08-migrating.md index f57c22db96..2b248b12a7 100644 --- a/docs/frontend-system/building-apps/08-migrating.md +++ b/docs/frontend-system/building-apps/08-migrating.md @@ -9,7 +9,7 @@ description: How to migrate existing apps to the new frontend system This section describes how to migrate an existing Backstage app package to use the new frontend system. The app package is typically found at `packages/app` in your project and is responsible for wiring together the Backstage frontend application. -> **Who is this for?** +> **Who is this for?** > This guide is intended for maintainers of Backstage app packages (`packages/app`) who want to upgrade from the legacy frontend system to the new extension-based architecture. > **Prerequisites:** @@ -22,10 +22,10 @@ This section describes how to migrate an existing Backstage app package to use t We recommend a **two-phase migration process** to ensure a smooth and manageable transition: -- **Phase 1: Minimal Changes for Hybrid Configuration** +- **Phase 1: Minimal Changes for Hybrid Configuration** In this phase, you make the smallest set of changes necessary to enable your app to run in a hybrid mode. This allows you to start using the new frontend system while still relying on compatibility helpers and legacy code. The goal is to unblock your migration quickly, so you can benefit from the new system without a full rewrite. -- **Phase 2: Complete Transition to the New Frontend System** +- **Phase 2: Complete Transition to the New Frontend System** After your app is running in hybrid mode, you can gradually refactor your codebase to remove legacy code and compatibility helpers. This phase focuses on fully adopting the new frontend architecture, ensuring your codebase is clean, maintainable, and takes full advantage of the new features. :::warning @@ -157,36 +157,6 @@ const app = createApp({ }); ``` -If you were binding routes from a legacy `createApp`, you will need to use the `convertLegacyRouteRefs` and/or `convertLegacyRouteRef` to convert the routes to be compatible with the new system. - -For example, if both the `catalogPlugin` and `scaffolderPlugin` are legacy plugins, you can bind their routes like this: - -```ts -import { createApp } from '@backstage/frontend-defaults'; -import { - // ... - convertLegacyRouteRefs, - convertLegacyRouteRef, -} from '@backstage/core-compat-api'; - -// Ommitting converted options changes -//... - -const app = createApp({ - features: [ - // ... - convertedOptionsModule, - ], - // highlight-add-start - bindRoutes({ bind }) { - bind(convertLegacyRouteRefs(catalogPlugin.externalRoutes), { - createComponent: convertLegacyRouteRef(scaffolderPlugin.routes.root), - }); - }, - // highlight-add-end -}); -``` - ### 3) Fixing the `app.createRoot` call The `app.createRoot(...)` no longer accepts any arguments. This represents a fundamental change that the new frontend system introduces. In the old system the app element tree that you passed to `app.createRoot(...)` was the primary way that you installed and configured plugins and features in your app. In the new system this is instead replaced by extensions that are wired together into an extension tree. Much more responsibility has now been shifted to plugins, for example you no longer have to manually provide the route path for each plugin page, but instead only configure it if you want to override the default. For more information on how the new system works, see the [architecture](../architecture/00-index.md) section. @@ -590,21 +560,6 @@ const app = createApp({ Route bindings can still be done using this option, but you now also have the ability to bind routes using static configuration instead. See the section on [binding routes](../architecture/36-routes.md#binding-external-route-references) for more information. -Note that if you are binding routes from a legacy plugin that was converted using `convertLegacyAppRoot`, you will need to use the `convertLegacyRouteRefs` and/or `convertLegacyRouteRef` to convert the routes to be compatible with the new system. - -For example, if both the `catalogPlugin` and `scaffolderPlugin` are legacy plugins, you can bind their routes like this: - -```ts -const app = createApp({ - features: convertLegacyAppRoot(...), - bindRoutes({ bind }) { - bind(convertLegacyRouteRefs(catalogPlugin.externalRoutes), { - createComponent: convertLegacyRouteRef(scaffolderPlugin.routes.root), - }); - }, -}); -``` - #### `__experimentalTranslations` Translations are now installed as extensions, created using `TranslationBlueprint`. diff --git a/docs/frontend-system/building-plugins/05-migrating.md b/docs/frontend-system/building-plugins/05-migrating.md index 2d954cb526..4940e0526b 100644 --- a/docs/frontend-system/building-plugins/05-migrating.md +++ b/docs/frontend-system/building-plugins/05-migrating.md @@ -38,7 +38,6 @@ In order to migrate the actual definition of the plugin you need to recreate the ```ts title="my-plugin/src/alpha.tsx" import { createFrontendPlugin } from '@backstage/frontend-plugin-api'; - import { convertLegacyRouteRefs } from '@backstage/core-compat-api'; export default createFrontendPlugin({ // The plugin ID is now provided as `pluginId` instead of `id` @@ -47,15 +46,12 @@ In order to migrate the actual definition of the plugin you need to recreate the // bind all the extensions to the plugin /* highlight-next-line */ extensions: [/* APIs will go here, but don't worry about those yet */], - // convert old route refs to the new system - /* highlight-next-line */ - routes: convertLegacyRouteRefs({ + routes: { ... - }), - /* highlight-next-line */ - externalRoutes: convertLegacyRouteRefs({ + }, + externalRoutes: { ... - }), + }, }); ``` @@ -110,18 +106,15 @@ it can be migrated as the following, keeping in mind that you may need to switch ```tsx import { PageBlueprint } from '@backstage/frontend-plugin-api'; -import { - compatWrapper, - convertLegacyRouteRef, -} from '@backstage/core-compat-api'; +import { compatWrapper } from '@backstage/core-compat-api'; const fooPage = PageBlueprint.make({ params: { // This is the path that was previously defined in the app code. // It's labelled as the default one because it can be changed via configuration. path: '/foo', - // You can reuse the existing routeRef by wrapping it with convertLegacyRouteRef. - routeRef: convertLegacyRouteRef(rootRouteRef), + // You can reuse the existing routeRef. + routeRef: rootRouteRef, // these inputs usually match the props required by the component. loader: () => import('./components/').then(m => diff --git a/packages/core-compat-api/src/convertLegacyRouteRef.test.ts b/packages/core-compat-api/src/convertLegacyRouteRef.test.ts index b61ba3a114..c21ac8c5bc 100644 --- a/packages/core-compat-api/src/convertLegacyRouteRef.test.ts +++ b/packages/core-compat-api/src/convertLegacyRouteRef.test.ts @@ -91,40 +91,32 @@ describe('convertLegacyRouteRef', () => { const ref3Internal = OpaqueExternalRouteRef.toInternal(ref3Converted); const ref4Internal = OpaqueExternalRouteRef.toInternal(ref4Converted); - expect(ref1Internal.getDescription()).toBe( - 'routeRef{type=absolute,id=ref1}', - ); + expect(ref1Internal.getDescription()).toBe('ref1'); expect(ref1Internal.getParams()).toEqual([]); - expect(ref2Internal.getDescription()).toBe( - 'routeRef{type=absolute,id=ref2}', - ); + expect(ref2Internal.getDescription()).toBe('ref2'); expect(ref2Internal.getParams()).toEqual(['p1', 'p2']); expect(ref1sub1Internal.getDescription()).toBe( - 'routeRef{type=sub,id=sub1}', + 'at /sub1 with parent routeRef{type=absolute,id=ref1}', ); expect(ref1sub1Internal.getParams()).toEqual([]); expect(ref1sub1Internal.getParent()).toBe(ref1); expect(ref1sub2Internal.getDescription()).toBe( - 'routeRef{type=sub,id=sub2}', + 'at /sub2/:p3 with parent routeRef{type=absolute,id=ref1}', ); expect(ref1sub2Internal.getParams()).toEqual(['p3']); expect(ref1sub2Internal.getParent()).toBe(ref1); expect(ref2sub1Internal.getDescription()).toBe( - 'routeRef{type=sub,id=sub1}', + 'at /sub1/:p3 with parent routeRef{type=absolute,id=ref2}', ); expect(ref2sub1Internal.getParams()).toEqual(['p1', 'p2', 'p3']); expect(ref2sub1Internal.getParent()).toBe(ref2); expect(ref3Internal.getDefaultTarget()).toBe(undefined); - expect(ref3Internal.getDescription()).toBe( - 'routeRef{type=external,id=ref3}', - ); + expect(ref3Internal.getDescription()).toBe('ref3'); expect(ref3Internal.getParams()).toEqual([]); expect(ref4Internal.getDefaultTarget()).toBe('ref2'); - expect(ref4Internal.getDescription()).toBe( - 'routeRef{type=external,id=ref4}', - ); + expect(ref4Internal.getDescription()).toBe('ref4'); expect(ref4Internal.getParams()).toEqual(['p1', 'p2']); }); @@ -166,34 +158,34 @@ describe('convertLegacyRouteRef', () => { expect(ref3).toBe(ref3Converted); expect(ref4).toBe(ref4Converted); - expect(String(ref1Converted)).toMatch(/^RouteRef\{created at '.*'\}$/); + expect(String(ref1Converted)).toMatch(/^routeRef\{id=undefined,at='.*'\}$/); expect(ref1Converted.params).toEqual([]); - expect(String(ref2Converted)).toMatch(/^RouteRef\{created at '.*'\}$/); + expect(String(ref2Converted)).toMatch(/^routeRef\{id=undefined,at='.*'\}$/); expect(ref2Converted.params).toEqual(['p1', 'p2']); expect(String(ref1sub1Converted)).toMatch( - /^SubRouteRef\{at \/sub1 with parent created at '.*'\}$/, + /^subRouteRef\{path='\/sub1',parent=routeRef\{id=undefined,at='.*'\}\}$/, ); expect(ref1sub1Converted.params).toEqual([]); expect(ref1sub1Converted.parent).toBe(ref1); expect(String(ref1sub2Converted)).toMatch( - /^SubRouteRef\{at \/sub2\/:p3 with parent created at '.*'\}$/, + /^subRouteRef\{path='\/sub2\/:p3',parent=routeRef\{id=undefined,at='.*'\}\}$/, ); expect(ref1sub2Converted.params).toEqual(['p3']); expect(ref1sub2Converted.parent).toBe(ref1); expect(String(ref2sub1Converted)).toMatch( - /^SubRouteRef\{at \/sub1\/:p3 with parent created at '.*'\}$/, + /^subRouteRef\{path='\/sub1\/:p3',parent=routeRef\{id=undefined,at='.*'\}\}$/, ); expect(ref2sub1Converted.params).toEqual(['p1', 'p2', 'p3']); expect(ref2sub1Converted.parent).toBe(ref2); expect(String(ref3Converted)).toMatch( - /^ExternalRouteRef\{created at '.*'\}$/, + /^externalRouteRef\{id=undefined,at='.*'\}$/, ); expect(ref3Converted.params).toEqual([]); expect(ref3Converted.optional).toBe(true); expect(String(ref4Converted)).toMatch( - /^ExternalRouteRef\{created at '.*'\}$/, + /^externalRouteRef\{id=undefined,at='.*'\}$/, ); expect(ref4Converted.params).toEqual(['p1', 'p2']); expect(ref4Converted.optional).toBe(true); diff --git a/packages/core-compat-api/src/convertLegacyRouteRef.ts b/packages/core-compat-api/src/convertLegacyRouteRef.ts index a8dfd8193e..e490d83a19 100644 --- a/packages/core-compat-api/src/convertLegacyRouteRef.ts +++ b/packages/core-compat-api/src/convertLegacyRouteRef.ts @@ -187,7 +187,7 @@ function convertNewToOld( [routeRefType]: 'absolute', params: newRef.getParams(), title: newRef.getDescription(), - } as Omit) as unknown as LegacyRouteRef; + } as Omit) as unknown as LegacyRouteRef; } if (ref.$$type === '@backstage/SubRouteRef') { const newRef = OpaqueSubRouteRef.toInternal(ref); @@ -195,7 +195,7 @@ function convertNewToOld( [routeRefType]: 'sub', parent: convertLegacyRouteRef(newRef.getParent()), params: newRef.getParams(), - } as Omit) as unknown as LegacySubRouteRef; + } as Omit) as unknown as LegacySubRouteRef; } if (ref.$$type === '@backstage/ExternalRouteRef') { const newRef = OpaqueExternalRouteRef.toInternal(ref); @@ -204,7 +204,7 @@ function convertNewToOld( optional: true, params: newRef.getParams(), defaultTarget: newRef.getDefaultTarget(), - } as Omit) as unknown as LegacyExternalRouteRef; + } as Omit) as unknown as LegacyExternalRouteRef; } throw new Error( diff --git a/packages/core-plugin-api/package.json b/packages/core-plugin-api/package.json index 430d081b49..921bc4af68 100644 --- a/packages/core-plugin-api/package.json +++ b/packages/core-plugin-api/package.json @@ -53,11 +53,13 @@ "@backstage/errors": "workspace:^", "@backstage/types": "workspace:^", "@backstage/version-bridge": "workspace:^", - "history": "^5.0.0" + "history": "^5.0.0", + "zod": "^3.22.4" }, "devDependencies": { "@backstage/cli": "workspace:^", "@backstage/core-app-api": "workspace:^", + "@backstage/frontend-plugin-api": "workspace:^", "@backstage/test-utils": "workspace:^", "@testing-library/dom": "^10.0.0", "@testing-library/jest-dom": "^6.0.0", diff --git a/packages/core-plugin-api/report.api.md b/packages/core-plugin-api/report.api.md index e1b1a243df..1e31cc5eb8 100644 --- a/packages/core-plugin-api/report.api.md +++ b/packages/core-plugin-api/report.api.md @@ -429,6 +429,8 @@ export type ExternalRouteRef< $$routeRefType: 'external'; params: ParamKeys; optional?: Optional; + readonly $$type: '@backstage/ExternalRouteRef'; + readonly T: Params; }; // @public @@ -698,6 +700,8 @@ export type RouteFunc = ( export type RouteRef = { $$routeRefType: 'absolute'; params: ParamKeys; + readonly $$type: '@backstage/RouteRef'; + readonly T: Params; }; // @public @@ -762,6 +766,8 @@ export type SubRouteRef = { parent: RouteRef; path: string; params: ParamKeys; + readonly $$type: '@backstage/SubRouteRef'; + readonly T: Params; }; // @public diff --git a/packages/core-plugin-api/src/routing/ExternalRouteRef.test.ts b/packages/core-plugin-api/src/routing/ExternalRouteRef.test.ts index 05e7a23d5d..8c3a5f0802 100644 --- a/packages/core-plugin-api/src/routing/ExternalRouteRef.test.ts +++ b/packages/core-plugin-api/src/routing/ExternalRouteRef.test.ts @@ -16,6 +16,7 @@ import { AnyParams, ExternalRouteRef } from './types'; import { createExternalRouteRef } from './ExternalRouteRef'; +import { RouteResolutionApi, RouteFunc } from '@backstage/frontend-plugin-api'; describe('ExternalRouteRef', () => { it('should be created', () => { @@ -24,7 +25,9 @@ describe('ExternalRouteRef', () => { }); expect(routeRef.params).toEqual([]); expect(routeRef.optional).toBe(false); - expect(String(routeRef)).toBe('routeRef{type=external,id=my-route-ref}'); + expect(String(routeRef)).toMatch( + /^routeRef\{type=external,id=my-route-ref\}$/, + ); }); it('should be created as optional', () => { @@ -109,4 +112,48 @@ describe('ExternalRouteRef', () => { // To avoid complains about missing expectations and unused vars expect([_1, _2, _3, _4, _5, _6].join('')).toEqual(expect.any(String)); }); + + describe('with new frontend system', () => { + const routeResolutionApi = { resolve: jest.fn() } as RouteResolutionApi; + + function expectType(): ( + v: U, + ) => [T, U] extends [U, T] ? { ok(): void } : { invalid: U } { + return () => ({ ok() {} } as any); + } + + it('should resolve routes correctly', () => { + expectType | undefined>()( + routeResolutionApi.resolve(createExternalRouteRef({ id: '1' })), + ).ok(); + expectType | undefined>()( + routeResolutionApi.resolve( + createExternalRouteRef({ id: '1', optional: true }), + ), + ).ok(); + expectType | undefined>()( + routeResolutionApi.resolve( + createExternalRouteRef({ id: '1', optional: false }), + ), + ).ok(); + + expectType | undefined>()( + routeResolutionApi.resolve( + createExternalRouteRef({ id: '1', params: ['x'] }), + ), + ).ok(); + expectType | undefined>()( + routeResolutionApi.resolve( + createExternalRouteRef({ id: '1', params: ['x'], optional: true }), + ), + ).ok(); + expectType | undefined>()( + routeResolutionApi.resolve( + createExternalRouteRef({ id: '1', params: ['x'], optional: false }), + ), + ).ok(); + + expect(1).toBe(1); + }); + }); }); diff --git a/packages/core-plugin-api/src/routing/ExternalRouteRef.ts b/packages/core-plugin-api/src/routing/ExternalRouteRef.ts index 16e2f9bc72..5d33aa7a67 100644 --- a/packages/core-plugin-api/src/routing/ExternalRouteRef.ts +++ b/packages/core-plugin-api/src/routing/ExternalRouteRef.ts @@ -52,12 +52,45 @@ export class ExternalRouteRefImpl< } toString() { + if (this.#nfsId) { + return `externalRouteRef{id=${this.#nfsId},legacyId=${this.id}}`; + } return `routeRef{type=external,id=${this.id}}`; } getDefaultTarget() { return this.defaultTarget; } + + // NFS implementation below + readonly $$type = '@backstage/ExternalRouteRef'; + readonly version = 'v1'; + readonly T = undefined as any; + + #nfsId: string | undefined = undefined; + + getParams(): string[] { + return this.params as string[]; + } + getDescription(): string { + if (this.#nfsId) { + return this.#nfsId; + } + return this.id; + } + setId(newId: string) { + if (!newId) { + throw new Error(`ExternalRouteRef id must be a non-empty string`); + } + if (this.#nfsId && this.#nfsId !== newId) { + throw new Error( + `ExternalRouteRef was referenced twice as both '${ + this.#nfsId + }' and '${newId}'`, + ); + } + this.#nfsId = newId; + } } /** @@ -106,5 +139,5 @@ export function createExternalRouteRef< (options.params ?? []) as ParamKeys>, Boolean(options.optional) as Optional, options?.defaultTarget, - ); + ) as ExternalRouteRef, Optional>; } diff --git a/packages/core-plugin-api/src/routing/RouteRef.test.ts b/packages/core-plugin-api/src/routing/RouteRef.test.ts index 489900d892..29ab228b9f 100644 --- a/packages/core-plugin-api/src/routing/RouteRef.test.ts +++ b/packages/core-plugin-api/src/routing/RouteRef.test.ts @@ -16,6 +16,7 @@ import { AnyParams, RouteRef, ParamKeys } from './types'; import { createRouteRef } from './RouteRef'; +import { RouteResolutionApi, RouteFunc } from '@backstage/frontend-plugin-api'; describe('RouteRef', () => { it('should be created', () => { @@ -23,7 +24,9 @@ describe('RouteRef', () => { id: 'my-route-ref', }); expect(routeRef.params).toEqual([]); - expect(String(routeRef)).toBe('routeRef{type=absolute,id=my-route-ref}'); + expect(String(routeRef)).toMatch( + /^routeRef\{type=absolute,id=my-route-ref\}$/, + ); }); it('should be created with params', () => { @@ -95,4 +98,37 @@ describe('RouteRef', () => { expect(true).toBeDefined(); }); + + describe('with new frontend system', () => { + const routeResolutionApi = { resolve: jest.fn() } as RouteResolutionApi; + + function expectType(): ( + v: U, + ) => [T, U] extends [U, T] ? { ok(): void } : { invalid: U } { + return () => ({ ok() {} } as any); + } + + it('should resolve routes correctly', () => { + expectType | undefined>()( + routeResolutionApi.resolve(createRouteRef({ id: '1' })), + ).ok(); + expectType | undefined>()( + routeResolutionApi.resolve(createRouteRef({ id: '1' })), + ).ok(); + expectType | undefined>()( + routeResolutionApi.resolve(createRouteRef({ id: '1', params: [] })), + ).ok(); + + expectType | undefined>()( + routeResolutionApi.resolve(createRouteRef({ id: '1', params: ['x'] })), + ).ok(); + expectType | undefined>()( + routeResolutionApi.resolve( + createRouteRef({ id: '1', params: ['x', 'y'] }), + ), + ).ok(); + + expect(1).toBe(1); + }); + }); }); diff --git a/packages/core-plugin-api/src/routing/RouteRef.ts b/packages/core-plugin-api/src/routing/RouteRef.ts index d78f3ddb8d..c8465dafde 100644 --- a/packages/core-plugin-api/src/routing/RouteRef.ts +++ b/packages/core-plugin-api/src/routing/RouteRef.ts @@ -45,8 +45,40 @@ export class RouteRefImpl } toString() { + if (this.#nfsId) { + return `routeRef{id=${this.#nfsId},legacyId=${this.id}}`; + } return `routeRef{type=absolute,id=${this.id}}`; } + + // NFS implementation below + readonly $$type = '@backstage/RouteRef'; + readonly version = 'v1'; + readonly T = undefined as any; + readonly alias = undefined; + + #nfsId: string | undefined = undefined; + + getParams() { + return this.params; + } + getDescription() { + if (this.#nfsId) { + return this.#nfsId; + } + return this.id; + } + setId(newId: string) { + if (!newId) { + throw new Error(`RouteRef id must be a non-empty string`); + } + if (this.#nfsId && this.#nfsId !== newId) { + throw new Error( + `RouteRef was referenced twice as both '${this.#nfsId}' and '${newId}'`, + ); + } + this.#nfsId = newId; + } } /** @@ -72,5 +104,5 @@ export function createRouteRef< return new RouteRefImpl( config.id, (config.params ?? []) as ParamKeys>, - ); + ) as RouteRef>; } diff --git a/packages/core-plugin-api/src/routing/SubRouteRef.test.ts b/packages/core-plugin-api/src/routing/SubRouteRef.test.ts index 1db429e178..ccec4c97d6 100644 --- a/packages/core-plugin-api/src/routing/SubRouteRef.test.ts +++ b/packages/core-plugin-api/src/routing/SubRouteRef.test.ts @@ -17,6 +17,7 @@ import { AnyParams, SubRouteRef } from './types'; import { createSubRouteRef } from './SubRouteRef'; import { createRouteRef } from './RouteRef'; +import { RouteResolutionApi, RouteFunc } from '@backstage/frontend-plugin-api'; const parent = createRouteRef({ id: 'parent' }); const parentX = createRouteRef({ id: 'parent-x', params: ['x'] }); @@ -31,7 +32,7 @@ describe('SubRouteRef', () => { expect(routeRef.path).toBe('/foo'); expect(routeRef.parent).toBe(parent); expect(routeRef.params).toEqual([]); - expect(String(routeRef)).toBe('routeRef{type=sub,id=my-route-ref}'); + expect(String(routeRef)).toMatch(/^routeRef\{type=sub,id=my-route-ref\}$/); }); it('should be created with params', () => { @@ -125,4 +126,51 @@ describe('SubRouteRef', () => { // To avoid complains about missing expectations and unused vars expect([_1, _2, _3, _4].join('')).toEqual(expect.any(String)); }); + + describe('with new frontend system', () => { + const routeResolutionApi = { resolve: jest.fn() } as RouteResolutionApi; + + function expectType(): ( + v: U, + ) => [T, U] extends [U, T] ? { ok(): void } : { invalid: U } { + return () => ({ ok() {} } as any); + } + + it('should resolve routes correctly', () => { + expectType | undefined>()( + routeResolutionApi.resolve( + createSubRouteRef({ id: '1', parent, path: '/foo' }), + ), + ).ok(); + expectType | undefined>()( + routeResolutionApi.resolve( + createSubRouteRef({ id: '1', parent: parentX, path: '/foo' }), + ), + ).ok(); + + expectType | undefined>()( + routeResolutionApi.resolve( + createSubRouteRef({ id: '1', parent, path: '/:y' }), + ), + ).ok(); + expectType | undefined>()( + routeResolutionApi.resolve( + createSubRouteRef({ id: '1', parent: parentX, path: '/:y' }), + ), + ).ok(); + + expectType | undefined>()( + routeResolutionApi.resolve( + createSubRouteRef({ id: '1', parent, path: '/:y/:z' }), + ), + ).ok(); + expectType | undefined>()( + routeResolutionApi.resolve( + createSubRouteRef({ id: '1', parent: parentX, path: '/:y/:z' }), + ), + ).ok(); + + expect(1).toBe(1); + }); + }); }); diff --git a/packages/core-plugin-api/src/routing/SubRouteRef.ts b/packages/core-plugin-api/src/routing/SubRouteRef.ts index ed9ded41d4..8db4c1947b 100644 --- a/packages/core-plugin-api/src/routing/SubRouteRef.ts +++ b/packages/core-plugin-api/src/routing/SubRouteRef.ts @@ -56,6 +56,21 @@ export class SubRouteRefImpl toString() { return `routeRef{type=sub,id=${this.id}}`; } + + // NFS implementation below + readonly $$type = '@backstage/SubRouteRef'; + readonly version = 'v1'; + readonly T = undefined as any; + + getParams(): string[] { + return this.params as string[]; + } + getParent(): RouteRef { + return this.parent; + } + getDescription(): string { + return `at ${this.path} with parent ${this.parent}`; + } } /** @@ -155,7 +170,7 @@ export function createSubRouteRef< path, parent, params as ParamKeys>, - ) as SubRouteRef>>; + ); // But skip type checking of the return value itself, because the conditional // type checking of the parent parameter overlap is tricky to express. diff --git a/packages/core-plugin-api/src/routing/types.ts b/packages/core-plugin-api/src/routing/types.ts index 4598574afb..b0a4d83918 100644 --- a/packages/core-plugin-api/src/routing/types.ts +++ b/packages/core-plugin-api/src/routing/types.ts @@ -98,6 +98,11 @@ export type RouteRef = { /** @deprecated access to this property will be removed in the future */ params: ParamKeys; + + /** Compatibility field for new frontend system */ + readonly $$type: '@backstage/RouteRef'; + /** Compatibility field for new frontend system */ + readonly T: Params; }; /** @@ -120,6 +125,11 @@ export type SubRouteRef = { /** @deprecated access to this property will be removed in the future */ params: ParamKeys; + + /** Compatibility field for new frontend system */ + readonly $$type: '@backstage/SubRouteRef'; + /** Compatibility field for new frontend system */ + readonly T: Params; }; /** @@ -142,6 +152,11 @@ export type ExternalRouteRef< params: ParamKeys; optional?: Optional; + + /** Compatibility field for new frontend system */ + readonly $$type: '@backstage/ExternalRouteRef'; + /** Compatibility field for new frontend system */ + readonly T: Params; }; /** diff --git a/packages/frontend-app-api/src/routing/RouteResolver.test.ts b/packages/frontend-app-api/src/routing/RouteResolver.test.ts index 1b23ac7131..7b9105f2ae 100644 --- a/packages/frontend-app-api/src/routing/RouteResolver.test.ts +++ b/packages/frontend-app-api/src/routing/RouteResolver.test.ts @@ -22,7 +22,12 @@ import { RouteRef, SubRouteRef, } from '@backstage/frontend-plugin-api'; -import { BackstagePlugin } from '@backstage/core-plugin-api'; +import { + BackstagePlugin, + createRouteRef as createLegacyRouteRef, + createSubRouteRef as createLegacySubRouteRef, + createExternalRouteRef as createLegacyExternalRouteRef, +} from '@backstage/core-plugin-api'; import { RouteResolver } from './RouteResolver'; import { MATCH_ALL_ROUTE } from './extractRouteInfoFromAppNode'; import { @@ -416,4 +421,196 @@ describe('RouteResolver', () => { '/my-parent/a%2F%23%26%3Fb', ); }); + + describe('with legacy route refs', () => { + const legacyRef1 = createLegacyRouteRef({ id: 'ref1' }); + const legacyRef2 = createLegacyRouteRef({ id: 'ref2', params: ['x'] }); + const legacyRef3 = createLegacyRouteRef({ id: 'ref3', params: ['y'] }); + const legacySubRef1 = createLegacySubRouteRef({ + id: 'sub1', + parent: legacyRef1, + path: '/foo', + }); + const legacySubRef2 = createLegacySubRouteRef({ + id: 'sub2', + parent: legacyRef1, + path: '/foo/:a', + }); + const legacySubRef3 = createLegacySubRouteRef({ + id: 'sub3', + parent: legacyRef2, + path: '/bar', + }); + const legacySubRef4 = createLegacySubRouteRef({ + id: 'sub4', + parent: legacyRef2, + path: '/bar/:a', + }); + const legacyExternalRef1 = createLegacyExternalRouteRef({ + id: 'external1', + }); + const legacyExternalRef2 = createLegacyExternalRouteRef({ + id: 'external2', + params: ['x'], + }); + + it('should not resolve anything with an empty resolver', () => { + const r = new RouteResolver( + new Map(), + new Map(), + [], + new Map(), + '', + emptyResolver, + new Map(), + ); + + expect(r.resolve(legacyRef1, src('/'))?.()).toBe(undefined); + expect(r.resolve(legacyRef2, src('/'))?.({ x: '1x' })).toBe(undefined); + expect(r.resolve(legacySubRef1, src('/'))?.()).toBe(undefined); + expect(r.resolve(legacySubRef2, src('/'))?.({ a: '2a' })).toBe(undefined); + expect(r.resolve(legacySubRef3, src('/'))?.({ x: '3x' })).toBe(undefined); + expect(r.resolve(legacySubRef4, src('/'))?.({ x: '4x', a: '4a' })).toBe( + undefined, + ); + expect(r.resolve(legacyExternalRef1, src('/'))?.()).toBe(undefined); + expect(r.resolve(legacyExternalRef2, src('/'))?.({ x: '5x' })).toBe( + undefined, + ); + }); + + it('should resolve an absolute route', () => { + const r = new RouteResolver( + new Map([[legacyRef1, 'my-route']]), + new Map(), + [{ routeRefs: new Set([legacyRef1]), path: 'my-route', ...rest }], + new Map(), + '', + emptyResolver, + new Map(), + ); + + expect(r.resolve(legacyRef1, src('/'))?.()).toBe('/my-route'); + expect(r.resolve(legacyRef2, src('/'))?.({ x: '1x' })).toBe(undefined); + expect(r.resolve(legacySubRef1, src('/'))?.()).toBe('/my-route/foo'); + expect(r.resolve(legacySubRef2, src('/'))?.({ a: '2a' })).toBe( + '/my-route/foo/2a', + ); + expect(r.resolve(legacySubRef3, src('/'))?.({ x: '3x' })).toBe(undefined); + expect(r.resolve(legacySubRef4, src('/'))?.({ x: '4x', a: '4a' })).toBe( + undefined, + ); + expect(r.resolve(legacyExternalRef1, src('/'))?.()).toBe(undefined); + expect(r.resolve(legacyExternalRef2, src('/'))?.({ x: '5x' })).toBe( + undefined, + ); + }); + + it('should resolve an absolute route with a param and with a parent', () => { + const r = new RouteResolver( + new Map([ + [legacyRef1, 'my-route'], + [legacyRef2, 'my-parent/:x'], + ]), + new Map([[legacyRef2, legacyRef1]]), + [ + { + routeRefs: new Set([legacyRef2]), + path: 'my-parent/:x', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { routeRefs: new Set([legacyRef1]), path: 'my-route', ...rest }, + ], + }, + ], + new Map([ + [legacyExternalRef1, legacyRef1], + [legacyExternalRef2, legacySubRef3], + ]), + '', + emptyResolver, + new Map(), + ); + + expect(r.resolve(legacyRef1, src('/'))?.()).toBe('/my-route'); + expect(r.resolve(legacyRef2, src('/'))?.({ x: '1x' })).toBe( + '/my-route/my-parent/1x', + ); + expect(r.resolve(legacySubRef1, src('/'))?.()).toBe('/my-route/foo'); + expect(r.resolve(legacySubRef2, src('/'))?.({ a: '2a' })).toBe( + '/my-route/foo/2a', + ); + expect(r.resolve(legacySubRef3, src('/'))?.({ x: '3x' })).toBe( + '/my-route/my-parent/3x/bar', + ); + expect(r.resolve(legacySubRef4, src('/'))?.({ x: '4x', a: '4a' })).toBe( + '/my-route/my-parent/4x/bar/4a', + ); + expect(r.resolve(legacyExternalRef1, src('/'))?.()).toBe('/my-route'); + expect(r.resolve(legacyExternalRef2, src('/'))?.({ x: '6x' })).toBe( + '/my-route/my-parent/6x/bar', + ); + }); + + it('should resolve the most specific match', () => { + const r = new RouteResolver( + new Map([ + [legacyRef1, 'deep'], + [legacyRef2, 'root/:x'], + [legacyRef3, 'sub/:y'], + ]), + new Map([ + [legacyRef3, legacyRef2], + [legacyRef1, legacyRef3], + ]), + [ + { + routeRefs: new Set([legacyRef2]), + path: 'root/:x', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { + routeRefs: new Set([legacyRef3]), + path: 'sub/:y', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { + routeRefs: new Set([legacyRef1]), + path: 'deep', + ...rest, + }, + ], + }, + ], + }, + ], + new Map(), + '', + emptyResolver, + new Map(), + ); + + expect(r.resolve(legacyRef2, src('/'))?.({ x: 'x' })).toBe('/root/x'); + expect(r.resolve(legacyRef3, src('/root/x'))?.({ y: 'y' })).toBe( + '/root/x/sub/y', + ); + + expect(() => r.resolve(legacyRef1, src('/'))?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(() => r.resolve(legacyRef1, src('/root/x'))?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(legacyRef1, src('/root/x/sub/y'))?.()).toBe( + '/root/x/sub/y/deep', + ); + // Without the MATCH_ALL_ROUTE, we wouldn't properly match the route here + expect( + r.resolve(legacyRef1, src('/root/x/sub/y/any/nested/path/here'))?.(), + ).toBe('/root/x/sub/y/deep'); + }); + }); }); diff --git a/packages/frontend-app-api/src/routing/collectRouteIds.test.ts b/packages/frontend-app-api/src/routing/collectRouteIds.test.ts index b1bab3fbbe..9575603bf4 100644 --- a/packages/frontend-app-api/src/routing/collectRouteIds.test.ts +++ b/packages/frontend-app-api/src/routing/collectRouteIds.test.ts @@ -39,10 +39,10 @@ describe('collectRouteIds', () => { const extRef = createExternalRouteRef(); expect(String(ref)).toMatch( - /^RouteRef\{created at '.*collectRouteIds\.test\.ts.*'\}$/, + /^routeRef\{id=undefined,at='.*collectRouteIds\.test\.ts.*'\}$/, ); expect(String(extRef)).toMatch( - /^ExternalRouteRef\{created at '.*collectRouteIds\.test\.ts.*'\}$/, + /^externalRouteRef\{id=undefined,at='.*collectRouteIds\.test\.ts.*'\}$/, ); const collected = collectRouteIds( @@ -62,8 +62,12 @@ describe('collectRouteIds', () => { 'test.extRef': extRef, }); - expect(String(ref)).toBe('RouteRef{test.ref}'); - expect(String(extRef)).toBe('ExternalRouteRef{test.extRef}'); + expect(String(ref)).toMatch( + /^routeRef\{id=test.ref,at='.*collectRouteIds\.test\.ts.*'\}$/, + ); + expect(String(extRef)).toMatch( + /^externalRouteRef\{id=test.extRef,at='.*collectRouteIds\.test\.ts.*'\}$/, + ); }); it('should report duplicate route IDs', () => { diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.test.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.test.ts index efcc9ef18c..8375cf6d3a 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.test.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.test.ts @@ -636,7 +636,7 @@ describe('discovery', () => { }, ), ).toThrow( - /Refused to resolve alias 'other.root' for RouteRef{created at 'at .*extractRouteInfoFromAppNode\.test\.ts:\d+:\d+'} as it points to a different plugin, the expected plugin is 'test' but the alias points to 'other'/, + /Refused to resolve alias 'other.root' for routeRef{id=undefined,at='.*extractRouteInfoFromAppNode\.test\.ts:\d+:\d+'} as it points to a different plugin, the expected plugin is 'test' but the alias points to 'other'/, ); }); @@ -662,7 +662,7 @@ describe('discovery', () => { }, ), ).toThrow( - /Alias loop detected for RouteRef{created at 'at .*extractRouteInfoFromAppNode\.test\.ts:\d+:\d+'}/, + /Alias loop detected for routeRef{id=undefined,at='.*extractRouteInfoFromAppNode\.test\.ts:\d+:\d+'}/, ); }); }); diff --git a/packages/frontend-plugin-api/report.api.md b/packages/frontend-plugin-api/report.api.md index f2fe60c82a..370e86727b 100644 --- a/packages/frontend-plugin-api/report.api.md +++ b/packages/frontend-plugin-api/report.api.md @@ -625,7 +625,7 @@ export function createExternalRouteRef< } | undefined = undefined, TParamKeys extends string = string, ->(options?: { +>(config?: { readonly params?: string extends TParamKeys ? (keyof TParams)[] : TParamKeys[]; diff --git a/packages/frontend-plugin-api/src/routing/ExternalRouteRef.test.ts b/packages/frontend-plugin-api/src/routing/ExternalRouteRef.test.ts index 93b5ed727c..15366c8a7a 100644 --- a/packages/frontend-plugin-api/src/routing/ExternalRouteRef.test.ts +++ b/packages/frontend-plugin-api/src/routing/ExternalRouteRef.test.ts @@ -25,10 +25,12 @@ describe('ExternalRouteRef', () => { expect(internal.getParams()).toEqual([]); expect(String(internal)).toMatch( - /^ExternalRouteRef\{created at '.*ExternalRouteRef\.test\.ts.*'\}$/, + /^externalRouteRef\{id=undefined,at='.*ExternalRouteRef\.test\.ts.*'\}$/, ); internal.setId('some-id'); - expect(String(internal)).toBe('ExternalRouteRef{some-id}'); + expect(String(internal)).toMatch( + /^externalRouteRef\{id=some-id,at='.*ExternalRouteRef\.test\.ts.*'\}$/, + ); }); it('should be created with params', () => { diff --git a/packages/frontend-plugin-api/src/routing/RouteRef.test.ts b/packages/frontend-plugin-api/src/routing/RouteRef.test.ts index f7b8e1b450..1bd546f821 100644 --- a/packages/frontend-plugin-api/src/routing/RouteRef.test.ts +++ b/packages/frontend-plugin-api/src/routing/RouteRef.test.ts @@ -27,7 +27,7 @@ describe('RouteRef', () => { expect(internal.getDescription()).toMatch(/RouteRef\.test\.ts/); expect(String(internal)).toMatch( - /^RouteRef\{created at .*RouteRef\.test\.ts.*\}$/, + /^routeRef\{id=undefined,at='.*RouteRef\.test\.ts.*'\}$/, ); expect(() => internal.setId('')).toThrow( @@ -35,7 +35,9 @@ describe('RouteRef', () => { ); internal.setId('some-id'); - expect(String(internal)).toBe('RouteRef{some-id}'); + expect(String(internal)).toMatch( + /^routeRef\{id=some-id,at='.*RouteRef\.test\.ts.*'\}$/, + ); internal.setId('some-id'); // Should allow same ID expect(() => internal.setId('some-other-id')).toThrow( diff --git a/packages/frontend-plugin-api/src/routing/SubRouteRef.test.ts b/packages/frontend-plugin-api/src/routing/SubRouteRef.test.ts index cd0b033d98..4746600f40 100644 --- a/packages/frontend-plugin-api/src/routing/SubRouteRef.test.ts +++ b/packages/frontend-plugin-api/src/routing/SubRouteRef.test.ts @@ -35,10 +35,12 @@ describe('SubRouteRef', () => { expect(internal.getParent()).toBe(internalParent); expect(internal.getParams()).toEqual([]); expect(String(internal)).toMatch( - /SubRouteRef\{at \/foo with parent created at '.*SubRouteRef\.test\.ts.*'\}/, + /^subRouteRef\{path='\/foo',parent=routeRef\{id=undefined,at='.*SubRouteRef\.test\.ts.*'\}\}$/, ); internalParent.setId('some-id'); - expect(String(internal)).toBe('SubRouteRef{at /foo with parent some-id}'); + expect(String(internal)).toMatch( + /^subRouteRef\{path='\/foo',parent=routeRef\{id=some-id,at='.*SubRouteRef\.test\.ts.*'\}\}$/, + ); }); it('should be created with params', () => { diff --git a/yarn.lock b/yarn.lock index 30725b6909..68357ca1da 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3641,6 +3641,7 @@ __metadata: "@backstage/config": "workspace:^" "@backstage/core-app-api": "workspace:^" "@backstage/errors": "workspace:^" + "@backstage/frontend-plugin-api": "workspace:^" "@backstage/test-utils": "workspace:^" "@backstage/types": "workspace:^" "@backstage/version-bridge": "workspace:^" @@ -3653,6 +3654,7 @@ __metadata: react: "npm:^18.0.2" react-dom: "npm:^18.0.2" react-router-dom: "npm:^6.3.0" + zod: "npm:^3.22.4" peerDependencies: "@types/react": ^17.0.0 || ^18.0.0 react: ^17.0.0 || ^18.0.0