From 30a115d39231cf774139d731d8f983281e3e8781 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 30 Nov 2020 17:52:41 +0100 Subject: [PATCH] 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; +}