From 3e7378953e1682dd15d94610e519e9d79823f854 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 1 Dec 2020 18:39:23 +0100 Subject: [PATCH] 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; }