From 5596e8698e64ed59dbe0237a4e0b41c67e6f524d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 27 Nov 2020 14:47:28 +0100 Subject: [PATCH 01/14] core-api: add basic useRouteRef and RouteResolver without support for params Co-authored-by: blam --- packages/core-api/src/routing/hooks.test.tsx | 87 ++++++++++++++++++++ packages/core-api/src/routing/hooks.tsx | 75 +++++++++++++++++ 2 files changed, 162 insertions(+) create mode 100644 packages/core-api/src/routing/hooks.test.tsx create mode 100644 packages/core-api/src/routing/hooks.tsx diff --git a/packages/core-api/src/routing/hooks.test.tsx b/packages/core-api/src/routing/hooks.test.tsx new file mode 100644 index 0000000000..c08891c8cc --- /dev/null +++ b/packages/core-api/src/routing/hooks.test.tsx @@ -0,0 +1,87 @@ +/* + * 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 } from '@testing-library/react'; +import React, { PropsWithChildren } from 'react'; +import { MemoryRouter, Routes } from 'react-router-dom'; +import { createRoutableExtension } from '../extensions'; +import { + childDiscoverer, + routeElementDiscoverer, + traverseElementTree, +} from '../extensions/traversal'; +import { createPlugin } from '../plugin'; +import { routeCollector, routeParentCollector } from './collectors'; +import { useRouteRef, RoutingProvider } from './hooks'; +import { createRouteRef } from './RouteRef'; + +const mockConfig = () => ({ path: '/unused', title: 'Unused' }); +const MockComponent = ({ children }: PropsWithChildren<{}>) => <>{children}; + +const plugin = createPlugin({ id: 'my-plugin' }); + +const ref1 = createRouteRef(mockConfig()); +const ref2 = createRouteRef(mockConfig()); +const ref3 = createRouteRef(mockConfig()); + +const MockRouteSource = () => { + const routeFunc = useRouteRef(ref2); + expect(routeFunc?.()).toBe('/foo/bar'); + return null; +}; + +const Extension1 = plugin.provide( + createRoutableExtension({ component: MockComponent, mountPoint: ref1 }), +); +const Extension2 = plugin.provide( + createRoutableExtension({ component: MockRouteSource, mountPoint: ref2 }), +); +const Extension3 = plugin.provide( + createRoutableExtension({ component: MockComponent, mountPoint: ref3 }), +); + +describe('discovery', () => { + it('should handle all react router Route patterns', () => { + const root = ( + + + + + + + + + + ); + + const { routes, routeParents } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routes: routeCollector, + routeParents: routeParentCollector, + }, + }); + + render( + + {root} + , + ); + + expect.assertions(2); + }); +}); diff --git a/packages/core-api/src/routing/hooks.tsx b/packages/core-api/src/routing/hooks.tsx new file mode 100644 index 0000000000..8a2aa72c92 --- /dev/null +++ b/packages/core-api/src/routing/hooks.tsx @@ -0,0 +1,75 @@ +/* + * 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 React, { createContext, ReactNode, useContext } from 'react'; +import { RouteRef } from './types'; + +export type RouteFunc = () => string; + +class RouteResolver { + constructor( + private readonly routes: Map, + private readonly routeParents: Map, + ) {} + + resolve(routeRef: RouteRef): RouteFunc { + let currentRouteRef: RouteRef | undefined = routeRef; + let fullPath = ''; + + while (currentRouteRef) { + const path = this.routes.get(currentRouteRef); + if (!path) { + throw new Error(`No path for ${currentRouteRef}`); + } + fullPath = `${path}${fullPath}`; + currentRouteRef = this.routeParents.get(currentRouteRef); + } + + return () => { + return fullPath; + }; + } +} + +const RoutingContext = createContext(undefined); + +export function useRouteRef(routeRef: RouteRef): RouteFunc | undefined { + const resolver = useContext(RoutingContext); + if (!resolver) { + throw new Error('No route resolver found in context'); + } + + return resolver.resolve(routeRef); +} + +type ProviderProps = { + routes: Map; + routeParents: Map; + children: ReactNode; +}; + +export const RoutingProvider = ({ + routes, + routeParents, + children, +}: ProviderProps) => { + const resolver = new RouteResolver(routes, routeParents); + return ( + + {children} + + ); +}; From 57ce2b2dc5adc04fb5d507e1bef683a7d58fc3fd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 27 Nov 2020 15:49:33 +0100 Subject: [PATCH 02/14] core-api: fix accidental global singletons in collectors Co-authored-by: blam --- .../src/extensions/traversal.test.tsx | 26 ++++++++++++------- packages/core-api/src/extensions/traversal.ts | 4 +-- packages/core-api/src/plugin/collectors.ts | 2 +- packages/core-api/src/routing/collectors.tsx | 4 +-- 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/packages/core-api/src/extensions/traversal.test.tsx b/packages/core-api/src/extensions/traversal.test.tsx index a69ee9de77..38571fdcc7 100644 --- a/packages/core-api/src/extensions/traversal.test.tsx +++ b/packages/core-api/src/extensions/traversal.test.tsx @@ -41,11 +41,14 @@ describe('discovery', () => { root, discoverers: [childDiscoverer], collectors: { - names: createCollector(Array(), (acc, el) => { - if (typeof el.type === 'string') { - acc.push(el.type); - } - }), + names: createCollector( + () => Array(), + (acc, el) => { + if (typeof el.type === 'string') { + acc.push(el.type); + } + }, + ), }, }); @@ -85,11 +88,14 @@ describe('discovery', () => { ), ], collectors: { - names: createCollector(Array(), (acc, el) => { - if (typeof el.type === 'string') { - acc.push(el.type); - } - }), + names: createCollector( + () => Array(), + (acc, el) => { + if (typeof el.type === 'string') { + acc.push(el.type); + } + }, + ), }, }); diff --git a/packages/core-api/src/extensions/traversal.ts b/packages/core-api/src/extensions/traversal.ts index a1fbdab25a..054f23daca 100644 --- a/packages/core-api/src/extensions/traversal.ts +++ b/packages/core-api/src/extensions/traversal.ts @@ -120,10 +120,10 @@ export function traverseElementTree(options: { } export function createCollector( - initialResult: Result, + accumulatorFactory: () => Result, visit: ReturnType>['visit'], ): Collector { - return () => ({ accumulator: initialResult, visit }); + return () => ({ accumulator: accumulatorFactory(), visit }); } export function childDiscoverer(element: ReactElement): ReactNode { diff --git a/packages/core-api/src/plugin/collectors.ts b/packages/core-api/src/plugin/collectors.ts index a222746f9e..3eadfc0185 100644 --- a/packages/core-api/src/plugin/collectors.ts +++ b/packages/core-api/src/plugin/collectors.ts @@ -34,7 +34,7 @@ import { getComponentData } from '../extensions'; import { createCollector } from '../extensions/traversal'; export const pluginCollector = createCollector( - new Set(), + () => new Set(), (acc, node) => { const plugin = getComponentData(node, 'core.plugin'); if (plugin) { diff --git a/packages/core-api/src/routing/collectors.tsx b/packages/core-api/src/routing/collectors.tsx index fab0cbe112..34edd8a0d5 100644 --- a/packages/core-api/src/routing/collectors.tsx +++ b/packages/core-api/src/routing/collectors.tsx @@ -20,7 +20,7 @@ import { getComponentData } from '../extensions'; import { createCollector } from '../extensions/traversal'; export const routeCollector = createCollector( - new Map(), + () => new Map(), (acc, node, parent) => { if (parent.props.element === node) { return; @@ -51,7 +51,7 @@ export const routeCollector = createCollector( ); export const routeParentCollector = createCollector( - new Map(), + () => new Map(), (acc, node, parent, parentRouteRef?: RouteRef) => { if (parent.props.element === node) { return parentRouteRef; From cf2855dc9e1155ec2478e2d5d10f6f1b1662c1a6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 27 Nov 2020 15:50:36 +0100 Subject: [PATCH 03/14] core-api: add initial support for route parameters with useRouteRef Co-authored-by: blam --- packages/core-api/src/routing/hooks.test.tsx | 75 +++++++++++++++++--- packages/core-api/src/routing/hooks.tsx | 7 +- 2 files changed, 70 insertions(+), 12 deletions(-) diff --git a/packages/core-api/src/routing/hooks.test.tsx b/packages/core-api/src/routing/hooks.test.tsx index c08891c8cc..65c7858954 100644 --- a/packages/core-api/src/routing/hooks.test.tsx +++ b/packages/core-api/src/routing/hooks.test.tsx @@ -27,6 +27,7 @@ import { createPlugin } from '../plugin'; import { routeCollector, routeParentCollector } from './collectors'; import { useRouteRef, RoutingProvider } from './hooks'; import { createRouteRef } from './RouteRef'; +import { RouteRef } from './types'; const mockConfig = () => ({ path: '/unused', title: 'Unused' }); const MockComponent = ({ children }: PropsWithChildren<{}>) => <>{children}; @@ -36,11 +37,19 @@ const plugin = createPlugin({ id: 'my-plugin' }); const ref1 = createRouteRef(mockConfig()); const ref2 = createRouteRef(mockConfig()); const ref3 = createRouteRef(mockConfig()); +const ref4 = createRouteRef(mockConfig()); -const MockRouteSource = () => { - const routeFunc = useRouteRef(ref2); - expect(routeFunc?.()).toBe('/foo/bar'); - return null; +const MockRouteSource = (props: { + name: string; + routeRef: RouteRef; + params?: Record; +}) => { + const routeFunc = useRouteRef(props.routeRef); + return ( +
+ Path at {props.name}: {routeFunc?.(props.params)} +
+ ); }; const Extension1 = plugin.provide( @@ -52,18 +61,21 @@ const Extension2 = plugin.provide( const Extension3 = plugin.provide( createRoutableExtension({ component: MockComponent, mountPoint: ref3 }), ); +const Extension4 = plugin.provide( + createRoutableExtension({ component: MockRouteSource, mountPoint: ref4 }), +); describe('discovery', () => { - it('should handle all react router Route patterns', () => { + it('should handle simple routeRef path creation for routeRefs used in other parts of the app', () => { const root = ( - + - + ); @@ -76,12 +88,57 @@ describe('discovery', () => { }, }); - render( + const rendered = render( {root} , ); - expect.assertions(2); + expect(rendered.getByText('Path at inside: /foo/bar')).toBeInTheDocument(); + expect(rendered.getByText('Path at outside: /foo/bar')).toBeInTheDocument(); + }); + + it('should handle routeRefs with parameters', () => { + const root = ( + + + + + + + + + ); + + const { routes, routeParents } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routes: routeCollector, + routeParents: routeParentCollector, + }, + }); + + const rendered = render( + + {root} + , + ); + + expect( + rendered.getByText('Path at inside: /foo/bar/bleb'), + ).toBeInTheDocument(); + expect( + rendered.getByText('Path at outside: /foo/bar/blob'), + ).toBeInTheDocument(); }); }); diff --git a/packages/core-api/src/routing/hooks.tsx b/packages/core-api/src/routing/hooks.tsx index 8a2aa72c92..739d74247c 100644 --- a/packages/core-api/src/routing/hooks.tsx +++ b/packages/core-api/src/routing/hooks.tsx @@ -16,8 +16,9 @@ import React, { createContext, ReactNode, useContext } from 'react'; import { RouteRef } from './types'; +import { generatePath } from 'react-router-dom'; -export type RouteFunc = () => string; +export type RouteFunc = (params?: Record) => string; class RouteResolver { constructor( @@ -38,8 +39,8 @@ class RouteResolver { currentRouteRef = this.routeParents.get(currentRouteRef); } - return () => { - return fullPath; + return (params?: Record) => { + return generatePath(fullPath, params); }; } } From 64993c1dd246776bb9c8ab171359d8ca81c0f56b Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 27 Nov 2020 16:10:19 +0100 Subject: [PATCH 04/14] core-api: add naive implementation of param-relative routing Co-authored-by: blam --- packages/core-api/src/routing/hooks.test.tsx | 40 +++++++++++++++++++- packages/core-api/src/routing/hooks.tsx | 11 +++--- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/packages/core-api/src/routing/hooks.test.tsx b/packages/core-api/src/routing/hooks.test.tsx index 65c7858954..6c38a6e18a 100644 --- a/packages/core-api/src/routing/hooks.test.tsx +++ b/packages/core-api/src/routing/hooks.test.tsx @@ -38,6 +38,7 @@ const ref1 = createRouteRef(mockConfig()); const ref2 = createRouteRef(mockConfig()); const ref3 = createRouteRef(mockConfig()); const ref4 = createRouteRef(mockConfig()); +const ref5 = createRouteRef(mockConfig()); const MockRouteSource = (props: { name: string; @@ -47,7 +48,7 @@ const MockRouteSource = (props: { const routeFunc = useRouteRef(props.routeRef); return (
- Path at {props.name}: {routeFunc?.(props.params)} + Path at {props.name}: {routeFunc(props.params)}
); }; @@ -64,6 +65,9 @@ const Extension3 = plugin.provide( const Extension4 = plugin.provide( createRoutableExtension({ component: MockRouteSource, mountPoint: ref4 }), ); +const Extension5 = plugin.provide( + createRoutableExtension({ component: MockComponent, mountPoint: ref5 }), +); describe('discovery', () => { it('should handle simple routeRef path creation for routeRefs used in other parts of the app', () => { @@ -100,7 +104,7 @@ describe('discovery', () => { it('should handle routeRefs with parameters', () => { const root = ( - + { rendered.getByText('Path at outside: /foo/bar/blob'), ).toBeInTheDocument(); }); + + it('should handle relative routing within parameterized routes', () => { + const root = ( + + + + + + + + + ); + + const { routes, routeParents } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routes: routeCollector, + routeParents: routeParentCollector, + }, + }); + + const rendered = render( + + {root} + , + ); + + expect( + rendered.getByText('Path at inside: /foo/blob/baz'), + ).toBeInTheDocument(); + }); }); diff --git a/packages/core-api/src/routing/hooks.tsx b/packages/core-api/src/routing/hooks.tsx index 739d74247c..821fd6f9de 100644 --- a/packages/core-api/src/routing/hooks.tsx +++ b/packages/core-api/src/routing/hooks.tsx @@ -16,7 +16,7 @@ import React, { createContext, ReactNode, useContext } from 'react'; import { RouteRef } from './types'; -import { generatePath } from 'react-router-dom'; +import { generatePath, useParams } from 'react-router-dom'; export type RouteFunc = (params?: Record) => string; @@ -26,7 +26,7 @@ class RouteResolver { private readonly routeParents: Map, ) {} - resolve(routeRef: RouteRef): RouteFunc { + resolve(routeRef: RouteRef, parentParams: Record): RouteFunc { let currentRouteRef: RouteRef | undefined = routeRef; let fullPath = ''; @@ -40,20 +40,21 @@ class RouteResolver { } return (params?: Record) => { - return generatePath(fullPath, params); + return generatePath(fullPath, { ...params, ...parentParams }); }; } } const RoutingContext = createContext(undefined); -export function useRouteRef(routeRef: RouteRef): RouteFunc | undefined { +export function useRouteRef(routeRef: RouteRef): RouteFunc { const resolver = useContext(RoutingContext); + const params = useParams(); if (!resolver) { throw new Error('No route resolver found in context'); } - return resolver.resolve(routeRef); + return resolver.resolve(routeRef, params); } type ProviderProps = { From 4ef2a8769b4741358cd80112af43a87feeca9acd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 27 Nov 2020 17:26:38 +0100 Subject: [PATCH 05/14] core-api: add validation of mounted route parameter uniqueness Co-authored-by: blam --- packages/core-api/src/routing/hooks.test.tsx | 27 +++++++++++++- packages/core-api/src/routing/hooks.tsx | 39 ++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/packages/core-api/src/routing/hooks.test.tsx b/packages/core-api/src/routing/hooks.test.tsx index 6c38a6e18a..c2b8679e5f 100644 --- a/packages/core-api/src/routing/hooks.test.tsx +++ b/packages/core-api/src/routing/hooks.test.tsx @@ -25,7 +25,7 @@ import { } from '../extensions/traversal'; import { createPlugin } from '../plugin'; import { routeCollector, routeParentCollector } from './collectors'; -import { useRouteRef, RoutingProvider } from './hooks'; +import { useRouteRef, RoutingProvider, validateRoutes } from './hooks'; import { createRouteRef } from './RouteRef'; import { RouteRef } from './types'; @@ -177,4 +177,29 @@ describe('discovery', () => { rendered.getByText('Path at inside: /foo/blob/baz'), ).toBeInTheDocument(); }); + + it('should handle relative routing of parameterized routes with duplicate param names', () => { + const root = ( + + + + + + + + ); + + const { routes, routeParents } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routes: routeCollector, + routeParents: routeParentCollector, + }, + }); + + expect(() => validateRoutes(routes, routeParents)).toThrow( + 'Parameter :id is duplicated in path /foo/:id/bar/:id', + ); + }); }); diff --git a/packages/core-api/src/routing/hooks.tsx b/packages/core-api/src/routing/hooks.tsx index 821fd6f9de..0c36928f10 100644 --- a/packages/core-api/src/routing/hooks.tsx +++ b/packages/core-api/src/routing/hooks.tsx @@ -75,3 +75,42 @@ export const RoutingProvider = ({ ); }; + +export function validateRoutes( + routes: 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: RouteRef | undefined = route; + + let fullPath = ''; + while (currentRouteRef) { + const path = routes.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}`, + ); + } + } + } + } + } +} From db28b524c48da657eec00e1deb92affece47a3df Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 30 Nov 2020 13:53:46 +0100 Subject: [PATCH 06/14] core-api: remove RouteRefRegistry and SubRouteRefs Co-authored-by: blam --- packages/core-api/src/routing/RouteRef.ts | 52 +----- .../src/routing/RouteRefRegistry.test.ts | 105 ------------ .../core-api/src/routing/RouteRefRegistry.ts | 155 ------------------ packages/core-api/src/routing/index.ts | 2 +- packages/core-api/src/routing/types.ts | 11 -- 5 files changed, 3 insertions(+), 322 deletions(-) delete mode 100644 packages/core-api/src/routing/RouteRefRegistry.test.ts delete mode 100644 packages/core-api/src/routing/RouteRefRegistry.ts diff --git a/packages/core-api/src/routing/RouteRef.ts b/packages/core-api/src/routing/RouteRef.ts index c33335cb38..e7160fdef6 100644 --- a/packages/core-api/src/routing/RouteRef.ts +++ b/packages/core-api/src/routing/RouteRef.ts @@ -14,43 +14,9 @@ * limitations under the License. */ -import { - ConcreteRoute, - routeReference, - ReferencedRoute, - resolveRoute, - RouteRefConfig, -} from './types'; -import { generatePath } from 'react-router-dom'; +import { RouteRefConfig } from './types'; -type SubRouteConfig = { - path: string; -}; - -export class SubRouteRef - implements ReferencedRoute { - constructor( - private readonly parent: ConcreteRoute, - private readonly config: SubRouteConfig, - ) {} - - get [routeReference]() { - return this; - } - - link(...args: Args): ConcreteRoute { - return { - [routeReference]: this, - [resolveRoute]: (path: string) => { - const ownPart = generatePath(this.config.path, args[0] ?? {}); - const parentPart = this.parent[resolveRoute](path); - return parentPart + ownPart; - }, - }; - } -} - -export class AbsoluteRouteRef implements ConcreteRoute { +export class AbsoluteRouteRef { constructor(private readonly config: RouteRefConfig) {} get icon() { @@ -65,20 +31,6 @@ export class AbsoluteRouteRef implements ConcreteRoute { get title() { return this.config.title; } - - createSubRoute( - config: SubRouteConfig, - ) { - return new SubRouteRef(this, config); - } - - get [routeReference]() { - return this; - } - - [resolveRoute](path: string) { - return path; - } } export function createRouteRef(config: RouteRefConfig): AbsoluteRouteRef { diff --git a/packages/core-api/src/routing/RouteRefRegistry.test.ts b/packages/core-api/src/routing/RouteRefRegistry.test.ts deleted file mode 100644 index fa1ef584f1..0000000000 --- a/packages/core-api/src/routing/RouteRefRegistry.test.ts +++ /dev/null @@ -1,105 +0,0 @@ -/* - * 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 { RouteRefRegistry } from './RouteRefRegistry'; -import { createRouteRef } from './RouteRef'; - -const dummyConfig = { path: '/', icon: () => null, title: 'my-title' }; -const ref1 = createRouteRef(dummyConfig); -const ref11 = createRouteRef(dummyConfig); -const ref12 = createRouteRef(dummyConfig); -const ref121 = createRouteRef(dummyConfig); -const ref2 = createRouteRef(dummyConfig); -const ref2a = ref2.createSubRoute({ path: '/a' }); -const ref2b = ref2.createSubRoute<{ id: string }>({ path: '/b/:id' }); - -describe('RouteRefRegistry', () => { - it('should be constructed with a root route', () => { - const registry = new RouteRefRegistry(); - expect(registry.resolveRoute([], [])).toBe(''); - }); - - it('should register and resolve some absolute routes', () => { - const registry = new RouteRefRegistry(); - expect(registry.registerRoute([ref1], '1')).toBe(true); - expect(registry.registerRoute([ref1, ref11], '11')).toBe(true); - expect(registry.registerRoute([ref1, ref12], '12')).toBe(true); - expect(registry.registerRoute([ref1, ref12, ref121], '121')).toBe(true); - expect(registry.registerRoute([ref1, ref12, ref121], 'duplicate')).toBe( - false, - ); - expect(registry.registerRoute([ref1, ref12], 'duplicate')).toBe(false); - expect(registry.registerRoute([ref2], '2')).toBe(true); - expect(registry.registerRoute([ref2], 'duplicate')).toBe(false); - expect(registry.registerRoute([ref2], '2')).toBe(true); - - expect(registry.resolveRoute([], [ref1])).toBe('/1'); - expect(registry.resolveRoute([], [ref11])).toBe(undefined); - expect(registry.resolveRoute([], [ref1, ref11])).toBe('/1/11'); - expect(registry.resolveRoute([ref1], [ref11])).toBe('/1/11'); - expect(registry.resolveRoute([ref1], [ref2])).toBe('/2'); - expect(registry.resolveRoute([ref1, ref12, ref121], [])).toBe('/1/12/121'); - expect(registry.resolveRoute([ref1, ref12, ref121], [ref121])).toBe( - '/1/12/121', - ); - expect(registry.resolveRoute([ref1, ref12, ref121], [ref12, ref121])).toBe( - '/1/12/121', - ); - expect(registry.resolveRoute([ref1, ref12, ref121], [ref12])).toBe('/1/12'); - expect(registry.resolveRoute([ref1, ref12, ref121], [ref1])).toBe('/1'); - }); - - it('should register and resolve with sub routes', () => { - const registry = new RouteRefRegistry(); - expect(registry.registerRoute([ref1], '1')).toBe(true); - expect(registry.registerRoute([ref2], '2')).toBe(true); - expect(registry.registerRoute([ref2a], '2')).toBe(true); - expect(registry.registerRoute([ref2a, ref1], '1')).toBe(true); - expect(registry.registerRoute([ref2a, ref2], '2')).toBe(true); - expect(registry.registerRoute([ref2b], '2')).toBe(true); - expect(registry.registerRoute([ref2b, ref1], '1')).toBe(true); - expect(registry.registerRoute([ref2b, ref2], '2')).toBe(true); - - expect(registry.resolveRoute([], [ref1])).toBe('/1'); - expect(registry.resolveRoute([], [ref2])).toBe('/2'); - expect(registry.resolveRoute([], [ref2a.link(), ref1])).toBe('/2/a/1'); - expect(registry.resolveRoute([], [ref2a.link(), ref2])).toBe('/2/a/2'); - expect(registry.resolveRoute([ref2a.link()], [ref2])).toBe('/2/a/2'); - expect(registry.resolveRoute([ref2a.link(), ref1], [ref2])).toBe('/2/a/2'); - expect(registry.resolveRoute([], [ref2b.link({ id: 'abc' }), ref1])).toBe( - '/2/b/abc/1', - ); - expect(registry.resolveRoute([], [ref2b.link({ id: 'xyz' }), ref2])).toBe( - '/2/b/xyz/2', - ); - expect(registry.resolveRoute([ref2b.link({ id: 'abc' })], [ref2])).toBe( - '/2/b/abc/2', - ); - expect( - registry.resolveRoute([ref2b.link({ id: 'abc' }), ref1], [ref2]), - ).toBe('/2/b/abc/2'); - }); - - it('should throw when registering routes incorrectly', () => { - const registry = new RouteRefRegistry(); - expect(() => { - registry.registerRoute([ref1, ref11], '11'); - }).toThrow('Could not find parent for new routing node'); - expect(() => { - registry.registerRoute([], '11'); - }).toThrow('Must provide at least 1 route to add routing node'); - }); -}); diff --git a/packages/core-api/src/routing/RouteRefRegistry.ts b/packages/core-api/src/routing/RouteRefRegistry.ts deleted file mode 100644 index 7e55cbe8f7..0000000000 --- a/packages/core-api/src/routing/RouteRefRegistry.ts +++ /dev/null @@ -1,155 +0,0 @@ -/* - * 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 { - ConcreteRoute, - routeReference, - resolveRoute, - ReferencedRoute, -} from './types'; - -const rootRoute: ConcreteRoute = { - get [routeReference]() { - return this; - }, - [resolveRoute]: () => '', -}; - -export type RouteRefResolver = { - resolveRoute(from: ConcreteRoute[], to: ConcreteRoute[]): string; -}; - -class Node { - readonly children = new Map(); - - constructor(readonly path: string, readonly parent: Node | undefined) {} - - /** - * Look up a node in the tree given a path. - */ - findNode(routes: ReferencedRoute[]): Node | undefined { - let node = this as Node | undefined; - - for (let i = 0; i < routes.length; i++) { - node = node?.children.get(routes[i][routeReference]); - } - - return node; - } - - /** - * Assigns a path to a leaf node in the routing tree. All ancestor - * nodes of the new leaf node must already exist, or an error will be thrown. - * - * Returns true if the node was added, or false if the node already existed. - */ - addNode(routes: ReferencedRoute[], path: string): boolean { - if (routes.length === 0) { - throw new Error('Must provide at least 1 route to add routing node'); - } - - const parentNode = this.findNode(routes.slice(0, -1)); - if (!parentNode) { - throw new Error('Could not find parent for new routing node'); - } - - const lastRoute = routes[routes.length - 1]; - const lastRouteRef = lastRoute[routeReference]; - - const existingNode = parentNode.children.get(lastRouteRef); - if (existingNode) { - return existingNode.path === path; - } - - parentNode.children.set(lastRouteRef, new Node(path, parentNode)); - return true; - } - - /** - * Resolve an absolute URL that represents this node in the routing tree, using - * using the supplied concrete routes and ancestors of this node. - * - * The length of the provided routes array must match the depth of - * the routing tree that this node is at, or an error will be thrown. - */ - resolve(routes: ConcreteRoute[]) { - const parts = Array(routes.length); - - let node = this as Node | undefined; - for (let i = routes.length - 1; i >= 0; i--) { - if (!node) { - throw new Error('Route resolve missing required parent'); - } - - const route = routes[i]; - parts[i] = route[resolveRoute](node.path); - - node = node.parent; - } - - if (node) { - throw new Error('Route resolve did not reach root'); - } - - return parts.join('/'); - } -} - -/** - * A registry for resolving route refs into concrete string routes. - */ -export class RouteRefRegistry { - private readonly root = new Node('', undefined); - - /** - * Register a new leaf path for a sequence of routes. All ancestor - * routes must already exist. - */ - registerRoute(routes: ReferencedRoute[], path: string): boolean { - return this.root.addNode(routes, path); - } - - /** - * Resolve an absolute path from a point in the routing tree. - * - * The route referenced by `from` must exist, and is the starting - * point for the search, walking up the tree until a subtree that - * matches the routes reference in `to` are found. - * - * If `from` is empty, the search starts and ends at the root node. - * If `to` is empty, the route referenced by `from` will always be returned. - */ - resolveRoute(from: ConcreteRoute[], to: ConcreteRoute[]): string | undefined { - // Keep track of the `from` routes and pop the last ones as we traverse up - // the routing tree. The list of concrete routes that we're passing to - // `node.resolve()` should only include the ones in the resolve path. - const concreteStack = from.slice(); - - let fromNode = this.root.findNode(from); - while (fromNode) { - const resolvedNode = fromNode.findNode(to); - if (resolvedNode) { - return resolvedNode.resolve([rootRoute].concat(concreteStack, to)); - } - - // Search at this level of the tree failed, move up to parent - concreteStack.pop(); - fromNode = fromNode.parent; - } - - return undefined; - } -} diff --git a/packages/core-api/src/routing/index.ts b/packages/core-api/src/routing/index.ts index 29de34ec42..45bb8975db 100644 --- a/packages/core-api/src/routing/index.ts +++ b/packages/core-api/src/routing/index.ts @@ -14,6 +14,6 @@ * limitations under the License. */ -export type { RouteRef, RouteRefConfig, ConcreteRoute } from './types'; +export type { RouteRef, RouteRefConfig } from './types'; export type { MutableRouteRef, AbsoluteRouteRef } from './RouteRef'; export { createRouteRef } from './RouteRef'; diff --git a/packages/core-api/src/routing/types.ts b/packages/core-api/src/routing/types.ts index 162ac74bde..edb1bba196 100644 --- a/packages/core-api/src/routing/types.ts +++ b/packages/core-api/src/routing/types.ts @@ -16,17 +16,6 @@ import { IconComponent } from '../icons'; -export const resolveRoute = Symbol('resolve-route'); -export const routeReference = Symbol('route-ref'); - -export type ReferencedRoute = { - [routeReference]: unknown; -}; - -export type ConcreteRoute = ReferencedRoute & { - [resolveRoute](path: string): string; -}; - export type RouteRef = { // TODO(Rugvip): Remove path, look up via registry instead path: string; From e4782fd3b92b1f56754ba99298081d07e963004c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 30 Nov 2020 14:20:05 +0100 Subject: [PATCH 07/14] core-api: rename routes to routePaths Co-authored-by: blam --- .../core-api/src/routing/collectors.test.tsx | 8 ++-- packages/core-api/src/routing/collectors.tsx | 2 +- packages/core-api/src/routing/hooks.test.tsx | 38 ++++++++++--------- packages/core-api/src/routing/hooks.tsx | 14 +++---- 4 files changed, 33 insertions(+), 29 deletions(-) diff --git a/packages/core-api/src/routing/collectors.test.tsx b/packages/core-api/src/routing/collectors.test.tsx index c2eefa7e82..2595b8c0a0 100644 --- a/packages/core-api/src/routing/collectors.test.tsx +++ b/packages/core-api/src/routing/collectors.test.tsx @@ -15,7 +15,7 @@ */ import React, { PropsWithChildren } from 'react'; -import { routeCollector, routeParentCollector } from './collectors'; +import { routePathCollector, routeParentCollector } from './collectors'; import { traverseElementTree, @@ -96,7 +96,7 @@ describe('discovery', () => { root, discoverers: [childDiscoverer, routeElementDiscoverer], collectors: { - routes: routeCollector, + routes: routePathCollector, routeParents: routeParentCollector, }, }); @@ -147,7 +147,7 @@ describe('discovery', () => { root, discoverers: [childDiscoverer, routeElementDiscoverer], collectors: { - routes: routeCollector, + routes: routePathCollector, routeParents: routeParentCollector, }, }); @@ -184,7 +184,7 @@ describe('discovery', () => { ), discoverers: [childDiscoverer, routeElementDiscoverer], collectors: { - routes: routeCollector, + routes: routePathCollector, routeParents: routeParentCollector, }, }), diff --git a/packages/core-api/src/routing/collectors.tsx b/packages/core-api/src/routing/collectors.tsx index 34edd8a0d5..37f9252329 100644 --- a/packages/core-api/src/routing/collectors.tsx +++ b/packages/core-api/src/routing/collectors.tsx @@ -19,7 +19,7 @@ import { RouteRef } from '../routing/types'; import { getComponentData } from '../extensions'; import { createCollector } from '../extensions/traversal'; -export const routeCollector = createCollector( +export const routePathCollector = createCollector( () => new Map(), (acc, node, parent) => { if (parent.props.element === node) { diff --git a/packages/core-api/src/routing/hooks.test.tsx b/packages/core-api/src/routing/hooks.test.tsx index c2b8679e5f..695cd35907 100644 --- a/packages/core-api/src/routing/hooks.test.tsx +++ b/packages/core-api/src/routing/hooks.test.tsx @@ -24,12 +24,16 @@ import { traverseElementTree, } from '../extensions/traversal'; import { createPlugin } from '../plugin'; -import { routeCollector, routeParentCollector } from './collectors'; +import { routePathCollector, routeParentCollector } from './collectors'; import { useRouteRef, RoutingProvider, validateRoutes } from './hooks'; import { createRouteRef } from './RouteRef'; -import { RouteRef } from './types'; +import { RouteRef, RouteRefConfig } from './types'; -const mockConfig = () => ({ path: '/unused', title: 'Unused' }); +const mockConfig = (extra?: Partial) => ({ + path: '/unused', + title: 'Unused', + ...extra, +}); const MockComponent = ({ children }: PropsWithChildren<{}>) => <>{children}; const plugin = createPlugin({ id: 'my-plugin' }); @@ -83,17 +87,17 @@ describe('discovery', () => { ); - const { routes, routeParents } = traverseElementTree({ + const { routePaths, routeParents } = traverseElementTree({ root, discoverers: [childDiscoverer, routeElementDiscoverer], collectors: { - routes: routeCollector, + routePaths: routePathCollector, routeParents: routeParentCollector, }, }); const rendered = render( - + {root} , ); @@ -123,17 +127,17 @@ describe('discovery', () => { ); - const { routes, routeParents } = traverseElementTree({ + const { routePaths, routeParents } = traverseElementTree({ root, discoverers: [childDiscoverer, routeElementDiscoverer], collectors: { - routes: routeCollector, + routePaths: routePathCollector, routeParents: routeParentCollector, }, }); const rendered = render( - + {root} , ); @@ -146,7 +150,7 @@ describe('discovery', () => { ).toBeInTheDocument(); }); - it('should handle relative routing within parameterized routes', () => { + it('should handle relative routing within parameterized routePaths', () => { const root = ( @@ -158,17 +162,17 @@ describe('discovery', () => { ); - const { routes, routeParents } = traverseElementTree({ + const { routePaths, routeParents } = traverseElementTree({ root, discoverers: [childDiscoverer, routeElementDiscoverer], collectors: { - routes: routeCollector, + routePaths: routePathCollector, routeParents: routeParentCollector, }, }); const rendered = render( - + {root} , ); @@ -178,7 +182,7 @@ describe('discovery', () => { ).toBeInTheDocument(); }); - it('should handle relative routing of parameterized routes with duplicate param names', () => { + it('should handle relative routing of parameterized routePaths with duplicate param names', () => { const root = ( @@ -189,16 +193,16 @@ describe('discovery', () => { ); - const { routes, routeParents } = traverseElementTree({ + const { routePaths, routeParents } = traverseElementTree({ root, discoverers: [childDiscoverer, routeElementDiscoverer], collectors: { - routes: routeCollector, + routePaths: routePathCollector, routeParents: routeParentCollector, }, }); - expect(() => validateRoutes(routes, routeParents)).toThrow( + expect(() => validateRoutes(routePaths, routeParents)).toThrow( 'Parameter :id is duplicated in path /foo/:id/bar/:id', ); }); diff --git a/packages/core-api/src/routing/hooks.tsx b/packages/core-api/src/routing/hooks.tsx index 0c36928f10..22299e3305 100644 --- a/packages/core-api/src/routing/hooks.tsx +++ b/packages/core-api/src/routing/hooks.tsx @@ -22,7 +22,7 @@ export type RouteFunc = (params?: Record) => string; class RouteResolver { constructor( - private readonly routes: Map, + private readonly routePaths: Map, private readonly routeParents: Map, ) {} @@ -31,7 +31,7 @@ class RouteResolver { let fullPath = ''; while (currentRouteRef) { - const path = this.routes.get(currentRouteRef); + const path = this.routePaths.get(currentRouteRef); if (!path) { throw new Error(`No path for ${currentRouteRef}`); } @@ -58,17 +58,17 @@ export function useRouteRef(routeRef: RouteRef): RouteFunc { } type ProviderProps = { - routes: Map; + routePaths: Map; routeParents: Map; children: ReactNode; }; export const RoutingProvider = ({ - routes, + routePaths, routeParents, children, }: ProviderProps) => { - const resolver = new RouteResolver(routes, routeParents); + const resolver = new RouteResolver(routePaths, routeParents); return ( {children} @@ -77,7 +77,7 @@ export const RoutingProvider = ({ }; export function validateRoutes( - routes: Map, + routePaths: Map, routeParents: Map, ) { const notLeafRoutes = new Set(routeParents.values()); @@ -92,7 +92,7 @@ export function validateRoutes( let fullPath = ''; while (currentRouteRef) { - const path = routes.get(currentRouteRef); + const path = routePaths.get(currentRouteRef); if (!path) { throw new Error(`No path for ${currentRouteRef}`); } From 30a115d39231cf774139d731d8f983281e3e8781 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 30 Nov 2020 17:52:41 +0100 Subject: [PATCH 08/14] core-api: add collector for route objects and use react-router matching to implement relative routing Co-authored-by: blam --- packages/core-api/src/routing/RouteRef.ts | 4 + packages/core-api/src/routing/collectors.tsx | 36 +++++- packages/core-api/src/routing/hooks.test.tsx | 121 ++++++++++++++++--- packages/core-api/src/routing/hooks.tsx | 89 +++++++++++--- packages/core-api/src/routing/types.ts | 9 ++ 5 files changed, 222 insertions(+), 37 deletions(-) diff --git a/packages/core-api/src/routing/RouteRef.ts b/packages/core-api/src/routing/RouteRef.ts index e7160fdef6..b9dd7c94fe 100644 --- a/packages/core-api/src/routing/RouteRef.ts +++ b/packages/core-api/src/routing/RouteRef.ts @@ -31,6 +31,10 @@ export class AbsoluteRouteRef { get title() { return this.config.title; } + + toString() { + return `routeRef{path=${this.path}}`; + } } export function createRouteRef(config: RouteRefConfig): AbsoluteRouteRef { diff --git a/packages/core-api/src/routing/collectors.tsx b/packages/core-api/src/routing/collectors.tsx index 37f9252329..38e5376547 100644 --- a/packages/core-api/src/routing/collectors.tsx +++ b/packages/core-api/src/routing/collectors.tsx @@ -15,7 +15,7 @@ */ import { isValidElement, ReactNode } from 'react'; -import { RouteRef } from '../routing/types'; +import { BackstageRouteObject, RouteRef } from '../routing/types'; import { getComponentData } from '../extensions'; import { createCollector } from '../extensions/traversal'; @@ -80,3 +80,37 @@ export const routeParentCollector = createCollector( return nextParent; }, ); + +export const routeObjectCollector = createCollector( + () => Array(), + (acc, node, parent, parentChildArr: BackstageRouteObject[] = acc) => { + if (parent.props.element === node) { + return parentChildArr; + } + + const path: string | undefined = node.props?.path; + const caseSensitive: boolean = Boolean(node.props?.caseSensitive); + const element: ReactNode = node.props?.element; + + let routeRef = getComponentData(node, 'core.mountPoint'); + if (!routeRef && isValidElement(element)) { + routeRef = getComponentData(element, 'core.mountPoint'); + } + if (routeRef) { + const children: BackstageRouteObject[] = []; + if (!path) { + throw new Error(`No path found for mount point ${routeRef}`); + } + parentChildArr.push({ + caseSensitive, + path, + element: null, + routeRef, + children, + }); + return children; + } + + return parentChildArr; + }, +); diff --git a/packages/core-api/src/routing/hooks.test.tsx b/packages/core-api/src/routing/hooks.test.tsx index 695cd35907..68d7abf38a 100644 --- a/packages/core-api/src/routing/hooks.test.tsx +++ b/packages/core-api/src/routing/hooks.test.tsx @@ -24,7 +24,11 @@ import { traverseElementTree, } from '../extensions/traversal'; import { createPlugin } from '../plugin'; -import { routePathCollector, routeParentCollector } from './collectors'; +import { + routePathCollector, + routeParentCollector, + routeObjectCollector, +} from './collectors'; import { useRouteRef, RoutingProvider, validateRoutes } from './hooks'; import { createRouteRef } from './RouteRef'; import { RouteRef, RouteRefConfig } from './types'; @@ -38,23 +42,31 @@ const MockComponent = ({ children }: PropsWithChildren<{}>) => <>{children}; const plugin = createPlugin({ id: 'my-plugin' }); -const ref1 = createRouteRef(mockConfig()); -const ref2 = createRouteRef(mockConfig()); -const ref3 = createRouteRef(mockConfig()); -const ref4 = createRouteRef(mockConfig()); -const ref5 = createRouteRef(mockConfig()); +const ref1 = createRouteRef(mockConfig({ path: '/wat1' })); +const ref2 = createRouteRef(mockConfig({ path: '/wat2' })); +const ref3 = createRouteRef(mockConfig({ path: '/wat3' })); +const ref4 = createRouteRef(mockConfig({ path: '/wat4' })); +const ref5 = createRouteRef(mockConfig({ path: '/wat5' })); const MockRouteSource = (props: { name: string; routeRef: RouteRef; params?: Record; }) => { - const routeFunc = useRouteRef(props.routeRef); - return ( -
- Path at {props.name}: {routeFunc(props.params)} -
- ); + try { + const routeFunc = useRouteRef(props.routeRef); + return ( +
+ Path at {props.name}: {routeFunc(props.params)} +
+ ); + } catch (ex) { + return ( +
+ Error at {props.name}: {ex.message} +
+ ); + } }; const Extension1 = plugin.provide( @@ -87,17 +99,22 @@ describe('discovery', () => { ); - const { routePaths, routeParents } = traverseElementTree({ + const { routePaths, routeParents, routeObjects } = traverseElementTree({ root, discoverers: [childDiscoverer, routeElementDiscoverer], collectors: { routePaths: routePathCollector, routeParents: routeParentCollector, + routeObjects: routeObjectCollector, }, }); const rendered = render( - + {root} , ); @@ -127,17 +144,22 @@ describe('discovery', () => { ); - const { routePaths, routeParents } = traverseElementTree({ + const { routePaths, routeParents, routeObjects } = traverseElementTree({ root, discoverers: [childDiscoverer, routeElementDiscoverer], collectors: { routePaths: routePathCollector, routeParents: routeParentCollector, + routeObjects: routeObjectCollector, }, }); const rendered = render( - + {root} , ); @@ -152,27 +174,38 @@ describe('discovery', () => { it('should handle relative routing within parameterized routePaths', () => { const root = ( - + + + ); - const { routePaths, routeParents } = traverseElementTree({ + const { routePaths, routeParents, routeObjects } = traverseElementTree({ root, discoverers: [childDiscoverer, routeElementDiscoverer], collectors: { routePaths: routePathCollector, routeParents: routeParentCollector, + routeObjects: routeObjectCollector, }, }); const rendered = render( - + {root} , ); @@ -182,6 +215,56 @@ describe('discovery', () => { ).toBeInTheDocument(); }); + it('should throw errors for routing to other routeRefs with unsupported parameters', () => { + const root = ( + + + + + + + + + + + ); + + const { routePaths, routeParents, routeObjects } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routePaths: routePathCollector, + routeParents: routeParentCollector, + routeObjects: routeObjectCollector, + }, + }); + + const rendered = render( + + {root} + , + ); + + expect( + rendered.getByText( + `Error at outsideWithParams: Cannot route to ${ref3} with parent ${ref5} as it has parameters`, + ), + ).toBeInTheDocument(); + expect( + rendered.getByText( + `Error at outsideNoParams: Cannot route to ${ref3} with parent ${ref5} as it has parameters`, + ), + ).toBeInTheDocument(); + }); + it('should handle relative routing of parameterized routePaths with duplicate param names', () => { const root = ( diff --git a/packages/core-api/src/routing/hooks.tsx b/packages/core-api/src/routing/hooks.tsx index 22299e3305..5dbaed4545 100644 --- a/packages/core-api/src/routing/hooks.tsx +++ b/packages/core-api/src/routing/hooks.tsx @@ -14,9 +14,9 @@ * limitations under the License. */ -import React, { createContext, ReactNode, useContext } from 'react'; -import { RouteRef } from './types'; -import { generatePath, useParams } from 'react-router-dom'; +import React, { createContext, ReactNode, useContext, useMemo } from 'react'; +import { BackstageRouteObject, RouteRef } from './types'; +import { generatePath, matchRoutes, useLocation } from 'react-router-dom'; export type RouteFunc = (params?: Record) => string; @@ -24,23 +24,71 @@ class RouteResolver { constructor( private readonly routePaths: Map, private readonly routeParents: Map, + private readonly routeObjects: BackstageRouteObject[], ) {} - resolve(routeRef: RouteRef, parentParams: Record): RouteFunc { - let currentRouteRef: RouteRef | undefined = routeRef; - let fullPath = ''; + resolve( + routeRef: RouteRef, + sourceLocation: ReturnType, + ): RouteFunc { + const match = matchRoutes(this.routeObjects, sourceLocation) ?? []; - while (currentRouteRef) { - const path = this.routePaths.get(currentRouteRef); - if (!path) { - throw new Error(`No path for ${currentRouteRef}`); + const lastPath = this.routePaths.get(routeRef); + if (!lastPath) { + throw new Error(`No path for ${routeRef}`); + } + const targetRefStack = Array(); + let matchIndex = -1; + + for ( + let currentRouteRef: RouteRef | undefined = routeRef; + currentRouteRef; + currentRouteRef = this.routeParents.get(currentRouteRef) + ) { + matchIndex = match.findIndex( + m => (m.route as BackstageRouteObject).routeRef === currentRouteRef, + ); + if (matchIndex !== -1) { + break; } - fullPath = `${path}${fullPath}`; - currentRouteRef = this.routeParents.get(currentRouteRef); + + 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 + return (params?: Record) => { - return generatePath(fullPath, { ...params, ...parentParams }); + return `${parentPath}${prefixPath}${generatePath(lastPath, params)}`; }; } } @@ -48,27 +96,34 @@ class RouteResolver { const RoutingContext = createContext(undefined); export function useRouteRef(routeRef: RouteRef): RouteFunc { + const sourceLocation = useLocation(); const resolver = useContext(RoutingContext); - const params = useParams(); - if (!resolver) { + const routeFunc = useMemo( + () => resolver && resolver.resolve(routeRef, sourceLocation), + [resolver, routeRef, sourceLocation], + ); + + if (!routeFunc) { throw new Error('No route resolver found in context'); } - return resolver.resolve(routeRef, params); + return routeFunc; } type ProviderProps = { routePaths: Map; routeParents: Map; + routeObjects: BackstageRouteObject[]; children: ReactNode; }; export const RoutingProvider = ({ routePaths, routeParents, + routeObjects, children, }: ProviderProps) => { - const resolver = new RouteResolver(routePaths, routeParents); + const resolver = new RouteResolver(routePaths, routeParents, routeObjects); return ( {children} diff --git a/packages/core-api/src/routing/types.ts b/packages/core-api/src/routing/types.ts index edb1bba196..2f13a97a91 100644 --- a/packages/core-api/src/routing/types.ts +++ b/packages/core-api/src/routing/types.ts @@ -28,3 +28,12 @@ export type RouteRefConfig = { icon?: IconComponent; title: string; }; + +// A duplicate of the react-router RouteObject, but with routeRef added +export interface BackstageRouteObject { + caseSensitive: boolean; + children?: BackstageRouteObject[]; + element: React.ReactNode; + path: string; + routeRef: RouteRef; +} From 9a918668948d2092e56420e0bc08e22bb6fdc999 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 30 Nov 2020 17:58:40 +0100 Subject: [PATCH 09/14] core-api: dry up route collectors --- packages/core-api/src/routing/collectors.tsx | 50 +++++++------------- 1 file changed, 16 insertions(+), 34 deletions(-) diff --git a/packages/core-api/src/routing/collectors.tsx b/packages/core-api/src/routing/collectors.tsx index 38e5376547..db6673536d 100644 --- a/packages/core-api/src/routing/collectors.tsx +++ b/packages/core-api/src/routing/collectors.tsx @@ -14,11 +14,22 @@ * limitations under the License. */ -import { isValidElement, ReactNode } from 'react'; +import { isValidElement, ReactElement, ReactNode } from 'react'; import { BackstageRouteObject, RouteRef } from '../routing/types'; import { getComponentData } from '../extensions'; import { createCollector } from '../extensions/traversal'; +function getMountPoint(node: ReactElement): RouteRef | undefined { + const element: ReactNode = node.props?.element; + + let routeRef = getComponentData(node, 'core.mountPoint'); + if (!routeRef && isValidElement(element)) { + routeRef = getComponentData(element, 'core.mountPoint'); + } + + return routeRef; +} + export const routePathCollector = createCollector( () => new Map(), (acc, node, parent) => { @@ -26,26 +37,13 @@ export const routePathCollector = createCollector( return; } - const path: string | undefined = node.props?.path; - const element: ReactNode = node.props?.element; - - const routeRef = getComponentData(node, 'core.mountPoint'); + const routeRef = getMountPoint(node); if (routeRef) { + const path: string | undefined = node.props?.path; if (!path) { throw new Error('Mounted routable extension must have a path'); } acc.set(routeRef, path); - } else if (isValidElement(element)) { - const elementRouteRef = getComponentData( - element, - 'core.mountPoint', - ); - if (elementRouteRef) { - if (!path) { - throw new Error('Route element must have a path'); - } - acc.set(elementRouteRef, path); - } } }, ); @@ -57,24 +55,12 @@ export const routeParentCollector = createCollector( return parentRouteRef; } - const element: ReactNode = node.props?.element; - let nextParent = parentRouteRef; - const routeRef = getComponentData(node, 'core.mountPoint'); + const routeRef = getMountPoint(node); if (routeRef) { acc.set(routeRef, parentRouteRef); nextParent = routeRef; - } else if (isValidElement(element)) { - const elementRouteRef = getComponentData( - element, - 'core.mountPoint', - ); - - if (elementRouteRef) { - acc.set(elementRouteRef, parentRouteRef); - nextParent = elementRouteRef; - } } return nextParent; @@ -90,12 +76,8 @@ export const routeObjectCollector = createCollector( const path: string | undefined = node.props?.path; const caseSensitive: boolean = Boolean(node.props?.caseSensitive); - const element: ReactNode = node.props?.element; - let routeRef = getComponentData(node, 'core.mountPoint'); - if (!routeRef && isValidElement(element)) { - routeRef = getComponentData(element, 'core.mountPoint'); - } + const routeRef = getMountPoint(node); if (routeRef) { const children: BackstageRouteObject[] = []; if (!path) { From ef63f951ea27161ff413ebd56cccbf912b3fe8c8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 30 Nov 2020 18:01:50 +0100 Subject: [PATCH 10/14] core-api: dry up routing hooks tests --- packages/core-api/src/routing/hooks.test.tsx | 104 +++++-------------- 1 file changed, 27 insertions(+), 77 deletions(-) diff --git a/packages/core-api/src/routing/hooks.test.tsx b/packages/core-api/src/routing/hooks.test.tsx index 68d7abf38a..4f79fea303 100644 --- a/packages/core-api/src/routing/hooks.test.tsx +++ b/packages/core-api/src/routing/hooks.test.tsx @@ -15,7 +15,7 @@ */ import { render } from '@testing-library/react'; -import React, { PropsWithChildren } from 'react'; +import React, { PropsWithChildren, ReactElement } from 'react'; import { MemoryRouter, Routes } from 'react-router-dom'; import { createRoutableExtension } from '../extensions'; import { @@ -85,6 +85,28 @@ const Extension5 = plugin.provide( createRoutableExtension({ component: MockComponent, mountPoint: ref5 }), ); +function withRoutingProvider(root: ReactElement) { + const { routePaths, routeParents, routeObjects } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routePaths: routePathCollector, + routeParents: routeParentCollector, + routeObjects: routeObjectCollector, + }, + }); + + return ( + + {root} + + ); +} + describe('discovery', () => { it('should handle simple routeRef path creation for routeRefs used in other parts of the app', () => { const root = ( @@ -99,25 +121,7 @@ describe('discovery', () => { ); - const { routePaths, routeParents, routeObjects } = traverseElementTree({ - root, - discoverers: [childDiscoverer, routeElementDiscoverer], - collectors: { - routePaths: routePathCollector, - routeParents: routeParentCollector, - routeObjects: routeObjectCollector, - }, - }); - - const rendered = render( - - {root} - , - ); + const rendered = render(withRoutingProvider(root)); expect(rendered.getByText('Path at inside: /foo/bar')).toBeInTheDocument(); expect(rendered.getByText('Path at outside: /foo/bar')).toBeInTheDocument(); @@ -144,25 +148,7 @@ describe('discovery', () => { ); - const { routePaths, routeParents, routeObjects } = traverseElementTree({ - root, - discoverers: [childDiscoverer, routeElementDiscoverer], - collectors: { - routePaths: routePathCollector, - routeParents: routeParentCollector, - routeObjects: routeObjectCollector, - }, - }); - - const rendered = render( - - {root} - , - ); + const rendered = render(withRoutingProvider(root)); expect( rendered.getByText('Path at inside: /foo/bar/bleb'), @@ -190,25 +176,7 @@ describe('discovery', () => { ); - const { routePaths, routeParents, routeObjects } = traverseElementTree({ - root, - discoverers: [childDiscoverer, routeElementDiscoverer], - collectors: { - routePaths: routePathCollector, - routeParents: routeParentCollector, - routeObjects: routeObjectCollector, - }, - }); - - const rendered = render( - - {root} - , - ); + const rendered = render(withRoutingProvider(root)); expect( rendered.getByText('Path at inside: /foo/blob/baz'), @@ -233,25 +201,7 @@ describe('discovery', () => { ); - const { routePaths, routeParents, routeObjects } = traverseElementTree({ - root, - discoverers: [childDiscoverer, routeElementDiscoverer], - collectors: { - routePaths: routePathCollector, - routeParents: routeParentCollector, - routeObjects: routeObjectCollector, - }, - }); - - const rendered = render( - - {root} - , - ); + const rendered = render(withRoutingProvider(root)); expect( rendered.getByText( From 3e7378953e1682dd15d94610e519e9d79823f854 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 1 Dec 2020 18:39:23 +0100 Subject: [PATCH 11/14] core-api: add optional typed params for RouteRefs and useRouteRef Co-authored-by: blam --- packages/core-api/src/routing/RouteRef.ts | 22 +++++----- packages/core-api/src/routing/hooks.test.tsx | 17 +++++--- packages/core-api/src/routing/hooks.tsx | 44 +++++++++++++------- packages/core-api/src/routing/index.ts | 1 - packages/core-api/src/routing/types.ts | 10 +++-- 5 files changed, 58 insertions(+), 36 deletions(-) diff --git a/packages/core-api/src/routing/RouteRef.ts b/packages/core-api/src/routing/RouteRef.ts index b9dd7c94fe..41384cafe2 100644 --- a/packages/core-api/src/routing/RouteRef.ts +++ b/packages/core-api/src/routing/RouteRef.ts @@ -14,10 +14,10 @@ * limitations under the License. */ -import { RouteRefConfig } from './types'; +import { RouteRefConfig, RouteRef } from './types'; -export class AbsoluteRouteRef { - constructor(private readonly config: RouteRefConfig) {} +export class AbsoluteRouteRef { + constructor(private readonly config: RouteRefConfig) {} get icon() { return this.config.icon; @@ -32,16 +32,18 @@ export class AbsoluteRouteRef { return this.config.title; } + get P(): Params { + throw new Error("Don't call me maybe"); + } + toString() { return `routeRef{path=${this.path}}`; } } -export function createRouteRef(config: RouteRefConfig): AbsoluteRouteRef { - return new AbsoluteRouteRef(config); +export function createRouteRef< + ParamKeys extends string, + Params extends { [param in string]: string } = { [name in ParamKeys]: string } +>(config: RouteRefConfig): RouteRef { + return new AbsoluteRouteRef(config); } - -// TODO(Rugvip): Added for backwards compatibility, remove once old usage is gone -// We may want to avoid exporting the AbsoluteRouteRef itself though, and consider -// a different model for how to create sub routes, just avoid this -export type MutableRouteRef = AbsoluteRouteRef; diff --git a/packages/core-api/src/routing/hooks.test.tsx b/packages/core-api/src/routing/hooks.test.tsx index 4f79fea303..ae1053b58d 100644 --- a/packages/core-api/src/routing/hooks.test.tsx +++ b/packages/core-api/src/routing/hooks.test.tsx @@ -29,11 +29,16 @@ import { routeParentCollector, routeObjectCollector, } from './collectors'; -import { useRouteRef, RoutingProvider, validateRoutes } from './hooks'; +import { + useRouteRef, + RoutingProvider, + validateRoutes, + RouteFunc, +} from './hooks'; import { createRouteRef } from './RouteRef'; import { RouteRef, RouteRefConfig } from './types'; -const mockConfig = (extra?: Partial) => ({ +const mockConfig = (extra?: Partial>) => ({ path: '/unused', title: 'Unused', ...extra, @@ -48,13 +53,13 @@ const ref3 = createRouteRef(mockConfig({ path: '/wat3' })); const ref4 = createRouteRef(mockConfig({ path: '/wat4' })); const ref5 = createRouteRef(mockConfig({ path: '/wat5' })); -const MockRouteSource = (props: { +const MockRouteSource = (props: { name: string; - routeRef: RouteRef; - params?: Record; + routeRef: RouteRef; + params?: T; }) => { try { - const routeFunc = useRouteRef(props.routeRef); + const routeFunc = useRouteRef(props.routeRef) as RouteFunc; return (
Path at {props.name}: {routeFunc(props.params)} diff --git a/packages/core-api/src/routing/hooks.tsx b/packages/core-api/src/routing/hooks.tsx index 5dbaed4545..563bc84a76 100644 --- a/packages/core-api/src/routing/hooks.tsx +++ b/packages/core-api/src/routing/hooks.tsx @@ -15,33 +15,42 @@ */ import React, { createContext, ReactNode, useContext, useMemo } from 'react'; -import { BackstageRouteObject, RouteRef } from './types'; +import { AnyRouteRef, BackstageRouteObject, RouteRef } from './types'; import { generatePath, matchRoutes, useLocation } from 'react-router-dom'; -export type RouteFunc = (params?: Record) => string; +// 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[keyof Params] extends never + ? readonly [] + : readonly [Params] +) => string; class RouteResolver { constructor( - private readonly routePaths: Map, - private readonly routeParents: Map, + private readonly routePaths: Map, + private readonly routeParents: Map, private readonly routeObjects: BackstageRouteObject[], ) {} - resolve( - routeRef: RouteRef, + resolve( + routeRef: RouteRef, sourceLocation: ReturnType, - ): RouteFunc { + ): RouteFunc { const match = matchRoutes(this.routeObjects, sourceLocation) ?? []; const lastPath = this.routePaths.get(routeRef); if (!lastPath) { throw new Error(`No path for ${routeRef}`); } - const targetRefStack = Array(); + const targetRefStack = Array(); let matchIndex = -1; for ( - let currentRouteRef: RouteRef | undefined = routeRef; + let currentRouteRef: AnyRouteRef | undefined = routeRef; currentRouteRef; currentRouteRef = this.routeParents.get(currentRouteRef) ) { @@ -87,15 +96,18 @@ class RouteResolver { .join('/') .replace(/\/\/+/g, '/'); // Normalize path to not contain repeated /'s - return (params?: Record) => { + const routeFunc: RouteFunc = (...[params]) => { return `${parentPath}${prefixPath}${generatePath(lastPath, params)}`; }; + return routeFunc; } } const RoutingContext = createContext(undefined); -export function useRouteRef(routeRef: RouteRef): RouteFunc { +export function useRouteRef( + routeRef: RouteRef, +): RouteFunc { const sourceLocation = useLocation(); const resolver = useContext(RoutingContext); const routeFunc = useMemo( @@ -111,8 +123,8 @@ export function useRouteRef(routeRef: RouteRef): RouteFunc { } type ProviderProps = { - routePaths: Map; - routeParents: Map; + routePaths: Map; + routeParents: Map; routeObjects: BackstageRouteObject[]; children: ReactNode; }; @@ -132,8 +144,8 @@ export const RoutingProvider = ({ }; export function validateRoutes( - routePaths: Map, - routeParents: Map, + routePaths: Map, + routeParents: Map, ) { const notLeafRoutes = new Set(routeParents.values()); notLeafRoutes.delete(undefined); @@ -143,7 +155,7 @@ export function validateRoutes( continue; } - let currentRouteRef: RouteRef | undefined = route; + let currentRouteRef: AnyRouteRef | undefined = route; let fullPath = ''; while (currentRouteRef) { diff --git a/packages/core-api/src/routing/index.ts b/packages/core-api/src/routing/index.ts index 45bb8975db..1493507f4e 100644 --- a/packages/core-api/src/routing/index.ts +++ b/packages/core-api/src/routing/index.ts @@ -15,5 +15,4 @@ */ export type { RouteRef, RouteRefConfig } from './types'; -export type { MutableRouteRef, AbsoluteRouteRef } from './RouteRef'; export { createRouteRef } from './RouteRef'; diff --git a/packages/core-api/src/routing/types.ts b/packages/core-api/src/routing/types.ts index 2f13a97a91..2726504ad2 100644 --- a/packages/core-api/src/routing/types.ts +++ b/packages/core-api/src/routing/types.ts @@ -16,14 +16,18 @@ import { IconComponent } from '../icons'; -export type RouteRef = { +export type RouteRef = { // TODO(Rugvip): Remove path, look up via registry instead path: string; icon?: IconComponent; title: string; + P: Params; }; -export type RouteRefConfig = { +export type AnyRouteRef = RouteRef; + +export type RouteRefConfig = { + params?: Array; path: string; icon?: IconComponent; title: string; @@ -35,5 +39,5 @@ export interface BackstageRouteObject { children?: BackstageRouteObject[]; element: React.ReactNode; path: string; - routeRef: RouteRef; + routeRef: AnyRouteRef; } From 950a3d3828c20156f397cedf6999d5a2355b703b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 7 Dec 2020 14:38:21 +0100 Subject: [PATCH 12/14] core-api: pass ReactNode as parent to collectors --- packages/core-api/src/extensions/traversal.ts | 8 ++++---- packages/core-api/src/routing/collectors.tsx | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/core-api/src/extensions/traversal.ts b/packages/core-api/src/extensions/traversal.ts index 054f23daca..bdf02d16c8 100644 --- a/packages/core-api/src/extensions/traversal.ts +++ b/packages/core-api/src/extensions/traversal.ts @@ -23,7 +23,7 @@ export type Collector = () => { visit( accumulator: Result, element: ReactElement, - parent: ReactElement, + parent: ReactElement | undefined, context: Context, ): Context; }; @@ -33,7 +33,7 @@ export type Collector = () => { * varying methods to discover child nodes and collect data along the way. */ export function traverseElementTree(options: { - root: ReactElement; + root: ReactNode; discoverers: Discoverer[]; collectors: { [name in keyof Results]: Collector }; }): Results { @@ -52,14 +52,14 @@ export function traverseElementTree(options: { // Internal representation of an element in the tree that we're iterating over type QueueItem = { node: ReactNode; - parent: ReactElement; + parent: ReactElement | undefined; contexts: { [name in string]: unknown }; }; const queue = [ { node: Children.toArray(options.root), - parent: options.root, + parent: undefined, contexts: {}, } as QueueItem, ]; diff --git a/packages/core-api/src/routing/collectors.tsx b/packages/core-api/src/routing/collectors.tsx index db6673536d..e47a59aa85 100644 --- a/packages/core-api/src/routing/collectors.tsx +++ b/packages/core-api/src/routing/collectors.tsx @@ -33,7 +33,7 @@ function getMountPoint(node: ReactElement): RouteRef | undefined { export const routePathCollector = createCollector( () => new Map(), (acc, node, parent) => { - if (parent.props.element === node) { + if (parent?.props.element === node) { return; } @@ -51,7 +51,7 @@ export const routePathCollector = createCollector( export const routeParentCollector = createCollector( () => new Map(), (acc, node, parent, parentRouteRef?: RouteRef) => { - if (parent.props.element === node) { + if (parent?.props.element === node) { return parentRouteRef; } @@ -70,7 +70,7 @@ export const routeParentCollector = createCollector( export const routeObjectCollector = createCollector( () => Array(), (acc, node, parent, parentChildArr: BackstageRouteObject[] = acc) => { - if (parent.props.element === node) { + if (parent?.props.element === node) { return parentChildArr; } From d8d5a17dade9cece3479268122a722ccd191e4ee Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 7 Dec 2020 18:35:11 +0100 Subject: [PATCH 13/14] changeset: add core-api changeset for RouteRef changes --- .changeset/red-worms-fold.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/red-worms-fold.md diff --git a/.changeset/red-worms-fold.md b/.changeset/red-worms-fold.md new file mode 100644 index 0000000000..081f686659 --- /dev/null +++ b/.changeset/red-worms-fold.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-api': minor +--- + +Remove exported `ConcreteRoute`, `MutableRouteRef`, `AbsoluteRouteRef` types, and add as of yet unused `params` option to `createRouteRef`. From 0873ed2d06b885c29a445cfdb2f8425ad18b96ec Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 7 Dec 2020 19:06:24 +0100 Subject: [PATCH 14/14] core-api: switch to deprecating routing types rather than removing --- .changeset/red-worms-fold.md | 8 ++++++-- packages/core-api/src/routing/RouteRef.ts | 10 +++++++-- packages/core-api/src/routing/index.ts | 8 +++++++- packages/core-api/src/routing/types.ts | 25 ++++++++++++++++++++++- 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/.changeset/red-worms-fold.md b/.changeset/red-worms-fold.md index 081f686659..41dcc75c4c 100644 --- a/.changeset/red-worms-fold.md +++ b/.changeset/red-worms-fold.md @@ -1,5 +1,9 @@ --- -'@backstage/core-api': minor +'@backstage/core-api': patch --- -Remove exported `ConcreteRoute`, `MutableRouteRef`, `AbsoluteRouteRef` types, and add as of yet unused `params` option to `createRouteRef`. +Deprecated the `ConcreteRoute`, `MutableRouteRef`, `AbsoluteRouteRef` types and added a new `RouteRef` type as replacement. + +Deprecated and disabled the `createSubRoute` method of `AbsoluteRouteRef`. + +Add an as of yet unused `params` option to `createRouteRef`. diff --git a/packages/core-api/src/routing/RouteRef.ts b/packages/core-api/src/routing/RouteRef.ts index 41384cafe2..0fbc57c6f1 100644 --- a/packages/core-api/src/routing/RouteRef.ts +++ b/packages/core-api/src/routing/RouteRef.ts @@ -32,8 +32,14 @@ export class AbsoluteRouteRef { return this.config.title; } - get P(): Params { - throw new Error("Don't call me maybe"); + /** + * This function should not be used, create a separate RouteRef instead + * @deprecated + */ + createSubRoute(): any { + throw new Error( + 'This method should not be called, create a separate RouteRef instead', + ); } toString() { diff --git a/packages/core-api/src/routing/index.ts b/packages/core-api/src/routing/index.ts index 1493507f4e..d682aa9348 100644 --- a/packages/core-api/src/routing/index.ts +++ b/packages/core-api/src/routing/index.ts @@ -14,5 +14,11 @@ * limitations under the License. */ -export type { RouteRef, RouteRefConfig } from './types'; +export type { + RouteRef, + RouteRefConfig, + AbsoluteRouteRef, + ConcreteRoute, + MutableRouteRef, +} from './types'; export { createRouteRef } from './RouteRef'; diff --git a/packages/core-api/src/routing/types.ts b/packages/core-api/src/routing/types.ts index 2726504ad2..e91ca7ea53 100644 --- a/packages/core-api/src/routing/types.ts +++ b/packages/core-api/src/routing/types.ts @@ -16,16 +16,39 @@ import { IconComponent } from '../icons'; +// @ts-ignore, we're just embedding the Params type for usage in other places export type RouteRef = { // TODO(Rugvip): Remove path, look up via registry instead path: string; icon?: IconComponent; title: string; - P: Params; + /** + * This function should not be used, create a separate RouteRef instead + * @deprecated + */ + createSubRoute(): any; }; export type AnyRouteRef = RouteRef; +/** + * This type should not be used + * @deprecated + */ +export type ConcreteRoute = {}; + +/** + * This type should not be used, use RouteRef instead + * @deprecated + */ +export type AbsoluteRouteRef = RouteRef<{}>; + +/** + * This type should not be used, use RouteRef instead + * @deprecated + */ +export type MutableRouteRef = RouteRef<{}>; + export type RouteRefConfig = { params?: Array; path: string;