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/.changeset/blue-insects-sin.md b/.changeset/blue-insects-sin.md new file mode 100644 index 0000000000..6b601d2a8f --- /dev/null +++ b/.changeset/blue-insects-sin.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-api': patch +--- + +Fixed a bug where FlatRoutes didn't handle React Fragments properly. diff --git a/.changeset/heavy-lemons-hope.md b/.changeset/heavy-lemons-hope.md new file mode 100644 index 0000000000..7a47ec3644 --- /dev/null +++ b/.changeset/heavy-lemons-hope.md @@ -0,0 +1,41 @@ +--- +'@backstage/core-api': patch +--- + +Add `SubRouteRef`s, which can be used to create a route ref with a fixed path relative to an absolute `RouteRef`. They are useful if you for example have a page that is mounted at a sub route of a routable extension component, and you want other plugins to be able to route to that page. + +For example: + +```tsx +// routes.ts +const rootRouteRef = createRouteRef({ id: 'root' }); +const detailsRouteRef = createSubRouteRef({ + id: 'root-sub', + parent: rootRouteRef, + path: '/details', +}); + +// plugin.ts +export const myPlugin = createPlugin({ + routes: { + root: rootRouteRef, + details: detailsRouteRef, + } +}) + +export const MyPage = plugin.provide(createRoutableExtension({ + component: () => import('./components/MyPage').then(m => m.MyPage), + mountPoint: rootRouteRef, +})) + +// components/MyPage.tsx +const MyPage = () => ( + + {/* myPlugin.routes.root will take the user to this page */} + }> + + {/* myPlugin.routes.details will take the user to this page */} + }> + +) +``` diff --git a/.github/styles/vocab.txt b/.github/styles/vocab.txt index cabc62e67a..8997360782 100644 --- a/.github/styles/vocab.txt +++ b/.github/styles/vocab.txt @@ -252,6 +252,7 @@ stefanalund subcomponent subcomponents subkey +subroutes subtree superfences superset diff --git a/docs/plugins/composability.md b/docs/plugins/composability.md index 75993dbc46..e95a67417b 100644 --- a/docs/plugins/composability.md +++ b/docs/plugins/composability.md @@ -369,6 +369,50 @@ It is currently not possible to have parameterized `ExternalRouteRef`s, or to bind an external route to a parameterized route, although this may be added in the future if needed. +### Subroutes + +The last kind of route refs that can be created are `SubRouteRef`s, 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 routable extension component, and you want other plugins to be able to route +to that page. + +For example: + +```tsx +// routes.ts +const rootRouteRef = createRouteRef({ id: 'root' }); +const detailsRouteRef = createSubRouteRef({ + id: 'root-sub', + parent: rootRouteRef, + path: '/details', +}); + +// plugin.ts +export const myPlugin = createPlugin({ + routes: { + root: rootRouteRef, + details: detailsRouteRef, + } +}) + +export const MyPage = plugin.provide(createRoutableExtension({ + component: () => import('./components/MyPage').then(m => m.MyPage), + mountPoint: rootRouteRef, +})) + +// components/MyPage.tsx +const MyPage = () => ( + + {/* myPlugin.routes.root will take the user to this page */} + }> + + {/* myPlugin.routes.details will take the user to this page */} + }> + +) +``` + ### New Catalog Components The established pattern for selecting what plugins should be available on each diff --git a/packages/core-api/src/app/App.test.tsx b/packages/core-api/src/app/App.test.tsx index b9fdf05444..a9f1e81a11 100644 --- a/packages/core-api/src/app/App.test.tsx +++ b/packages/core-api/src/app/App.test.tsx @@ -23,13 +23,17 @@ import { createRoutableExtension } from '../extensions'; import { defaultSystemIcons } from '../icons'; import { createPlugin } from '../plugin'; import { useRouteRef } from '../routing/hooks'; -import { createExternalRouteRef, createRouteRef } from '../routing/RouteRef'; +import { + createExternalRouteRef, + createRouteRef, + createSubRouteRef, +} from '../routing'; 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 }); }); @@ -39,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); @@ -49,20 +53,39 @@ describe('generateBoundRoutes', () => { }); describe('Integration Test', () => { - const plugin1RouteRef = createRouteRef({ path: '/blah1', title: '' }); - const plugin2RouteRef = createRouteRef({ - path: '/blah2', - title: '', + const plugin1RouteRef = createRouteRef({ id: 'ref-1' }); + const plugin2RouteRef = createRouteRef({ id: 'ref-2', params: ['x'] }); + const subRouteRef1 = createSubRouteRef({ + id: 'sub1', + path: '/sub1', + parent: plugin1RouteRef, + }); + const subRouteRef2 = createSubRouteRef({ + id: 'sub2', + path: '/sub2/:x', + parent: plugin1RouteRef, + }); + const subRouteRef3 = createSubRouteRef({ + id: 'sub3', + path: '/sub3', + parent: plugin2RouteRef, + }); + const subRouteRef4 = createSubRouteRef({ + id: 'sub4', + path: '/sub4/:y', + parent: plugin2RouteRef, + }); + const extRouteRef1 = createExternalRouteRef({ id: 'extRouteRef1' }); + const extRouteRef2 = createExternalRouteRef({ + id: 'extRouteRef2', params: ['x'], }); - const err = createExternalRouteRef({ id: 'err' }); - const errParams = createExternalRouteRef({ id: 'errParams', params: ['x'] }); - const errOptional = createExternalRouteRef({ - id: 'errOptional', + const extRouteRef3 = createExternalRouteRef({ + id: 'extRouteRef3', optional: true, }); - const errParamsOptional = createExternalRouteRef({ - id: 'errParamsOptional', + const extRouteRef4 = createExternalRouteRef({ + id: 'extRouteRef4', optional: true, params: ['x'], }); @@ -70,10 +93,10 @@ describe('Integration Test', () => { const plugin1 = createPlugin({ id: 'blob', externalRoutes: { - err, - errParams, - errOptional, - errParamsOptional, + extRouteRef1, + extRouteRef2, + extRouteRef3, + extRouteRef4, }, }); @@ -92,19 +115,28 @@ describe('Integration Test', () => { createRoutableExtension({ component: () => Promise.resolve((_: PropsWithChildren<{ path?: string }>) => { - const errLink = useRouteRef(err); - const errParamsLink = useRouteRef(errParams); - const errOptionalLink = useRouteRef(errOptional); - const errParamsOptionalLink = useRouteRef(errParamsOptional); + const link1 = useRouteRef(plugin1RouteRef); + const link2 = useRouteRef(plugin2RouteRef); + const subLink1 = useRouteRef(subRouteRef1); + const subLink2 = useRouteRef(subRouteRef2); + const subLink3 = useRouteRef(subRouteRef3); + const subLink4 = useRouteRef(subRouteRef4); + const extLink1 = useRouteRef(extRouteRef1); + const extLink2 = useRouteRef(extRouteRef2); + const extLink3 = useRouteRef(extRouteRef3); + const extLink4 = useRouteRef(extRouteRef4); return (
- err: {errLink()} - errParams: {errParamsLink({ x: 'a' })} - errOptional: {errOptionalLink?.() ?? ''} - - errParamsOptional:{' '} - {errParamsOptionalLink?.({ x: 'b' }) ?? ''} - + link1: {link1()} + link2: {link2({ x: 'a' })} + subLink1: {subLink1()} + subLink2: {subLink2({ x: 'a' })} + subLink3: {subLink3({ x: 'b' })} + subLink4: {subLink4({ x: 'c', y: 'd' })} + extLink1: {extLink1()} + extLink2: {extLink2({ x: 'a' })} + extLink3: {extLink3?.() ?? ''} + extLink4: {extLink4?.({ x: 'b' }) ?? ''}
); }), @@ -136,10 +168,10 @@ describe('Integration Test', () => { components, bindRoutes: ({ bind }) => { bind(plugin1.externalRoutes, { - err: plugin1RouteRef, - errParams: plugin2RouteRef, - errOptional: plugin1RouteRef, - errParamsOptional: plugin2RouteRef, + extRouteRef1: plugin1RouteRef, + extRouteRef2: plugin2RouteRef, + extRouteRef3: subRouteRef1, + extRouteRef4: plugin2RouteRef, }); }, }); @@ -152,16 +184,22 @@ describe('Integration Test', () => { - + , ); - expect(screen.getByText('err: /')).toBeInTheDocument(); - expect(screen.getByText('errParams: /foo')).toBeInTheDocument(); - expect(screen.getByText('errOptional: /')).toBeInTheDocument(); - expect(screen.getByText('errParamsOptional: /foo')).toBeInTheDocument(); + expect(screen.getByText('link1: /')).toBeInTheDocument(); + expect(screen.getByText('link2: /foo/a')).toBeInTheDocument(); + expect(screen.getByText('subLink1: /sub1')).toBeInTheDocument(); + expect(screen.getByText('subLink2: /sub2/a')).toBeInTheDocument(); + expect(screen.getByText('subLink3: /foo/b/sub3')).toBeInTheDocument(); + expect(screen.getByText('subLink4: /foo/c/sub4/d')).toBeInTheDocument(); + expect(screen.getByText('extLink1: /')).toBeInTheDocument(); + expect(screen.getByText('extLink2: /foo/a')).toBeInTheDocument(); + expect(screen.getByText('extLink3: /sub1')).toBeInTheDocument(); + expect(screen.getByText('extLink4: /foo/b')).toBeInTheDocument(); }); it('runs happy paths without optional routes', async () => { @@ -181,8 +219,8 @@ describe('Integration Test', () => { components, bindRoutes: ({ bind }) => { bind(plugin1.externalRoutes, { - err: plugin1RouteRef, - errParams: plugin2RouteRef, + extRouteRef1: plugin1RouteRef, + extRouteRef2: plugin2RouteRef, }); }, }); @@ -201,10 +239,10 @@ describe('Integration Test', () => { , ); - expect(screen.getByText('err: /')).toBeInTheDocument(); - expect(screen.getByText('errParams: /foo')).toBeInTheDocument(); - expect(screen.getByText('errOptional: ')).toBeInTheDocument(); - expect(screen.getByText('errParamsOptional: ')).toBeInTheDocument(); + expect(screen.getByText('extLink1: /')).toBeInTheDocument(); + expect(screen.getByText('extLink2: /foo')).toBeInTheDocument(); + expect(screen.getByText('extLink3: ')).toBeInTheDocument(); + expect(screen.getByText('extLink4: ')).toBeInTheDocument(); }); it('should throw some error when the route has duplicate params', () => { @@ -224,8 +262,8 @@ describe('Integration Test', () => { components, bindRoutes: ({ bind }) => { bind(plugin1.externalRoutes, { - err: plugin1RouteRef, - errParams: plugin2RouteRef, + extRouteRef1: plugin1RouteRef, + extRouteRef2: plugin2RouteRef, }); }, }); diff --git a/packages/core-api/src/app/App.tsx b/packages/core-api/src/app/App.tsx index 3c58930709..9e14c9a5fb 100644 --- a/packages/core-api/src/app/App.tsx +++ b/packages/core-api/src/app/App.tsx @@ -57,7 +57,8 @@ import { routeParentCollector, routePathCollector, } from '../routing/collectors'; -import { RoutingProvider, validateRoutes } from '../routing/hooks'; +import { RoutingProvider } from '../routing/hooks'; +import { validateRoutes } from '../routing/validation'; import { AppContextProvider } from './AppContext'; import { AppIdentity } from './AppIdentity'; import { AppThemeProvider } from './AppThemeProvider'; diff --git a/packages/core-api/src/app/types.ts b/packages/core-api/src/app/types.ts index df4180c6e4..5bdf51837d 100644 --- a/packages/core-api/src/app/types.ts +++ b/packages/core-api/src/app/types.ts @@ -21,6 +21,7 @@ import { ExternalRouteRef, RouteRef } from '../routing'; import { AnyApiFactory } from '../apis'; import { AppTheme, ProfileInfo } from '../apis/definitions'; import { AppConfig } from '@backstage/config'; +import { SubRouteRef } from '../routing/types'; export type BootErrorPageProps = { step: 'load-config'; @@ -102,7 +103,7 @@ type TargetRouteMap = { infer Params, any > - ? RouteRef + ? RouteRef | SubRouteRef : never; }; diff --git a/packages/core-api/src/routing/ExternalRouteRef.test.ts b/packages/core-api/src/routing/ExternalRouteRef.test.ts new file mode 100644 index 0000000000..785ad2732f --- /dev/null +++ b/packages/core-api/src/routing/ExternalRouteRef.test.ts @@ -0,0 +1,119 @@ +/* + * 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, ExternalRouteRef } from './types'; +import { createExternalRouteRef, isExternalRouteRef } from './ExternalRouteRef'; +import { isSubRouteRef } from './SubRouteRef'; +import { isRouteRef } from './RouteRef'; + +describe('ExternalRouteRef', () => { + it('should be created', () => { + const routeRef: ExternalRouteRef = createExternalRouteRef({ + id: 'my-route-ref', + }); + expect(routeRef.params).toEqual([]); + expect(routeRef.optional).toBe(false); + expect(String(routeRef)).toBe('routeRef{type=external,id=my-route-ref}'); + expect(isRouteRef(routeRef)).toBe(false); + expect(isSubRouteRef(routeRef)).toBe(false); + expect(isExternalRouteRef(routeRef)).toBe(true); + + expect(isRouteRef({} as ExternalRouteRef)).toBe(false); + }); + + it('should be created as optional', () => { + const routeRef: ExternalRouteRef<{ + x: string; + y: string; + }> = createExternalRouteRef({ + id: 'my-other-route-ref', + params: [], + optional: true, + }); + expect(routeRef.params).toEqual([]); + expect(routeRef.optional).toEqual(true); + }); + + it('should be created with params', () => { + const routeRef: ExternalRouteRef<{ + x: string; + y: string; + }> = createExternalRouteRef({ + id: 'my-other-route-ref', + params: ['x', 'y'], + }); + expect(routeRef.params).toEqual(['x', 'y']); + expect(routeRef.optional).toEqual(false); + }); + + it('should be created as optional with params', () => { + const routeRef: ExternalRouteRef<{ + x: string; + y: string; + }> = createExternalRouteRef({ + id: 'my-other-route-ref', + params: ['x', 'y'], + optional: true, + }); + expect(routeRef.params).toEqual(['x', 'y']); + expect(routeRef.optional).toEqual(true); + }); + + it('should properly infer and validate parameter types and assignments', () => { + function validateType( + _ref: ExternalRouteRef, + ) {} + + const _1 = createExternalRouteRef({ id: '1', params: ['notX'] }); + // @ts-expect-error + validateType<{ x: string }, any>(_1); + validateType<{ notX: string }, any>(_1); + + const _2 = createExternalRouteRef({ + id: '2', + params: ['x'], + optional: true, + }); + // @ts-expect-error + validateType(_2); + validateType<{ x: string }, true>(_2); + + const _3 = createExternalRouteRef({ id: '3', params: ['x', 'y'] }); + // @ts-expect-error + validateType<{ x: string }, any>(_3); + // TODO(Rugvip): Ideally this would fail as well, but settle for validating it at runtime instead + validateType<{ x: string; y: string; z: string }, any>(_3); + validateType<{ x: string; y: string }, false>(_3); + + const _4 = createExternalRouteRef({ id: '4', params: [] }); + // @ts-expect-error + validateType<{ x: string }, any>(_4); + validateType(_4); + + const _5 = createExternalRouteRef({ id: '5' }); + // @ts-expect-error + validateType<{ x: string }, any>(_5); + validateType(_5); + + const _6 = createExternalRouteRef({ id: '6', optional: true }); + // @ts-expect-error + validateType(_6); + validateType(_6); + + // To avoid complains about missing expectations and unused vars + expect([_1, _2, _3, _4, _5, _6].join('')).toEqual(expect.any(String)); + }); +}); diff --git a/packages/core-api/src/routing/ExternalRouteRef.ts b/packages/core-api/src/routing/ExternalRouteRef.ts new file mode 100644 index 0000000000..e6af9cf8a7 --- /dev/null +++ b/packages/core-api/src/routing/ExternalRouteRef.ts @@ -0,0 +1,84 @@ +/* + * 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 { + RouteRef, + SubRouteRef, + ExternalRouteRef, + routeRefType, + AnyParams, + ParamKeys, + OptionalParams, +} from './types'; + +export class ExternalRouteRefImpl< + Params extends AnyParams, + Optional extends boolean +> implements ExternalRouteRef { + readonly [routeRefType] = 'external'; + + constructor( + private readonly id: string, + readonly params: ParamKeys, + readonly optional: Optional, + ) {} + + toString() { + return `routeRef{type=external,id=${this.id}}`; + } +} + +export function createExternalRouteRef< + Params extends { [param in ParamKey]: string }, + Optional extends boolean = false, + ParamKey extends string = never +>(options: { + /** + * An identifier for this route, used to identify it in error messages + */ + id: string; + + /** + * The parameters that will be provided to the external route reference. + */ + params?: ParamKey[]; + + /** + * 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, `useRouteRef` will return `undefined`. + */ + optional?: Optional; +}): ExternalRouteRef, Optional> { + return new ExternalRouteRefImpl( + options.id, + (options.params ?? []) as ParamKeys>, + Boolean(options.optional) as Optional, + ); +} + +export function isExternalRouteRef< + Params extends AnyParams, + Optional extends boolean +>( + routeRef: + | RouteRef + | SubRouteRef + | ExternalRouteRef, +): routeRef is ExternalRouteRef { + return routeRef[routeRefType] === 'external'; +} diff --git a/packages/core-api/src/routing/FlatRoutes.test.tsx b/packages/core-api/src/routing/FlatRoutes.test.tsx new file mode 100644 index 0000000000..a9b83d1016 --- /dev/null +++ b/packages/core-api/src/routing/FlatRoutes.test.tsx @@ -0,0 +1,111 @@ +/* + * 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 { render, RenderResult } from '@testing-library/react'; +import React, { ReactNode } from 'react'; +import { MemoryRouter, Route, Routes, useOutlet } from 'react-router-dom'; +import { AppContext } from '../app'; +import { AppContextProvider } from '../app/AppContext'; +import { FlatRoutes } from './FlatRoutes'; + +function makeRouteRenderer(node: ReactNode) { + let rendered: RenderResult | undefined = undefined; + return (path: string) => { + const content = ( + ({ + NotFoundErrorPage: () => <>Not Found, + }), + } as unknown) as AppContext + } + > + + + ); + if (rendered) { + rendered.unmount(); + rendered.rerender(content); + } else { + rendered = render(content); + } + return rendered; + }; +} + +describe('FlatRoutes', () => { + it('renders some routes', () => { + const renderRoute = makeRouteRenderer( + + a} /> + b} /> + , + ); + expect(renderRoute('/a').getByText('a')).toBeInTheDocument(); + expect(renderRoute('/b').getByText('b')).toBeInTheDocument(); + expect(renderRoute('/c').getByText('Not Found')).toBeInTheDocument(); + expect(renderRoute('/b').queryByText('Not Found')).not.toBeInTheDocument(); + expect(renderRoute('/a').getByText('a')).toBeInTheDocument(); + }); + + it('is not sensitive to ordering and overlapping routes', () => { + // The '/*' suffixes here are intentional and will be ignored by FlatRoutes + const routes = ( + <> + a-1} /> + a} /> + a-2} /> + + ); + const renderRoute = makeRouteRenderer({routes}); + expect(renderRoute('/a').getByText('a')).toBeInTheDocument(); + expect(renderRoute('/a-1').getByText('a-1')).toBeInTheDocument(); + expect(renderRoute('/a-2').getByText('a-2')).toBeInTheDocument(); + renderRoute('').unmount(); + + // This uses regular Routes from react-router, not that a-2 renders a, which is the behavior we're working around + const renderBadRoute = makeRouteRenderer({routes}); + expect(renderBadRoute('/a').getByText('a')).toBeInTheDocument(); + expect(renderBadRoute('/a-1').getByText('a-1')).toBeInTheDocument(); + expect(renderBadRoute('/a-2').getByText('a')).toBeInTheDocument(); + }); + + it('renders children straight as outlets', () => { + const MyPage = () => { + return <>Outlet: {useOutlet()}; + }; + + // The '/*' suffixes here are intentional and will be ignored by FlatRoutes + const routes = ( + <> + }> + a + + }> + a-b + + }> + b + + + ); + const renderRoute = makeRouteRenderer({routes}); + expect(renderRoute('/a').getByText('Outlet: a')).toBeInTheDocument(); + expect(renderRoute('/a/b').getByText('Outlet: a-b')).toBeInTheDocument(); + expect(renderRoute('/b').getByText('Outlet: b')).toBeInTheDocument(); + }); +}); diff --git a/packages/core-api/src/routing/FlatRoutes.tsx b/packages/core-api/src/routing/FlatRoutes.tsx index 3702442811..5f2cfc150d 100644 --- a/packages/core-api/src/routing/FlatRoutes.tsx +++ b/packages/core-api/src/routing/FlatRoutes.tsx @@ -27,44 +27,38 @@ type RouteObject = { // Similar to the same function from react-router, this collects routes from the // children, but only the first level of routes function createRoutesFromChildren(childrenNode: ReactNode): RouteObject[] { - return Children.toArray(childrenNode) - .flatMap(child => { - if (!isValidElement(child)) { - return []; - } + return Children.toArray(childrenNode).flatMap(child => { + if (!isValidElement(child)) { + return []; + } - const { children } = child.props; + const { children } = child.props; - if (child.type === Fragment) { - return createRoutesFromChildren(children); - } + if (child.type === Fragment) { + return createRoutesFromChildren(children); + } - let path = child.props.path as string | undefined; + let path = child.props.path as string | undefined; - // TODO(Rugvip): Work around plugins registering empty paths, remove once deprecated routes are gone - if (path === '') { - return []; - } - path = path?.replace(/\/\*$/, '') ?? '/'; + // TODO(Rugvip): Work around plugins registering empty paths, remove once deprecated routes are gone + if (path === '') { + return []; + } + path = path?.replace(/\/\*$/, '') ?? '/'; - return [ - { - path, - element: child, - children: children && [ - { - path: '/*', - element: children, - }, - ], - }, - ]; - }) - .sort((a, b) => b.path.localeCompare(a.path)) - .map(obj => { - obj.path = obj.path === '/' ? '/' : `${obj.path}/*`; - return obj; - }); + return [ + { + path, + element: child, + children: children && [ + { + path: '/*', + element: children, + }, + ], + }, + ]; + }); } type FlatRoutesProps = { @@ -74,7 +68,14 @@ type FlatRoutesProps = { export const FlatRoutes = (props: FlatRoutesProps): JSX.Element | null => { const app = useApp(); const { NotFoundErrorPage } = app.getComponents(); - const routes = createRoutesFromChildren(props.children); + const routes = createRoutesFromChildren(props.children) + // Routes are sorted to work around a bug where prefixes are unexpectedly matched + .sort((a, b) => b.path.localeCompare(a.path)) + // We make sure all routes have '/*' appended, except '/' + .map(obj => { + obj.path = obj.path === '/' ? '/' : `${obj.path}/*`; + return obj; + }); // TODO(Rugvip): Possibly add a way to skip this, like a noNotFoundPage prop routes.push({ 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 24f93eeeeb..7ce2b2afbd 100644 --- a/packages/core-api/src/routing/RouteRef.ts +++ b/packages/core-api/src/routing/RouteRef.ts @@ -16,14 +16,16 @@ import { RouteRef, + SubRouteRef, ExternalRouteRef, routeRefType, AnyParams, ParamKeys, + OptionalParams, } 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; @@ -31,24 +33,19 @@ export type RouteRefConfig = { title: string; }; -class RouteRefBase { - constructor(type: string, id: string) { - this.toString = () => `routeRef{type=${type},id=${id}}`; - } -} - export class RouteRefImpl - extends RouteRefBase implements RouteRef { readonly [routeRefType] = 'absolute'; - constructor(private readonly config: RouteRefConfig) { - super('absolute', config.title); - } - - 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; @@ -60,14 +57,14 @@ export class RouteRefImpl } get title() { - return this.config.title; + return this.config.title ?? this.id; + } + + toString() { + return `routeRef{type=absolute,id=${this.id}}`; } } -type OptionalParams< - Params extends { [param in string]: string } -> = Params[keyof Params] extends never ? undefined : Params; - export function createRouteRef< // Params is the type that we care about and the one to be embedded in the route ref. // For example, given the params ['name', 'kind'], Params will be {name: string, kind: string} @@ -77,63 +74,33 @@ 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>, - }); -} - -export class ExternalRouteRefImpl< - Params extends AnyParams, - Optional extends boolean - > - extends RouteRefBase - implements ExternalRouteRef { - readonly [routeRefType] = 'external'; - - constructor( - id: string, - readonly params: ParamKeys, - readonly optional: Optional, - ) { - super('external', id); + const id = config.id || config.title; + if (!id) { + throw new Error('RouteRef must be provided a non-empty id'); } -} - -export function createExternalRouteRef< - Params extends { [param in ParamKey]: string }, - Optional extends boolean = false, - ParamKey extends string = never ->(options: { - /** - * An identifier for this route, used to identify it in error messages - */ - id: string; - - /** - * The parameters that will be provided to the external route reference. - */ - params?: ParamKey[]; - - /** - * 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, `useRouteRef` will return `undefined`. - */ - optional?: Optional; -}): ExternalRouteRef, Optional> { - return new ExternalRouteRefImpl, Optional>( - options.id, - (options.params ?? []) as ParamKeys>, - Boolean(options.optional) as Optional, + return new RouteRefImpl( + id, + (config.params ?? []) as ParamKeys>, + config, ); } + +export function isRouteRef( + routeRef: + | RouteRef + | SubRouteRef + | ExternalRouteRef, +): routeRef is RouteRef { + return routeRef[routeRefType] === 'absolute'; +} diff --git a/packages/core-api/src/routing/RouteResolver.test.ts b/packages/core-api/src/routing/RouteResolver.test.ts new file mode 100644 index 0000000000..e5394f6163 --- /dev/null +++ b/packages/core-api/src/routing/RouteResolver.test.ts @@ -0,0 +1,236 @@ +/* + * 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 { createRouteRef } from './RouteRef'; +import { createSubRouteRef } from './SubRouteRef'; +import { createExternalRouteRef } from './ExternalRouteRef'; +import { RouteResolver } from './RouteResolver'; +import { ExternalRouteRef, RouteRef, SubRouteRef } from './types'; + +const element = () => null; +const rest = { element, caseSensitive: false }; + +const ref1 = createRouteRef({ id: 'rr1' }); +const ref2 = createRouteRef({ id: 'rr2', params: ['x'] }); +const ref3 = createRouteRef({ id: 'rr3', params: ['y'] }); +const subRef1 = createSubRouteRef({ + id: 'srr1', + parent: ref1, + path: '/foo', +}); +const subRef2 = createSubRouteRef({ + id: 'srr2', + parent: ref1, + path: '/foo/:a', +}); +const subRef3 = createSubRouteRef({ + id: 'srr3', + parent: ref2, + path: '/bar', +}); +const subRef4 = createSubRouteRef({ + id: 'srr4', + parent: ref2, + path: '/bar/:a', +}); +const externalRef1 = createExternalRouteRef({ id: 'err1' }); +const externalRef2 = createExternalRouteRef({ + id: 'err2', + optional: true, +}); +const externalRef3 = createExternalRouteRef({ id: 'err3', params: ['x'] }); +const externalRef4 = createExternalRouteRef({ + id: 'err4', + optional: true, + params: ['x'], +}); + +describe('RouteResolver', () => { + it('should not resolve anything with an empty resolver', () => { + const r = new RouteResolver(new Map(), new Map(), [], new Map()); + + expect(r.resolve(ref1, '/')?.()).toBe(undefined); + expect(r.resolve(ref2, '/')?.({ x: '1x' })).toBe(undefined); + expect(r.resolve(subRef1, '/')?.()).toBe(undefined); + expect(r.resolve(subRef2, '/')?.({ a: '2a' })).toBe(undefined); + expect(r.resolve(subRef3, '/')?.({ x: '3x' })).toBe(undefined); + expect(r.resolve(subRef4, '/')?.({ x: '4x', a: '4a' })).toBe(undefined); + expect(r.resolve(externalRef1, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef2, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef3, '/')?.({ x: '5x' })).toBe(undefined); + expect(r.resolve(externalRef4, '/')?.({ x: '6x' })).toBe(undefined); + }); + + it('should resolve an absolute route', () => { + const r = new RouteResolver( + new Map([[ref1, '/my-route']]), + new Map(), + [{ routeRefs: new Set([ref1]), path: '/my-route', ...rest }], + new Map(), + ); + + expect(r.resolve(ref1, '/')?.()).toBe('/my-route'); + expect(r.resolve(ref2, '/')?.({ x: '1x' })).toBe(undefined); + expect(r.resolve(subRef1, '/')?.()).toBe('/my-route/foo'); + expect(r.resolve(subRef2, '/')?.({ a: '2a' })).toBe('/my-route/foo/2a'); + expect(r.resolve(subRef3, '/')?.({ x: '3x' })).toBe(undefined); + expect(r.resolve(subRef4, '/')?.({ x: '4x', a: '4a' })).toBe(undefined); + expect(r.resolve(externalRef1, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef2, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef3, '/')?.({ x: '5x' })).toBe(undefined); + expect(r.resolve(externalRef4, '/')?.({ x: '6x' })).toBe(undefined); + }); + + it('should resolve an absolute route with a param and with a parent', () => { + const r = new RouteResolver( + new Map([ + [ref1, '/my-route'], + [ref2, '/my-parent/:x'], + ]), + new Map([[ref2, ref1]]), + [ + { + routeRefs: new Set([ref2]), + path: '/my-parent/:x', + ...rest, + children: [ + { routeRefs: new Set([ref1]), path: '/my-route', ...rest }, + ], + }, + ], + new Map([ + [externalRef1, ref1], + [externalRef3, ref2], + [externalRef4, subRef3], + ]), + ); + + expect(r.resolve(ref1, '/')?.()).toBe('/my-route'); + expect(r.resolve(ref2, '/')?.({ x: '1x' })).toBe('/my-route/my-parent/1x'); + expect(r.resolve(subRef1, '/')?.()).toBe('/my-route/foo'); + expect(r.resolve(subRef2, '/')?.({ a: '2a' })).toBe('/my-route/foo/2a'); + expect(r.resolve(subRef3, '/')?.({ x: '3x' })).toBe( + '/my-route/my-parent/3x/bar', + ); + expect(r.resolve(subRef4, '/')?.({ x: '4x', a: '4a' })).toBe( + '/my-route/my-parent/4x/bar/4a', + ); + expect(r.resolve(externalRef1, '/')?.()).toBe('/my-route'); + expect(r.resolve(externalRef2, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef3, '/')?.({ x: '5x' })).toBe( + '/my-route/my-parent/5x', + ); + expect(r.resolve(externalRef4, '/')?.({ x: '6x' })).toBe( + '/my-route/my-parent/6x/bar', + ); + }); + + it('should resolve an absolute route with multiple parents', () => { + const r = new RouteResolver( + new Map([ + [ref1, '/my-route'], + [ref2, '/my-parent/:x'], + [ref3, '/my-grandparent/:y'], + ]), + new Map([ + [ref1, ref2], + [ref2, ref3], + ]), + [ + { + routeRefs: new Set([ref3]), + path: '/my-grandparent/:y', + ...rest, + children: [ + { + routeRefs: new Set([ref2]), + path: '/my-parent/:x', + ...rest, + children: [ + { routeRefs: new Set([ref1]), path: '/my-route', ...rest }, + ], + }, + ], + }, + ], + new Map([ + [externalRef1, ref1], + [externalRef3, ref2], + [externalRef4, subRef3], + ]), + ); + + const l = '/my-grandparent/my-y/my-parent/my-x'; + expect(r.resolve(ref1, l)?.()).toBe( + '/my-grandparent/my-y/my-parent/my-x/my-route', + ); + expect(() => r.resolve(ref1, '/')?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(ref2, l)?.({ x: '1x' })).toBe( + '/my-grandparent/my-y/my-parent/1x', + ); + expect(r.resolve(ref2, '/my-grandparent/my-y')?.({ x: '1x' })).toBe( + '/my-grandparent/my-y/my-parent/1x', + ); + expect(() => r.resolve(ref2, '/')?.({ x: '1x' })).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(subRef1, l)?.()).toBe( + '/my-grandparent/my-y/my-parent/my-x/my-route/foo', + ); + expect(() => r.resolve(subRef1, '/')?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(subRef2, l)?.({ a: '2a' })).toBe( + '/my-grandparent/my-y/my-parent/my-x/my-route/foo/2a', + ); + expect(() => r.resolve(subRef2, '/')?.({ a: '2a' })).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(subRef3, l)?.({ x: '3x' })).toBe( + '/my-grandparent/my-y/my-parent/3x/bar', + ); + expect(r.resolve(subRef3, '/my-grandparent/my-y')?.({ x: '3x' })).toBe( + '/my-grandparent/my-y/my-parent/3x/bar', + ); + expect(r.resolve(subRef4, l)?.({ x: '4x', a: '4a' })).toBe( + '/my-grandparent/my-y/my-parent/4x/bar/4a', + ); + expect( + r.resolve(subRef4, '/my-grandparent/my-y')?.({ x: '4x', a: '4a' }), + ).toBe('/my-grandparent/my-y/my-parent/4x/bar/4a'); + expect(r.resolve(externalRef1, l)?.()).toBe( + '/my-grandparent/my-y/my-parent/my-x/my-route', + ); + expect(() => r.resolve(externalRef1, '/')?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(externalRef2, l)?.()).toBe(undefined); + expect(r.resolve(externalRef3, l)?.({ x: '5x' })).toBe( + '/my-grandparent/my-y/my-parent/5x', + ); + expect(() => r.resolve(externalRef3, '/')?.({ x: '5x' })).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(externalRef4, l)?.({ x: '6x' })).toBe( + '/my-grandparent/my-y/my-parent/6x/bar', + ); + expect(() => r.resolve(externalRef4, '/')?.({ x: '6x' })).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + }); +}); diff --git a/packages/core-api/src/routing/RouteResolver.ts b/packages/core-api/src/routing/RouteResolver.ts new file mode 100644 index 0000000000..fb44b8c486 --- /dev/null +++ b/packages/core-api/src/routing/RouteResolver.ts @@ -0,0 +1,223 @@ +/* + * 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 { generatePath, matchRoutes } from 'react-router-dom'; +import { + AnyRouteRef, + BackstageRouteObject, + RouteRef, + ExternalRouteRef, + AnyParams, + SubRouteRef, + routeRefType, + RouteFunc, +} from './types'; +import { isRouteRef } from './RouteRef'; +import { isSubRouteRef } from './SubRouteRef'; +import { isExternalRouteRef } from './ExternalRouteRef'; + +// Joins a list of paths together, avoiding trailing and duplicate slashes +function joinPaths(...paths: string[]): string { + const normalized = paths.join('/').replace(/\/\/+/g, '/'); + if (normalized !== '/' && normalized.endsWith('/')) { + return normalized.slice(0, -1); + } + return normalized; +} + +/** + * Resolves the absolute route ref that our target route ref is pointing pointing to, as well + * as the relative target path. + * + * Returns an undefined target ref if one could not be fully resolved. + */ +function resolveTargetRef( + anyRouteRef: AnyRouteRef, + routePaths: Map, + routeBindings: Map, +): readonly [RouteRef | undefined, string] { + // First we figure out which absolute route ref we're dealing with, an if there was an sub route path to append. + // For sub routes it will be the parent path, while for external routes it will be the bound route. + let targetRef: RouteRef; + let subRoutePath = ''; + if (isRouteRef(anyRouteRef)) { + targetRef = anyRouteRef; + } else if (isSubRouteRef(anyRouteRef)) { + targetRef = anyRouteRef.parent; + subRoutePath = anyRouteRef.path; + } else if (isExternalRouteRef(anyRouteRef)) { + const resolvedRoute = routeBindings.get(anyRouteRef); + if (!resolvedRoute) { + return [undefined, '']; + } + if (isRouteRef(resolvedRoute)) { + targetRef = resolvedRoute; + } else if (isSubRouteRef(resolvedRoute)) { + targetRef = resolvedRoute.parent; + subRoutePath = resolvedRoute.path; + } else { + throw new Error( + `ExternalRouteRef was bound to invalid target, ${resolvedRoute}`, + ); + } + } else if (anyRouteRef[routeRefType]) { + throw new Error( + `Unknown or invalid route ref type, ${anyRouteRef[routeRefType]}`, + ); + } else { + throw new Error(`Unknown object passed to useRouteRef, got ${anyRouteRef}`); + } + + // Bail if no absolute path could be resolved + if (!targetRef) { + return [undefined, '']; + } + + // Find the path that our target route is bound to + const resolvedPath = routePaths.get(targetRef); + if (!resolvedPath) { + return [undefined, '']; + } + + // SubRouteRefs join the path from the parent route with its own path + const targetPath = joinPaths(resolvedPath, subRoutePath); + return [targetRef, targetPath]; +} + +/** + * Resolves the complete base path for navigating to the target RouteRef. + */ +function resolveBasePath( + targetRef: RouteRef, + sourceLocation: Parameters[1], + routePaths: Map, + routeParents: Map, + routeObjects: BackstageRouteObject[], +) { + // While traversing the app element tree we build up the routeObjects structure + // used here. It is the same kind of structure that react-router creates, with the + // addition that associated route refs are stored throughout the tree. This lets + // us look up all route refs that can be reached from our source location. + // Because of the similar route object structure, we can use `matchRoutes` from + // react-router to do the lookup of our current location. + const match = matchRoutes(routeObjects, sourceLocation) ?? []; + + // While we search for a common routing root between our current location and + // the target route, we build a list of all route refs we find that we need + // to traverse to reach the target. + const refDiffList = Array(); + + let matchIndex = -1; + for ( + let targetSearchRef: RouteRef | undefined = targetRef; + targetSearchRef; + targetSearchRef = routeParents.get(targetSearchRef) + ) { + // The match contains a list of all ancestral route refs present at our current location + // Starting at the desired target ref and traversing back through its parents, we search + // for a target ref that is present in the match for our current location. When a match + // is found it means we have found a common base to resolve the route from. + matchIndex = match.findIndex(m => + (m.route as BackstageRouteObject).routeRefs.has(targetSearchRef!), + ); + if (matchIndex !== -1) { + break; + } + + // Every time we move a step up in the ancestry of the target ref, we add the current ref + // to the diff list, which ends up being the list of route refs to traverse form the common base + // in order to reach our target. + refDiffList.unshift(targetSearchRef); + } + + // If our target route is present in the initial match we need to construct the final path + // from the parent of the matched route segment. That's to allow the caller of the route + // function to supply their own params. + if (refDiffList.length === 0) { + matchIndex -= 1; + } + + // This is the part of the route tree that the target and source locations have in common. + // We re-use the existing pathname directly along with all params. + const parentPath = matchIndex === -1 ? '' : match[matchIndex].pathname; + + // This constructs the mid section of the path using paths resolved from all route refs + // we need to traverse to reach our target except for the very last one. None of these + // paths are allowed to require any parameters, as the caller would have no way of knowing + // what parameters those are. + const diffPath = joinPaths( + ...refDiffList.slice(0, -1).map(ref => { + const path = routePaths.get(ref); + if (!path) { + throw new Error(`No path for ${ref}`); + } + if (path.includes(':')) { + throw new Error( + `Cannot route to ${targetRef} with parent ${ref} as it has parameters`, + ); + } + return path; + }), + ); + + return parentPath + diffPath; +} + +export class RouteResolver { + constructor( + private readonly routePaths: Map, + private readonly routeParents: Map, + private readonly routeObjects: BackstageRouteObject[], + private readonly routeBindings: Map< + ExternalRouteRef, + RouteRef | SubRouteRef + >, + ) {} + + resolve( + anyRouteRef: + | RouteRef + | SubRouteRef + | ExternalRouteRef, + sourceLocation: Parameters[1], + ): RouteFunc | undefined { + // First figure out what our target absolute ref is, as well as our target path. + const [targetRef, targetPath] = resolveTargetRef( + anyRouteRef, + this.routePaths, + this.routeBindings, + ); + if (!targetRef) { + return undefined; + } + + // Next we figure out the base path, which is the combination of the common parent path + // between our current location and our target location, as well as the additional path + // that is the difference between the parent path and the base of our target location. + const basePath = resolveBasePath( + targetRef, + sourceLocation, + this.routePaths, + this.routeParents, + this.routeObjects, + ); + + const routeFunc: RouteFunc = (...[params]) => { + return basePath + generatePath(targetPath, params); + }; + return routeFunc; + } +} diff --git a/packages/core-api/src/routing/SubRouteRef.test.ts b/packages/core-api/src/routing/SubRouteRef.test.ts new file mode 100644 index 0000000000..1c1a3c1b21 --- /dev/null +++ b/packages/core-api/src/routing/SubRouteRef.test.ts @@ -0,0 +1,134 @@ +/* + * 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, SubRouteRef } from './types'; +import { createSubRouteRef, isSubRouteRef } from './SubRouteRef'; +import { createRouteRef, isRouteRef } from './RouteRef'; +import { isExternalRouteRef } from './ExternalRouteRef'; + +const parent = createRouteRef({ id: 'parent' }); +const parentX = createRouteRef({ id: 'parent-x', params: ['x'] }); + +describe('SubRouteRef', () => { + it('should be created', () => { + const routeRef: SubRouteRef = createSubRouteRef({ + parent, + id: 'my-route-ref', + path: '/foo', + }); + 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(isRouteRef(routeRef)).toBe(false); + expect(isSubRouteRef(routeRef)).toBe(true); + expect(isExternalRouteRef(routeRef)).toBe(false); + + expect(isRouteRef({} as SubRouteRef)).toBe(false); + }); + + it('should be created with params', () => { + const routeRef: SubRouteRef<{ bar: string }> = createSubRouteRef({ + parent, + id: 'my-other-route-ref', + path: '/foo/:bar', + }); + expect(routeRef.path).toBe('/foo/:bar'); + expect(routeRef.parent).toBe(parent); + expect(routeRef.params).toEqual(['bar']); + }); + + it('should be created with merged params', () => { + const routeRef: SubRouteRef<{ + x: string; + y: string; + z: string; + }> = createSubRouteRef({ + parent: parentX, + id: 'my-other-route-ref', + path: '/foo/:y/:z', + }); + expect(routeRef.path).toBe('/foo/:y/:z'); + expect(routeRef.parent).toBe(parentX); + expect(routeRef.params).toEqual(['x', 'y', 'z']); + }); + + it('should be created with params from parent', () => { + const routeRef: SubRouteRef<{ x: string }> = createSubRouteRef({ + parent: parentX, + id: 'my-other-route-ref', + path: '/foo/bar', + }); + expect(routeRef.path).toBe('/foo/bar'); + expect(routeRef.parent).toBe(parentX); + expect(routeRef.params).toEqual(['x']); + }); + + it.each([ + ['foo', "SubRouteRef path must start with '/', got 'foo'"], + [':foo', "SubRouteRef path must start with '/', got ':foo'"], + ['', "SubRouteRef path must start with '/', got ''"], + ['/', "SubRouteRef path must not end with '/', got '/'"], + ['/foo/', "SubRouteRef path must not end with '/', got '/foo/'"], + ['/foo/:x', 'SubRouteRef may not have params that overlap with its parent'], + ['/:/foo', "SubRouteRef path has invalid param, got ''"], + ['/:inva:lid/foo', "SubRouteRef path has invalid param, got 'inva:lid'"], + ['/:inva=lid/foo', "SubRouteRef path has invalid param, got 'inva=lid'"], + ])('should throw if path is invalid, %s', (path, message) => { + expect(() => + createSubRouteRef({ path, parent: parentX, id: path }), + ).toThrow(message); + }); + + it('should properly infer and parse path parameters', () => { + function validateType(_ref: SubRouteRef) {} + + const _1 = createSubRouteRef({ id: '1', parent, path: '/foo/bar' }); + // @ts-expect-error + validateType<{ x: string }>(_1); + validateType(_1); + + const _2 = createSubRouteRef({ id: '2', parent, path: '/foo/:x/:y' }); + // @ts-expect-error + validateType(_2); + // @ts-expect-error + validateType<{ x: string; z: string }>(_2); + // @ts-expect-error + validateType<{ y: 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 = createSubRouteRef({ id: '3', parent: parentX, path: '/foo' }); + // @ts-expect-error + validateType(_3); + // @ts-expect-error + validateType<{ y: string }>(_3); + validateType<{ x: string }>(_3); + + const _4 = createSubRouteRef({ id: '4', parent: parentX, path: '/foo/:y' }); + // @ts-expect-error + validateType(_4); + // @ts-expect-error + validateType<{ x: string; z: string }>(_4); + // @ts-expect-error + validateType<{ y: string }>(_4); + validateType<{ x: string; y: string }>(_4); + + // To avoid complains about missing expectations and unused vars + expect([_1, _2, _3, _4].join('')).toEqual(expect.any(String)); + }); +}); diff --git a/packages/core-api/src/routing/SubRouteRef.ts b/packages/core-api/src/routing/SubRouteRef.ts new file mode 100644 index 0000000000..7ddfc89c80 --- /dev/null +++ b/packages/core-api/src/routing/SubRouteRef.ts @@ -0,0 +1,128 @@ +/* + * 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, + ExternalRouteRef, + OptionalParams, + ParamKeys, + RouteRef, + routeRefType, + SubRouteRef, +} from './types'; + +// Should match the pattern in react-router +const PARAM_PATTERN = /^\w+$/; + +export class SubRouteRefImpl + implements SubRouteRef { + readonly [routeRefType] = 'sub'; + + constructor( + private readonly id: string, + readonly path: string, + readonly parent: RouteRef, + readonly params: ParamKeys, + ) {} + + toString() { + return `routeRef{type=sub,id=${this.id}}`; + } +} + +// These utility types help us infer a Param object type from a string path +// For example, `/foo/:bar/:baz` inferred to `{ bar: string, baz: string }` +type ParamPart = S extends `:${infer Param}` ? Param : never; +type ParamNames = S extends `${infer Part}/${infer Rest}` + ? ParamPart | ParamNames + : ParamPart; +type PathParams = { [name in ParamNames]: string }; + +/** + * Merges a param object type with with an optional params type into a params object + */ +type MergeParams< + P1 extends { [param in string]: string }, + P2 extends AnyParams +> = (P1[keyof P1] extends never ? {} : P1) & (P2 extends undefined ? {} : P2); + +/** + * Creates a SubRouteRef type given the desired parameters and parent route parameters. + * The parameters types are merged together while ensuring that there is no overlap between the two. + */ +type MakeSubRouteRef< + Params extends { [param in string]: string }, + ParentParams extends AnyParams +> = keyof Params & keyof ParentParams extends never + ? SubRouteRef>> + : never; + +export function createSubRouteRef< + Path extends string, + ParentParams extends AnyParams = never +>(config: { + id: string; + path: Path; + parent: RouteRef; +}): MakeSubRouteRef, ParentParams> { + const { id, path, parent } = config; + type Params = PathParams; + + // Collect runtime parameters from the path, e.g. ['bar', 'baz'] from '/foo/:bar/:baz' + const pathParams = path + .split('/') + .filter(p => p.startsWith(':')) + .map(p => p.substring(1)); + const params = [...parent.params, ...pathParams]; + + if (parent.params.some(p => pathParams.includes(p as string))) { + throw new Error( + 'SubRouteRef may not have params that overlap with its parent', + ); + } + if (!path.startsWith('/')) { + throw new Error(`SubRouteRef path must start with '/', got '${path}'`); + } + if (path.endsWith('/')) { + throw new Error(`SubRouteRef path must not end with '/', got '${path}'`); + } + for (const param of pathParams) { + if (!PARAM_PATTERN.test(param)) { + throw new Error(`SubRouteRef path has invalid param, got '${param}'`); + } + } + + // We ensure that the type of the return type is sane here + const subRouteRef = new SubRouteRefImpl( + id, + 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. + return subRouteRef as any; +} + +export function isSubRouteRef( + routeRef: + | RouteRef + | SubRouteRef + | ExternalRouteRef, +): routeRef is SubRouteRef { + return routeRef[routeRefType] === 'sub'; +} diff --git a/packages/core-api/src/routing/hooks.test.tsx b/packages/core-api/src/routing/hooks.test.tsx index 0ada37b18d..9a65286fd6 100644 --- a/packages/core-api/src/routing/hooks.test.tsx +++ b/packages/core-api/src/routing/hooks.test.tsx @@ -29,18 +29,11 @@ import { routeParentCollector, routeObjectCollector, } from './collectors'; -import { - useRouteRef, - RoutingProvider, - validateRoutes, - RouteFunc, -} from './hooks'; -import { - createRouteRef, - createExternalRouteRef, - RouteRefConfig, -} from './RouteRef'; -import { AnyRouteRef, RouteRef, ExternalRouteRef } from './types'; +import { validateRoutes } from './validation'; +import { useRouteRef, RoutingProvider } from './hooks'; +import { createRouteRef, RouteRefConfig } from './RouteRef'; +import { createExternalRouteRef } from './ExternalRouteRef'; +import { AnyRouteRef, RouteFunc, RouteRef, ExternalRouteRef } from './types'; const mockConfig = (extra?: Partial>) => ({ path: '/unused', diff --git a/packages/core-api/src/routing/hooks.tsx b/packages/core-api/src/routing/hooks.tsx index 4d419485f6..2e65efaba6 100644 --- a/packages/core-api/src/routing/hooks.tsx +++ b/packages/core-api/src/routing/hooks.tsx @@ -15,103 +15,16 @@ */ import React, { createContext, ReactNode, useContext, useMemo } from 'react'; +import { useLocation } from 'react-router-dom'; import { - AnyRouteRef, BackstageRouteObject, RouteRef, ExternalRouteRef, AnyParams, + SubRouteRef, + RouteFunc, } from './types'; -import { generatePath, matchRoutes, useLocation } from 'react-router-dom'; - -// The extra TS magic here is to require a single params argument if the RouteRef -// had at least one param defined, but require 0 arguments if there are no params defined. -// Without this we'd have to pass in empty object to all parameter-less RouteRefs -// just to make TypeScript happy, or we would have to make the argument optional in -// which case you might forget to pass it in when it is actually required. -export type RouteFunc = ( - ...[params]: Params extends undefined ? readonly [] : readonly [Params] -) => string; - -class RouteResolver { - constructor( - private readonly routePaths: Map, - private readonly routeParents: Map, - private readonly routeObjects: BackstageRouteObject[], - private readonly routeBindings: Map, - ) {} - - resolve( - routeRefOrExternalRouteRef: RouteRef | ExternalRouteRef, - sourceLocation: ReturnType, - ): RouteFunc | undefined { - const routeRef = - this.routeBindings.get(routeRefOrExternalRouteRef) ?? - (routeRefOrExternalRouteRef as RouteRef); - - const match = matchRoutes(this.routeObjects, sourceLocation) ?? []; - - // If our route isn't bound to a path we fail the resolution and let the caller decide the failure mode - const lastPath = this.routePaths.get(routeRef); - if (!lastPath) { - return undefined; - } - const targetRefStack = Array(); - let matchIndex = -1; - - for ( - let currentRouteRef: AnyRouteRef | undefined = routeRef; - currentRouteRef; - currentRouteRef = this.routeParents.get(currentRouteRef) - ) { - matchIndex = match.findIndex(m => - (m.route as BackstageRouteObject).routeRefs.has(currentRouteRef!), - ); - if (matchIndex !== -1) { - break; - } - - targetRefStack.unshift(currentRouteRef); - } - - // If our target route is present in the initial match we need to construct the final path - // from the parent of the matched route segment. That's to allow the caller of the route - // function to supply their own params. - if (targetRefStack.length === 0) { - matchIndex -= 1; - } - - // This is the part of the route tree that the target and source locations have in common. - // We re-use the existing pathname directly along with all params. - const parentPath = matchIndex === -1 ? '' : match[matchIndex].pathname; - - // This constructs the mid section of the path using paths resolved from all route refs - // we need to traverse to reach our target except for the very last one. None of these - // paths are allowed to require any parameters, as the called would have no way of knowing - // what parameters those are. - const prefixPath = targetRefStack - .slice(0, -1) - .map(ref => { - const path = this.routePaths.get(ref); - if (!path) { - throw new Error(`No path for ${ref}`); - } - if (path.includes(':')) { - throw new Error( - `Cannot route to ${routeRef} with parent ${ref} as it has parameters`, - ); - } - return path; - }) - .join('/') - .replace(/\/\/+/g, '/'); // Normalize path to not contain repeated /'s - - const routeFunc: RouteFunc = (...[params]) => { - return `${parentPath}${prefixPath}${generatePath(lastPath, params)}`; - }; - return routeFunc; - } -} +import { RouteResolver } from './RouteResolver'; const RoutingContext = createContext(undefined); @@ -119,10 +32,13 @@ export function useRouteRef( routeRef: ExternalRouteRef, ): Optional extends true ? RouteFunc | undefined : RouteFunc; export function useRouteRef( - routeRef: RouteRef, + routeRef: RouteRef | SubRouteRef, ): RouteFunc; export function useRouteRef( - routeRef: RouteRef | ExternalRouteRef, + routeRef: + | RouteRef + | SubRouteRef + | ExternalRouteRef, ): RouteFunc | undefined { const sourceLocation = useLocation(); const resolver = useContext(RoutingContext); @@ -144,10 +60,10 @@ export function useRouteRef( } type ProviderProps = { - routePaths: Map; - routeParents: Map; + routePaths: Map; + routeParents: Map; routeObjects: BackstageRouteObject[]; - routeBindings: Map; + routeBindings: Map; children: ReactNode; }; @@ -170,42 +86,3 @@ export const RoutingProvider = ({ ); }; - -export function validateRoutes( - routePaths: Map, - routeParents: Map, -) { - const notLeafRoutes = new Set(routeParents.values()); - notLeafRoutes.delete(undefined); - - for (const route of routeParents.keys()) { - if (notLeafRoutes.has(route)) { - continue; - } - - let currentRouteRef: AnyRouteRef | undefined = route; - - let fullPath = ''; - while (currentRouteRef) { - const path = routePaths.get(currentRouteRef); - if (!path) { - throw new Error(`No path for ${currentRouteRef}`); - } - fullPath = `${path}${fullPath}`; - currentRouteRef = routeParents.get(currentRouteRef); - } - - const params = fullPath.match(/:(\w+)/g); - if (params) { - for (let j = 0; j < params.length; j++) { - for (let i = j + 1; i < params.length; i++) { - if (params[i] === params[j]) { - throw new Error( - `Parameter ${params[i]} is duplicated in path ${fullPath}`, - ); - } - } - } - } - } -} diff --git a/packages/core-api/src/routing/index.ts b/packages/core-api/src/routing/index.ts index 2a0685f583..c6134cfe4f 100644 --- a/packages/core-api/src/routing/index.ts +++ b/packages/core-api/src/routing/index.ts @@ -22,6 +22,8 @@ export type { ExternalRouteRef, } from './types'; export { FlatRoutes } from './FlatRoutes'; -export { createRouteRef, createExternalRouteRef } from './RouteRef'; +export { createRouteRef } from './RouteRef'; +export { createSubRouteRef } from './SubRouteRef'; +export { createExternalRouteRef } from './ExternalRouteRef'; export type { RouteRefConfig } from './RouteRef'; export { useRouteRef } from './hooks'; diff --git a/packages/core-api/src/routing/types.ts b/packages/core-api/src/routing/types.ts index 69de5345a5..56b991f3af 100644 --- a/packages/core-api/src/routing/types.ts +++ b/packages/core-api/src/routing/types.ts @@ -21,6 +21,18 @@ export type AnyParams = { [param in string]: string } | undefined; export type ParamKeys = keyof Params extends never ? [] : (keyof Params)[]; +export type OptionalParams< + Params extends { [param in string]: string } +> = Params[keyof Params] extends never ? undefined : Params; + +// The extra TS magic here is to require a single params argument if the RouteRef +// had at least one param defined, but require 0 arguments if there are no params defined. +// Without this we'd have to pass in empty object to all parameter-less RouteRefs +// just to make TypeScript happy, or we would have to make the argument optional in +// which case you might forget to pass it in when it is actually required. +export type RouteFunc = ( + ...[params]: Params extends undefined ? readonly [] : readonly [Params] +) => string; export const routeRefType: unique symbol = getGlobalSingleton( 'route-ref-type', @@ -41,6 +53,16 @@ export type RouteRef = { title?: string; }; +export type SubRouteRef = { + readonly [routeRefType]: 'sub'; + + parent: RouteRef; + + path: string; + + params: ParamKeys; +}; + export type ExternalRouteRef< Params extends AnyParams = any, Optional extends boolean = any @@ -52,7 +74,10 @@ export type ExternalRouteRef< optional?: Optional; }; -export type AnyRouteRef = RouteRef | ExternalRouteRef; +export type AnyRouteRef = + | RouteRef + | SubRouteRef + | ExternalRouteRef; // TODO(Rugvip): None of these should be found in the wild anymore, remove in next minor release /** @deprecated */ @@ -68,5 +93,5 @@ export interface BackstageRouteObject { children?: BackstageRouteObject[]; element: React.ReactNode; path: string; - routeRefs: Set; + routeRefs: Set; } diff --git a/packages/core-api/src/routing/validation.ts b/packages/core-api/src/routing/validation.ts new file mode 100644 index 0000000000..2d32471a14 --- /dev/null +++ b/packages/core-api/src/routing/validation.ts @@ -0,0 +1,56 @@ +/* + * 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 { AnyRouteRef } from './types'; + +export function validateRoutes( + routePaths: Map, + routeParents: Map, +) { + const notLeafRoutes = new Set(routeParents.values()); + notLeafRoutes.delete(undefined); + + for (const route of routeParents.keys()) { + if (notLeafRoutes.has(route)) { + continue; + } + + let currentRouteRef: AnyRouteRef | undefined = route; + + let fullPath = ''; + while (currentRouteRef) { + const path = routePaths.get(currentRouteRef); + if (!path) { + throw new Error(`No path for ${currentRouteRef}`); + } + fullPath = `${path}${fullPath}`; + currentRouteRef = routeParents.get(currentRouteRef); + } + + const params = fullPath.match(/:(\w+)/g); + if (params) { + for (let j = 0; j < params.length; j++) { + for (let i = j + 1; i < params.length; i++) { + if (params[i] === params[j]) { + throw new Error( + `Parameter ${params[i]} is duplicated in path ${fullPath}`, + ); + } + } + } + } + } +} diff --git a/packages/test-utils/src/testUtils/appWrappers.tsx b/packages/test-utils/src/testUtils/appWrappers.tsx index 05cd0b8fd4..301366cbbd 100644 --- a/packages/test-utils/src/testUtils/appWrappers.tsx +++ b/packages/test-utils/src/testUtils/appWrappers.tsx @@ -24,6 +24,7 @@ import privateExports, { RouteRef, ExternalRouteRef, attachComponentData, + createRouteRef, } from '@backstage/core-api'; import { RenderResult } from '@testing-library/react'; import { renderWithEffects } from '@backstage/test-utils-core'; @@ -65,6 +66,13 @@ type TestAppOptions = { mountedRoutes?: { [path: string]: RouteRef | ExternalRouteRef }; }; +function isExternalRouteRef( + routeRef: RouteRef | ExternalRouteRef, +): routeRef is ExternalRouteRef { + // TODO(Rugvip): Least ugly workaround for now, but replace :D + return String(routeRef).includes('{type=external,'); +} + /** * Wraps a component inside a Backstage test app, providing a mocked theme * and app context, along with mocked APIs. @@ -77,6 +85,7 @@ export function wrapInTestApp( options: TestAppOptions = {}, ): ReactElement { const { routeEntries = ['/'] } = options; + const boundRoutes = new Map(); const app = new PrivateAppImpl({ apis: [], @@ -99,7 +108,16 @@ export function wrapInTestApp( }, ], defaultApis: mockApis, - bindRoutes: () => {}, + bindRoutes: ({ bind }) => { + for (const [externalRef, absoluteRef] of boundRoutes) { + bind( + { ref: externalRef }, + { + ref: absoluteRef, + }, + ); + } + }, }); let Wrapper: ComponentType; @@ -112,7 +130,16 @@ export function wrapInTestApp( const routeElements = Object.entries(options.mountedRoutes ?? {}).map( ([path, routeRef]) => { const Page = () =>
Mounted at {path}
; - attachComponentData(Page, 'core.mountPoint', routeRef); + + // Allow external route refs to be bound to paths as well, for convenience. + // We work around it by creating and binding an absolute ref to the external one. + if (isExternalRouteRef(routeRef)) { + const absoluteRef = createRouteRef({ id: 'id' }); + boundRoutes.set(routeRef, absoluteRef); + attachComponentData(Page, 'core.mountPoint', absoluteRef); + } else { + attachComponentData(Page, 'core.mountPoint', routeRef); + } return } />; }, );