paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 40 Packages Projects Releases Wiki Activity

Merge pull request #10667 from backstage/rugvip/collectrefactor

Browse Source 
core-app-api: route collection refactoring in preparation for react-router changes
This commit is contained in:
Patrik Oldsberg
2022-04-12 14:46:35 +02:00
committed by GitHub
parent 526f5a6079 3ff2bfb66e
commit edfd12c232
6 changed files with 182 additions and 202 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/eight-hounds-dream.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/core-app-api': patch
---
Refactored the route collection logic to prepare for future changes and avoid duplicate element tree traversal for the analytics context.
packages/core-app-api/src/app/AppManager.tsx
+22 -20
View File
@@ -17,8 +17,10 @@
import { Config } from '@backstage/config';
import React, {
ComponentType,
createContext,
PropsWithChildren,
ReactElement,
useContext,
useEffect,
useMemo,
useState,
@@ -55,9 +57,7 @@ import {
import { pluginCollector } from '../plugins/collectors';
import {
featureFlagCollector,
routeObjectCollector,
routeParentCollector,
routePathCollector,
routingV1Collector,
} from '../routing/collectors';
import { RoutingProvider } from '../routing/RoutingProvider';
import { RouteTracker } from '../routing/RouteTracker';
@@ -79,6 +79,7 @@ import { AppThemeProvider } from './AppThemeProvider';
import { defaultConfigLoader } from './defaultConfigLoader';
import { ApiRegistry } from '../apis/system/ApiRegistry';
import { resolveRouteBindings } from './resolveRouteBindings';
import { BackstageRouteObject } from '../routing/types';
type CompatiblePlugin =
| BackstagePlugin<any, any>
@@ -86,6 +87,10 @@ type CompatiblePlugin =
output(): Array<{ type: 'feature-flag'; name: string }>;
});
const InternalAppContext = createContext<{
routeObjects: BackstageRouteObject[];
}>({ routeObjects: [] });
/**
* Get the app base path from the configured app baseUrl.
*
@@ -205,20 +210,12 @@ export class AppManager implements BackstageApp {
[],
);
const {
routePaths,
routeParents,
routeObjects,
featureFlags,
routeBindings,
} = useMemo(() => {
const { routing, featureFlags, routeBindings } = useMemo(() => {
const result = traverseElementTree({
root: children,
discoverers: [childDiscoverer, routeElementDiscoverer],
collectors: {
routePaths: routePathCollector,
routeParents: routeParentCollector,
routeObjects: routeObjectCollector,
routing: routingV1Collector,
collectedPlugins: pluginCollector,
featureFlags: featureFlagCollector,
},
@@ -241,7 +238,7 @@ export class AppManager implements BackstageApp {
if (!routesHaveBeenValidated) {
routesHaveBeenValidated = true;
validateRouteParameters(routePaths, routeParents);
validateRouteParameters(routing.paths, routing.parents);
validateRouteBindings(
routeBindings,
this.plugins as Iterable<BackstagePlugin<any, any>>,
@@ -304,13 +301,17 @@ export class AppManager implements BackstageApp {
<AppContextProvider appContext={appContext}>
<ThemeProvider>
<RoutingProvider
routePaths={routePaths}
routeParents={routeParents}
routeObjects={routeObjects}
routePaths={routing.paths}
routeParents={routing.parents}
routeObjects={routing.objects}
routeBindings={routeBindings}
basePath={getBasePath(loadedConfig.api)}
>
{children}
<InternalAppContext.Provider
value={{ routeObjects: routing.objects }}
>
{children}
</InternalAppContext.Provider>
</RoutingProvider>
</ThemeProvider>
</AppContextProvider>
@@ -345,6 +346,7 @@ export class AppManager implements BackstageApp {
const AppRouter = ({ children }: PropsWithChildren<{}>) => {
const configApi = useApi(configApiRef);
const mountPath = `${getBasePath(configApi)}/*`;
const { routeObjects } = useContext(InternalAppContext);
// If the app hasn't configured a sign-in page, we just continue as guest.
if (!SignInPageComponent) {
@@ -370,7 +372,7 @@ export class AppManager implements BackstageApp {
return (
<RouterComponent>
<RouteTracker tree={children} />
<RouteTracker routeObjects={routeObjects} />
<Routes>
<Route path={mountPath} element={<>{children}</>} />
</Routes>
@@ -380,7 +382,7 @@ export class AppManager implements BackstageApp {
return (
<RouterComponent>
<RouteTracker tree={children} />
<RouteTracker routeObjects={routeObjects} />
<SignInPageWrapper component={SignInPageComponent}>
<Routes>
<Route path={mountPath} element={<>{children}</>} />
packages/core-app-api/src/routing/RouteTracker.tsx
+6 -19
View File
@@ -14,7 +14,7 @@
* limitations under the License.
*/
import React, { useEffect, useMemo } from 'react';
import React, { useEffect } from 'react';
import { matchRoutes, useLocation } from 'react-router-dom';
import {
useAnalytics,
@@ -22,12 +22,6 @@ import {
CommonAnalyticsContext,
RouteRef,
} from '@backstage/core-plugin-api';
import { routeObjectCollector } from './collectors';
import {
childDiscoverer,
routeElementDiscoverer,
traverseElementTree,
} from '../extensions/traversal';
import { BackstageRouteObject } from './types';
/**
@@ -97,19 +91,12 @@ const TrackNavigation = ({
* Logs a "navigate" event with appropriate plugin-level analytics context
* attributes each time the user navigates to a page.
*/
export const RouteTracker = ({ tree }: { tree: React.ReactNode }) => {
export const RouteTracker = ({
routeObjects,
}: {
routeObjects: BackstageRouteObject[];
}) => {
const { pathname, search, hash } = useLocation();
// todo(iamEAP): Work this into the existing traversal and make the data
// available on the provider. Then grab from app instance on the router.
const { routeObjects } = useMemo(() => {
return traverseElementTree({
root: tree,
discoverers: [childDiscoverer, routeElementDiscoverer],
collectors: {
routeObjects: routeObjectCollector,
},
});
}, [tree]);
return (
<AnalyticsContext attributes={getExtensionContext(pathname, routeObjects)}>
packages/core-app-api/src/routing/RoutingProvider.test.tsx
+11 -18
View File
@@ -34,11 +34,7 @@ import {
ExternalRouteRef,
} from '@backstage/core-plugin-api';
import { RoutingProvider } from './RoutingProvider';
import {
routePathCollector,
routeParentCollector,
routeObjectCollector,
} from './collectors';
import { routingV1Collector } from './collectors';
import { validateRouteParameters } from './validation';
import { RouteResolver } from './RouteResolver';
import { AnyRouteRef, RouteFunc } from './types';
@@ -135,21 +131,19 @@ function withRoutingProvider(
root: ReactElement,
routeBindings: [ExternalRouteRef, RouteRef][] = [],
) {
const { routePaths, routeParents, routeObjects } = traverseElementTree({
const { routing } = traverseElementTree({
root,
discoverers: [childDiscoverer, routeElementDiscoverer],
collectors: {
routePaths: routePathCollector,
routeParents: routeParentCollector,
routeObjects: routeObjectCollector,
routing: routingV1Collector,
},
});
return (
<RoutingProvider
routePaths={routePaths}
routeParents={routeParents}
routeObjects={routeObjects}
routePaths={routing.paths}
routeParents={routing.parents}
routeObjects={routing.objects}
routeBindings={new Map(routeBindings)}
basePath=""
>
@@ -314,18 +308,17 @@ describe('discovery', () => {
</MemoryRouter>
);
const { routePaths, routeParents } = traverseElementTree({
const { routing } = traverseElementTree({
root,
discoverers: [childDiscoverer, routeElementDiscoverer],
collectors: {
routePaths: routePathCollector,
routeParents: routeParentCollector,
routing: routingV1Collector,
},
});
expect(() => validateRouteParameters(routePaths, routeParents)).toThrow(
'Parameter :id is duplicated in path /foo/:id/bar/:id',
);
expect(() =>
validateRouteParameters(routing.paths, routing.parents),
).toThrow('Parameter :id is duplicated in path /foo/:id/bar/:id');
});
});
packages/core-app-api/src/routing/collectors.test.tsx
+21 -33
View File
@@ -15,11 +15,7 @@
*/
import React, { PropsWithChildren } from 'react';
import {
routePathCollector,
routeParentCollector,
routeObjectCollector,
} from './collectors';
import { routingV1Collector } from './collectors';
import {
traverseElementTree,
@@ -162,30 +158,28 @@ describe('discovery', () => {
</MemoryRouter>
);
const { routes, routeParents, routeObjects } = traverseElementTree({
const { routing } = traverseElementTree({
root,
discoverers: [childDiscoverer, routeElementDiscoverer],
collectors: {
routes: routePathCollector,
routeParents: routeParentCollector,
routeObjects: routeObjectCollector,
routing: routingV1Collector,
},
});
expect(sortedEntries(routes)).toEqual([
expect(sortedEntries(routing.paths)).toEqual([
[ref1, '/foo'],
[ref2, '/bar/:id'],
[ref3, '/baz'],
[ref4, '/divsoup'],
[ref5, '/blop'],
]);
expect(sortedEntries(routeParents)).toEqual([
expect(sortedEntries(routing.parents)).toEqual([
[ref1, undefined],
[ref2, ref1],
[ref3, ref2],
[ref4, undefined],
[ref5, ref1],
]);
expect(routeObjects).toEqual([
expect(routing.objects).toEqual([
routeObj(
'/foo',
[ref1],
@@ -220,22 +214,21 @@ describe('discovery', () => {
</MemoryRouter>
);
const { routes, routeParents } = traverseElementTree({
const { routing } = traverseElementTree({
root,
discoverers: [childDiscoverer, routeElementDiscoverer],
collectors: {
routes: routePathCollector,
routeParents: routeParentCollector,
routing: routingV1Collector,
},
});
expect(sortedEntries(routes)).toEqual([
expect(sortedEntries(routing.paths)).toEqual([
[ref1, '/foo'],
[ref2, '/bar/:id'],
[ref3, '/baz'],
[ref4, '/divsoup'],
[ref5, '/blop'],
]);
expect(sortedEntries(routeParents)).toEqual([
expect(sortedEntries(routing.parents)).toEqual([
[ref1, undefined],
[ref2, ref1],
[ref3, undefined],
@@ -266,30 +259,28 @@ describe('discovery', () => {
</MemoryRouter>
);
const { routes, routeParents, routeObjects } = traverseElementTree({
const { routing } = traverseElementTree({
root,
discoverers: [childDiscoverer, routeElementDiscoverer],
collectors: {
routes: routePathCollector,
routeParents: routeParentCollector,
routeObjects: routeObjectCollector,
routing: routingV1Collector,
},
});
expect(sortedEntries(routes)).toEqual([
expect(sortedEntries(routing.paths)).toEqual([
[ref1, '/foo'],
[ref2, '/foo'],
[ref3, '/bar'],
[ref4, '/baz'],
[ref5, '/baz'],
]);
expect(sortedEntries(routeParents)).toEqual([
expect(sortedEntries(routing.parents)).toEqual([
[ref1, undefined],
[ref2, undefined],
[ref3, undefined],
[ref4, ref3],
[ref5, ref3],
]);
expect(routeObjects).toEqual([
expect(routing.objects).toEqual([
routeObj('/foo', [ref1, ref2], [], 'gathered'),
routeObj(
'/bar',
@@ -317,30 +308,28 @@ describe('discovery', () => {
</MemoryRouter>
);
const { routes, routeParents, routeObjects } = traverseElementTree({
const { routing } = traverseElementTree({
root,
discoverers: [childDiscoverer, routeElementDiscoverer],
collectors: {
routes: routePathCollector,
routeParents: routeParentCollector,
routeObjects: routeObjectCollector,
routing: routingV1Collector,
},
});
expect(sortedEntries(routes)).toEqual([
expect(sortedEntries(routing.paths)).toEqual([
[ref1, '/foo'],
[ref2, '/bar'],
[ref3, '/baz'],
[ref4, '/blop'],
[ref5, '/bar'],
]);
expect(sortedEntries(routeParents)).toEqual([
expect(sortedEntries(routing.parents)).toEqual([
[ref1, undefined],
[ref2, ref1],
[ref3, ref1],
[ref4, ref3],
[ref5, ref1],
]);
expect(routeObjects).toEqual([
expect(routing.objects).toEqual([
routeObj(
'/foo',
[ref1],
@@ -376,8 +365,7 @@ describe('discovery', () => {
root,
discoverers: [childDiscoverer, routeElementDiscoverer],
collectors: {
routes: routePathCollector,
routeParents: routeParentCollector,
routing: routingV1Collector,
},
});
}).toThrow('Mounted routable extension must have a path');
packages/core-app-api/src/routing/collectors.tsx
+117 -112
View File
@@ -14,106 +14,16 @@
* limitations under the License.
*/
import { isValidElement, ReactElement, ReactNode } from 'react';
import {
RouteRef,
getComponentData,
BackstagePlugin,
} from '@backstage/core-plugin-api';
import { isValidElement } from 'react';
import { BackstageRouteObject } from './types';
import { createCollector } from '../extensions/traversal';
import { FeatureFlagged, FeatureFlaggedProps } from './FeatureFlagged';
function getMountPoint(node: ReactElement): RouteRef | undefined {
const element: ReactNode = node.props?.element;
let routeRef = getComponentData<RouteRef>(node, 'core.mountPoint');
if (!routeRef && isValidElement(element)) {
routeRef = getComponentData<RouteRef>(element, 'core.mountPoint');
}
return routeRef;
}
export const routePathCollector = createCollector(
() => new Map<RouteRef, string>(),
(acc, node, parent, ctxPath: string | undefined) => {
// The context path is used during mount point gathering to assign the same path
// to all discovered mount points
let currentCtxPath = ctxPath;
if (parent?.props.element === node) {
return currentCtxPath;
}
// Start gathering mount points when we encounter a mount point gathering flag
if (getComponentData<boolean>(node, 'core.gatherMountPoints')) {
const path: string | undefined = node.props?.path;
if (!path) {
throw new Error('Mount point gatherer must have a path');
}
currentCtxPath = path;
}
const routeRef = getMountPoint(node);
if (routeRef) {
let path: string | undefined = node.props?.path;
// If we're gathering mount points we use the context path as out path, unless
// the element has its own path, in which case we use that instead and stop gathering
if (currentCtxPath) {
if (path) {
currentCtxPath = undefined;
} else {
path = currentCtxPath;
}
}
if (!path) {
throw new Error('Mounted routable extension must have a path');
}
acc.set(routeRef, path);
}
return currentCtxPath;
},
);
export const routeParentCollector = createCollector(
() => new Map<RouteRef, RouteRef | undefined>(),
(acc, node, parent, parentRouteRef?: RouteRef | { sticky: RouteRef }) => {
if (parent?.props.element === node) {
return parentRouteRef;
}
let nextParent = parentRouteRef;
const routeRef = getMountPoint(node);
if (routeRef) {
// "sticky" route ref is when we've encountered a mount point gatherer, and we want a
// mount points beneath it to have the same parent, regardless of internal structure
if (parentRouteRef && 'sticky' in parentRouteRef) {
acc.set(routeRef, parentRouteRef.sticky);
// When we encounter a mount point with an explicit path, we stop gathering
// mount points within the children and remove the sticky state
if (node.props?.path) {
nextParent = routeRef;
} else {
nextParent = parentRouteRef;
}
} else {
acc.set(routeRef, parentRouteRef);
nextParent = routeRef;
}
}
// Mount point gatherers are marked as "sticky"
if (getComponentData<boolean>(node, 'core.gatherMountPoints')) {
return { sticky: nextParent };
}
return nextParent;
},
);
// We always add a child that matches all subroutes but without any route refs. This makes
// sure that we're always able to match each route no matter how deep the navigation goes.
// The route resolver then takes care of selecting the most specific match in order to find
@@ -125,21 +35,107 @@ export const MATCH_ALL_ROUTE: BackstageRouteObject = {
routeRefs: new Set(),
};
export const routeObjectCollector = createCollector(
() => Array<BackstageRouteObject>(),
(acc, node, parent, parentObj: BackstageRouteObject | undefined) => {
const parentChildren = parentObj?.children ?? acc;
interface RoutingV1CollectorContext {
path?: string;
routeRef?: RouteRef;
obj?: BackstageRouteObject;
sticky?: boolean;
}
/**
* This is the old V1 logic for collecting the routing model.
* It is being replaced by a new collector because this collection
* logic does not work well beyond react-router v6 beta.
*
* The breaking change is that react-router now requires route
* elements to be `Route` components, and directly renders the
* element prop rather than the `Route` itself. This means it is
* no longer possible to create utility route components. In order
* to fill this gap and in general simplify the route collection
* logic, a new route collection logic is created.
*
* @internal
*/
export const routingV1Collector = createCollector(
() => ({
paths: new Map<RouteRef, string>(),
parents: new Map<RouteRef, RouteRef | undefined>(),
objects: new Array<BackstageRouteObject>(),
}),
(acc, node, parent, ctx?: RoutingV1CollectorContext) => {
// Ignore the top-level element within element props, since it's already been collected.
if (parent?.props.element === node) {
return parentObj;
return ctx;
}
let currentObj = ctx?.obj;
let currentParentRouteRef = ctx?.routeRef;
let sticky = ctx?.sticky;
const path: string | undefined = node.props?.path;
const parentChildren = currentObj?.children ?? acc.objects;
const caseSensitive: boolean = Boolean(node.props?.caseSensitive);
const routeRef = getMountPoint(node);
// The context path is used during mount point gathering to assign the same path
// to all discovered mount points
let currentCtxPath = ctx?.path;
// Start gathering mount points when we encounter a mount point gathering flag
if (getComponentData<boolean>(node, 'core.gatherMountPoints')) {
if (!path) {
throw new Error('Mount point gatherer must have a path');
}
currentCtxPath = path;
}
// Route refs are discovered on the element itself, and on the top-level
// element within the element prop if it exists.
const element = node.props?.element;
let routeRef = getComponentData<RouteRef>(node, 'core.mountPoint');
if (!routeRef && isValidElement(element)) {
routeRef = getComponentData<RouteRef>(element, 'core.mountPoint');
}
if (routeRef) {
// First the path gathering
let routePath: string | undefined = path;
// If we're gathering mount points we use the context path as out path, unless
// the element has its own path, in which case we use that instead and stop gathering
if (currentCtxPath) {
if (routePath) {
currentCtxPath = undefined;
} else {
routePath = currentCtxPath;
}
}
if (!routePath) {
throw new Error('Mounted routable extension must have a path');
}
acc.paths.set(routeRef, routePath);
// Then the parent gathering
// "sticky" route ref is when we've encountered a mount point gatherer, and we want a
// mount points beneath it to have the same parent, regardless of internal structure
if (currentParentRouteRef && sticky) {
acc.parents.set(routeRef, currentParentRouteRef);
// When we encounter a mount point with an explicit path, we stop gathering
// mount points within the children and remove the sticky state
if (path) {
currentParentRouteRef = routeRef;
sticky = false;
}
} else {
acc.parents.set(routeRef, currentParentRouteRef);
currentParentRouteRef = routeRef;
}
// Then construct the objects
if (path) {
const newObject: BackstageRouteObject = {
currentObj = {
caseSensitive,
path,
element: 'mounted',
@@ -150,11 +146,14 @@ export const routeObjectCollector = createCollector(
'core.plugin',
),
};
parentChildren.push(newObject);
return newObject;
parentChildren.push(currentObj);
} else {
currentObj?.routeRefs.add(routeRef);
}
}
parentObj?.routeRefs.add(routeRef);
if (getComponentData<boolean>(node, 'core.gatherMountPoints')) {
sticky = true;
}
const isGatherer = getComponentData<boolean>(
@@ -165,19 +164,25 @@ export const routeObjectCollector = createCollector(
if (!path) {
throw new Error('Mount point gatherer must have a path');
}
const newObject: BackstageRouteObject = {
caseSensitive,
path,
element: 'gathered',
routeRefs: new Set(),
children: [MATCH_ALL_ROUTE],
plugin: parentObj?.plugin,
};
parentChildren.push(newObject);
return newObject;
if (!routeRef) {
currentObj = {
caseSensitive,
path,
element: 'gathered',
routeRefs: new Set(),
children: [MATCH_ALL_ROUTE],
plugin: ctx?.obj?.plugin,
};
parentChildren.push(currentObj);
}
}
return parentObj;
return {
obj: currentObj,
path: currentCtxPath,
routeRef: currentParentRouteRef,
sticky,
};
},
);