core-api: deprecate RouteRef title config and introduce id + add tests

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

.changeset/beige-lemons-accept.md

@@ -0,0 +1,5 @@
+---
+'@backstage/core-api': patch
+---
+
+Fully deprecate `title` option of `RouteRef`s and introduce `id` instead.

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',

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<undefined> = 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<T extends AnyParams>(_ref: RouteRef<T>) {}
+
+    const _1 = createRouteRef({ id: '1', params: ['x'] });
+    // @ts-expect-error
+    validateType<{ y: string }>(_1);
+    // @ts-expect-error
+    validateType<undefined>(_1);
+    validateType<{ x: string }>(_1);
+
+    const _2 = createRouteRef({ id: '2', params: ['x', 'y'] });
+    // @ts-expect-error
+    validateType<{ x: string }>(_2);
+    // @ts-expect-error
+    validateType<undefined>(_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<undefined>(_3);
+
+    const _4 = createRouteRef({ id: '4' });
+    // @ts-expect-error
+    validateType<{ x: string }>(_4);
+    validateType<undefined>(_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}');
+  });
+});

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 extends AnyParams> = {
   params?: ParamKeys<Params>;
   path?: string;
@@ -37,11 +37,15 @@ export class RouteRefImpl<Params extends AnyParams>
   implements RouteRef<Params> {
   readonly [routeRefType] = 'absolute';
 
-  constructor(private readonly config: RouteRefConfig<Params>) {}
-
-  get params(): ParamKeys<Params> {
-    return this.config.params as any;
-  }
+  constructor(
+    private readonly id: string,
+    readonly params: ParamKeys<Params>,
+    private readonly config: {
+      path?: string;
+      icon?: IconComponent;
+      title?: string;
+    },
+  ) {}
 
   get icon() {
     return this.config.icon;
@@ -53,11 +57,11 @@ export class RouteRefImpl<Params extends AnyParams>
   }
 
   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<OptionalParams<Params>> {
-  return new RouteRefImpl({
-    ...config,
-    params: (config.params ?? []) as ParamKeys<OptionalParams<Params>>,
-  });
+  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<OptionalParams<Params>>,
+    config,
+  );
 }
 
 export function isRouteRef<Params extends AnyParams>(