diff --git a/.changeset/beige-lemons-accept.md b/.changeset/beige-lemons-accept.md new file mode 100644 index 0000000000..8204b87b5a --- /dev/null +++ b/.changeset/beige-lemons-accept.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-api': patch +--- + +Fully deprecate `title` option of `RouteRef`s and introduce `id` instead. diff --git a/packages/core-api/src/app/App.test.tsx b/packages/core-api/src/app/App.test.tsx index e474a8b851..a9f1e81a11 100644 --- a/packages/core-api/src/app/App.test.tsx +++ b/packages/core-api/src/app/App.test.tsx @@ -33,7 +33,7 @@ import { generateBoundRoutes, PrivateAppImpl } from './App'; describe('generateBoundRoutes', () => { it('runs happy path', () => { const external = { myRoute: createExternalRouteRef({ id: '1' }) }; - const ref = createRouteRef({ path: '', title: '' }); + const ref = createRouteRef({ id: 'ref-1' }); const result = generateBoundRoutes(({ bind }) => { bind(external, { myRoute: ref }); }); @@ -43,7 +43,7 @@ describe('generateBoundRoutes', () => { it('throws on unknown keys', () => { const external = { myRoute: createExternalRouteRef({ id: '2' }) }; - const ref = createRouteRef({ path: '', title: '' }); + const ref = createRouteRef({ id: 'ref-2' }); expect(() => generateBoundRoutes(({ bind }) => { bind(external, { someOtherRoute: ref } as any); @@ -53,12 +53,8 @@ describe('generateBoundRoutes', () => { }); describe('Integration Test', () => { - const plugin1RouteRef = createRouteRef({ path: '/blah1', title: '' }); - const plugin2RouteRef = createRouteRef({ - path: '/blah2', - title: '', - params: ['x'], - }); + const plugin1RouteRef = createRouteRef({ id: 'ref-1' }); + const plugin2RouteRef = createRouteRef({ id: 'ref-2', params: ['x'] }); const subRouteRef1 = createSubRouteRef({ id: 'sub1', path: '/sub1', diff --git a/packages/core-api/src/routing/RouteRef.test.ts b/packages/core-api/src/routing/RouteRef.test.ts new file mode 100644 index 0000000000..279589a6c7 --- /dev/null +++ b/packages/core-api/src/routing/RouteRef.test.ts @@ -0,0 +1,94 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { AnyParams, RouteRef } from './types'; +import { createRouteRef, isRouteRef } from './RouteRef'; +import { isSubRouteRef } from './SubRouteRef'; +import { isExternalRouteRef } from './ExternalRouteRef'; +import MyIcon from '@material-ui/icons/AcUnit'; + +describe('RouteRef', () => { + it('should be created', () => { + const routeRef: RouteRef = createRouteRef({ + id: 'my-route-ref', + }); + expect(routeRef.params).toEqual([]); + expect(String(routeRef)).toBe('routeRef{type=absolute,id=my-route-ref}'); + expect(isRouteRef(routeRef)).toBe(true); + expect(isSubRouteRef(routeRef)).toBe(false); + expect(isExternalRouteRef(routeRef)).toBe(false); + + expect(isRouteRef({} as RouteRef)).toBe(false); + }); + + it('should be created with params', () => { + const routeRef: RouteRef<{ + x: string; + y: string; + }> = createRouteRef({ + id: 'my-other-route-ref', + params: ['x', 'y'], + }); + expect(routeRef.params).toEqual(['x', 'y']); + }); + + it('should properly infer and validate parameter types and assignments', () => { + function validateType(_ref: RouteRef) {} + + const _1 = createRouteRef({ id: '1', params: ['x'] }); + // @ts-expect-error + validateType<{ y: string }>(_1); + // @ts-expect-error + validateType(_1); + validateType<{ x: string }>(_1); + + const _2 = createRouteRef({ id: '2', params: ['x', 'y'] }); + // @ts-expect-error + validateType<{ x: string }>(_2); + // @ts-expect-error + validateType(_2); + // @ts-expect-error + validateType<{ x: string; z: string }>(_2); + // TODO(Rugvip): Ideally this would fail as well, but settle for validating it at runtime instead + validateType<{ x: string; y: string; z: string }>(_2); + validateType<{ x: string; y: string }>(_2); + + const _3 = createRouteRef({ id: '3', params: [] }); + // @ts-expect-error + validateType<{ x: string }>(_3); + validateType(_3); + + const _4 = createRouteRef({ id: '4' }); + // @ts-expect-error + validateType<{ x: string }>(_4); + validateType(_4); + + // To avoid complains about missing expectations and unused vars + expect([_1, _2, _3, _4].join('')).toEqual(expect.any(String)); + }); + + it('should support deprecated access', () => { + const routeRef = createRouteRef({ + title: 'My Ref', + path: '/my-path', + icon: MyIcon, + }); + expect(routeRef.title).toBe('My Ref'); + expect(routeRef.path).toBe('/my-path'); + expect(routeRef.icon).toBe(MyIcon); + expect(String(routeRef)).toBe('routeRef{type=absolute,id=My Ref}'); + }); +}); diff --git a/packages/core-api/src/routing/RouteRef.ts b/packages/core-api/src/routing/RouteRef.ts index 3f14d2f49b..7ce2b2afbd 100644 --- a/packages/core-api/src/routing/RouteRef.ts +++ b/packages/core-api/src/routing/RouteRef.ts @@ -25,7 +25,7 @@ import { } from './types'; import { IconComponent } from '../icons'; -// TODO(Rugvip): Remove this once we get rid of the deprecated fields, it's not exported +// TODO(Rugvip): Remove this in the next breaking release, it's exported but unused export type RouteRefConfig = { params?: ParamKeys; path?: string; @@ -37,11 +37,15 @@ export class RouteRefImpl implements RouteRef { readonly [routeRefType] = 'absolute'; - constructor(private readonly config: RouteRefConfig) {} - - get params(): ParamKeys { - return this.config.params as any; - } + constructor( + private readonly id: string, + readonly params: ParamKeys, + private readonly config: { + path?: string; + icon?: IconComponent; + title?: string; + }, + ) {} get icon() { return this.config.icon; @@ -53,11 +57,11 @@ export class RouteRefImpl } get title() { - return this.config.title; + return this.config.title ?? this.id; } toString() { - return `routeRef{type=absolute,id=${this.config.title}}`; + return `routeRef{type=absolute,id=${this.id}}`; } } @@ -70,18 +74,26 @@ export function createRouteRef< // Param = {} if the params array is empty. ParamKey extends string = never >(config: { + /** The id of the route ref, used to identify it when printed */ + id?: string; + /** A list of parameter names that the path that this route ref is bound to must contain */ params?: ParamKey[]; /** @deprecated Route refs no longer decide their own path */ path?: string; /** @deprecated Route refs no longer decide their own icon */ icon?: IconComponent; /** @deprecated Route refs no longer decide their own title */ - title: string; + title?: string; }): RouteRef> { - return new RouteRefImpl({ - ...config, - params: (config.params ?? []) as ParamKeys>, - }); + const id = config.id || config.title; + if (!id) { + throw new Error('RouteRef must be provided a non-empty id'); + } + return new RouteRefImpl( + id, + (config.params ?? []) as ParamKeys>, + config, + ); } export function isRouteRef(