frontend-app-api: add collection of route binding errors
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
This commit is contained in:
@@ -109,6 +109,21 @@ export type AppErrorTypes = {
|
||||
node: AppNode;
|
||||
};
|
||||
};
|
||||
ROUTE_DUPLICATE: {
|
||||
context: {
|
||||
routeId: string;
|
||||
};
|
||||
};
|
||||
ROUTE_BINDING_INVALID_VALUE: {
|
||||
context: {
|
||||
routeId: string;
|
||||
};
|
||||
};
|
||||
ROUTE_NOT_FOUND: {
|
||||
context: {
|
||||
routeId: string;
|
||||
};
|
||||
};
|
||||
};
|
||||
|
||||
// @public
|
||||
|
||||
@@ -20,6 +20,18 @@ import {
|
||||
createFrontendPlugin,
|
||||
} from '@backstage/frontend-plugin-api';
|
||||
import { collectRouteIds } from './collectRouteIds';
|
||||
import { createErrorCollector } from '../wiring/createErrorCollector';
|
||||
|
||||
const collector = createErrorCollector();
|
||||
|
||||
afterEach(() => {
|
||||
const errors = collector.collectErrors();
|
||||
if (errors) {
|
||||
throw new Error(
|
||||
`Unexpected errors: ${errors.map(e => e.message).join(', ')}`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
describe('collectRouteIds', () => {
|
||||
it('should assign IDs to routes', () => {
|
||||
@@ -33,13 +45,16 @@ describe('collectRouteIds', () => {
|
||||
/^ExternalRouteRef\{created at '.*collectRouteIds\.test\.ts.*'\}$/,
|
||||
);
|
||||
|
||||
const collected = collectRouteIds([
|
||||
createFrontendPlugin({
|
||||
pluginId: 'test',
|
||||
routes: { ref },
|
||||
externalRoutes: { extRef },
|
||||
}),
|
||||
]);
|
||||
const collected = collectRouteIds(
|
||||
[
|
||||
createFrontendPlugin({
|
||||
pluginId: 'test',
|
||||
routes: { ref },
|
||||
externalRoutes: { extRef },
|
||||
}),
|
||||
],
|
||||
collector,
|
||||
);
|
||||
expect(Object.fromEntries(collected.routes)).toEqual({
|
||||
'test.ref': ref,
|
||||
});
|
||||
@@ -50,4 +65,40 @@ describe('collectRouteIds', () => {
|
||||
expect(String(ref)).toBe('RouteRef{test.ref}');
|
||||
expect(String(extRef)).toBe('ExternalRouteRef{test.extRef}');
|
||||
});
|
||||
|
||||
it('should report duplicate route IDs', () => {
|
||||
const ref = createRouteRef();
|
||||
const extRef = createExternalRouteRef();
|
||||
|
||||
collectRouteIds(
|
||||
[
|
||||
createFrontendPlugin({
|
||||
pluginId: 'test',
|
||||
routes: { 'mid.ref': ref },
|
||||
externalRoutes: { 'mid.extRef': extRef },
|
||||
}),
|
||||
createFrontendPlugin({
|
||||
pluginId: 'test.mid',
|
||||
routes: { ref },
|
||||
externalRoutes: { extRef },
|
||||
}),
|
||||
],
|
||||
collector,
|
||||
);
|
||||
|
||||
expect(collector.collectErrors()).toEqual([
|
||||
{
|
||||
code: 'ROUTE_DUPLICATE',
|
||||
message:
|
||||
"Duplicate route id 'test.mid.ref' encountered while collecting routes",
|
||||
context: { routeId: 'test.mid.ref' },
|
||||
},
|
||||
{
|
||||
code: 'ROUTE_DUPLICATE',
|
||||
message:
|
||||
"Duplicate external route id 'test.mid.extRef' encountered while collecting routes",
|
||||
context: { routeId: 'test.mid.extRef' },
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -30,6 +30,7 @@ import { toInternalExternalRouteRef } from '../../../frontend-plugin-api/src/rou
|
||||
// eslint-disable-next-line @backstage/no-relative-monorepo-imports
|
||||
import { toInternalSubRouteRef } from '../../../frontend-plugin-api/src/routing/SubRouteRef';
|
||||
import { OpaqueFrontendPlugin } from '@internal/frontend';
|
||||
import { ErrorCollector } from '../wiring/createErrorCollector';
|
||||
|
||||
/** @internal */
|
||||
export interface RouteRefsById {
|
||||
@@ -38,7 +39,10 @@ export interface RouteRefsById {
|
||||
}
|
||||
|
||||
/** @internal */
|
||||
export function collectRouteIds(features: FrontendFeature[]): RouteRefsById {
|
||||
export function collectRouteIds(
|
||||
features: FrontendFeature[],
|
||||
collector: ErrorCollector,
|
||||
): RouteRefsById {
|
||||
const routesById = new Map<string, RouteRef | SubRouteRef>();
|
||||
const externalRoutesById = new Map<string, ExternalRouteRef>();
|
||||
|
||||
@@ -50,7 +54,12 @@ export function collectRouteIds(features: FrontendFeature[]): RouteRefsById {
|
||||
for (const [name, ref] of Object.entries(feature.routes)) {
|
||||
const refId = `${feature.id}.${name}`;
|
||||
if (routesById.has(refId)) {
|
||||
throw new Error(`Unexpected duplicate route '${refId}'`);
|
||||
collector.report({
|
||||
code: 'ROUTE_DUPLICATE',
|
||||
message: `Duplicate route id '${refId}' encountered while collecting routes`,
|
||||
context: { routeId: refId },
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
if (isRouteRef(ref)) {
|
||||
@@ -65,7 +74,12 @@ export function collectRouteIds(features: FrontendFeature[]): RouteRefsById {
|
||||
for (const [name, ref] of Object.entries(feature.externalRoutes)) {
|
||||
const refId = `${feature.id}.${name}`;
|
||||
if (externalRoutesById.has(refId)) {
|
||||
throw new Error(`Unexpected duplicate external route '${refId}'`);
|
||||
collector.report({
|
||||
code: 'ROUTE_DUPLICATE',
|
||||
message: `Duplicate external route id '${refId}' encountered while collecting routes`,
|
||||
context: { routeId: refId },
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
const internalRef = toInternalExternalRouteRef(ref);
|
||||
|
||||
@@ -20,6 +20,18 @@ import {
|
||||
} from '@backstage/frontend-plugin-api';
|
||||
import { resolveRouteBindings } from './resolveRouteBindings';
|
||||
import { ConfigReader } from '@backstage/config';
|
||||
import { createErrorCollector } from '../wiring/createErrorCollector';
|
||||
|
||||
const collector = createErrorCollector();
|
||||
|
||||
afterEach(() => {
|
||||
const errors = collector.collectErrors();
|
||||
if (errors) {
|
||||
throw new Error(
|
||||
`Unexpected errors: ${errors.map(e => e.message).join(', ')}`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
const emptyIds = { routes: new Map(), externalRoutes: new Map() };
|
||||
|
||||
@@ -33,23 +45,30 @@ describe('resolveRouteBindings', () => {
|
||||
},
|
||||
new ConfigReader({}),
|
||||
emptyIds,
|
||||
collector,
|
||||
);
|
||||
|
||||
expect(result.get(external.myRoute)).toBe(ref);
|
||||
});
|
||||
|
||||
it('throws on unknown keys', () => {
|
||||
it('reports error on unknown keys', () => {
|
||||
const external = { myRoute: createExternalRouteRef() };
|
||||
const ref = createRouteRef();
|
||||
expect(() =>
|
||||
resolveRouteBindings(
|
||||
({ bind }) => {
|
||||
bind(external, { someOtherRoute: ref } as any);
|
||||
},
|
||||
new ConfigReader({}),
|
||||
emptyIds,
|
||||
),
|
||||
).toThrow('Key someOtherRoute is not an existing external route');
|
||||
resolveRouteBindings(
|
||||
({ bind }) => {
|
||||
bind(external, { someOtherRoute: ref } as any);
|
||||
},
|
||||
new ConfigReader({}),
|
||||
emptyIds,
|
||||
collector,
|
||||
);
|
||||
expect(collector.collectErrors()).toEqual([
|
||||
{
|
||||
code: 'ROUTE_NOT_FOUND',
|
||||
message: 'Key someOtherRoute is not an existing external route',
|
||||
context: { routeId: 'someOtherRoute' },
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('reads bindings from config', () => {
|
||||
@@ -64,6 +83,7 @@ describe('resolveRouteBindings', () => {
|
||||
routes: new Map([['myTarget', myTarget]]),
|
||||
externalRoutes: new Map([['mySource', mySource]]),
|
||||
},
|
||||
collector,
|
||||
);
|
||||
|
||||
expect(result.get(mySource)).toBe(myTarget);
|
||||
@@ -85,6 +105,7 @@ describe('resolveRouteBindings', () => {
|
||||
routes: new Map([['myTarget', myTarget]]),
|
||||
externalRoutes: new Map([['mySource', mySource]]),
|
||||
},
|
||||
collector,
|
||||
).get(mySource),
|
||||
).toBe(undefined);
|
||||
|
||||
@@ -100,57 +121,74 @@ describe('resolveRouteBindings', () => {
|
||||
routes: new Map([['myTarget', myTarget]]),
|
||||
externalRoutes: new Map([['mySource', mySource]]),
|
||||
},
|
||||
collector,
|
||||
).get(mySource),
|
||||
).toBe(myTarget);
|
||||
});
|
||||
|
||||
it('throws on invalid config', () => {
|
||||
it('reports errors on invalid config', () => {
|
||||
expect(() =>
|
||||
resolveRouteBindings(
|
||||
() => {},
|
||||
new ConfigReader({ app: { routes: { bindings: 'derp' } } }),
|
||||
emptyIds,
|
||||
collector,
|
||||
),
|
||||
).toThrow(
|
||||
"Invalid type in config for key 'app.routes.bindings' in 'mock-config', got string, wanted object",
|
||||
);
|
||||
|
||||
expect(() =>
|
||||
resolveRouteBindings(
|
||||
() => {},
|
||||
new ConfigReader({ app: { routes: { bindings: { mySource: true } } } }),
|
||||
emptyIds,
|
||||
),
|
||||
).toThrow(
|
||||
"Invalid config at app.routes.bindings['mySource'], value must be a non-empty string",
|
||||
resolveRouteBindings(
|
||||
() => {},
|
||||
new ConfigReader({ app: { routes: { bindings: { mySource: true } } } }),
|
||||
emptyIds,
|
||||
collector,
|
||||
);
|
||||
expect(collector.collectErrors()).toEqual([
|
||||
{
|
||||
code: 'ROUTE_BINDING_INVALID_VALUE',
|
||||
message:
|
||||
"Invalid config at app.routes.bindings['mySource'], value must be a non-empty string or false",
|
||||
context: { routeId: 'mySource' },
|
||||
},
|
||||
]);
|
||||
|
||||
expect(() =>
|
||||
resolveRouteBindings(
|
||||
() => {},
|
||||
new ConfigReader({
|
||||
app: { routes: { bindings: { mySource: 'myTarget' } } },
|
||||
}),
|
||||
emptyIds,
|
||||
),
|
||||
).toThrow(
|
||||
"Invalid config at app.routes.bindings, 'mySource' is not a valid external route",
|
||||
resolveRouteBindings(
|
||||
() => {},
|
||||
new ConfigReader({
|
||||
app: { routes: { bindings: { mySource: 'myTarget' } } },
|
||||
}),
|
||||
emptyIds,
|
||||
collector,
|
||||
);
|
||||
expect(collector.collectErrors()).toEqual([
|
||||
{
|
||||
code: 'ROUTE_NOT_FOUND',
|
||||
message:
|
||||
"Invalid config at app.routes.bindings, 'mySource' is not a valid external route",
|
||||
context: { routeId: 'mySource' },
|
||||
},
|
||||
]);
|
||||
|
||||
expect(() =>
|
||||
resolveRouteBindings(
|
||||
() => {},
|
||||
new ConfigReader({
|
||||
app: { routes: { bindings: { mySource: 'myTarget' } } },
|
||||
}),
|
||||
{
|
||||
...emptyIds,
|
||||
externalRoutes: new Map([['mySource', createExternalRouteRef()]]),
|
||||
},
|
||||
),
|
||||
).toThrow(
|
||||
"Invalid config at app.routes.bindings['mySource'], 'myTarget' is not a valid route",
|
||||
resolveRouteBindings(
|
||||
() => {},
|
||||
new ConfigReader({
|
||||
app: { routes: { bindings: { mySource: 'myTarget' } } },
|
||||
}),
|
||||
{
|
||||
...emptyIds,
|
||||
externalRoutes: new Map([['mySource', createExternalRouteRef()]]),
|
||||
},
|
||||
collector,
|
||||
);
|
||||
expect(collector.collectErrors()).toEqual([
|
||||
{
|
||||
code: 'ROUTE_NOT_FOUND',
|
||||
message:
|
||||
"Invalid config at app.routes.bindings['mySource'], 'myTarget' is not a valid route",
|
||||
context: { routeId: 'myTarget' },
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('can have default targets, but at the lowest priority', () => {
|
||||
@@ -170,6 +208,7 @@ describe('resolveRouteBindings', () => {
|
||||
() => {},
|
||||
new ConfigReader({}),
|
||||
routesById,
|
||||
collector,
|
||||
);
|
||||
|
||||
expect(result.get(source)).toBe(target1);
|
||||
@@ -181,6 +220,7 @@ describe('resolveRouteBindings', () => {
|
||||
app: { routes: { bindings: { source: 'target2' } } },
|
||||
}),
|
||||
routesById,
|
||||
collector,
|
||||
);
|
||||
|
||||
expect(result.get(source)).toBe(target2);
|
||||
@@ -192,6 +232,7 @@ describe('resolveRouteBindings', () => {
|
||||
},
|
||||
new ConfigReader({}),
|
||||
routesById,
|
||||
collector,
|
||||
);
|
||||
|
||||
expect(result.get(source)).toBe(target2);
|
||||
@@ -210,6 +251,7 @@ describe('resolveRouteBindings', () => {
|
||||
() => {},
|
||||
new ConfigReader({}),
|
||||
routesById,
|
||||
collector,
|
||||
);
|
||||
|
||||
expect(result.get(source)).toBe(target1);
|
||||
@@ -219,6 +261,7 @@ describe('resolveRouteBindings', () => {
|
||||
() => {},
|
||||
new ConfigReader({ app: { routes: { bindings: { source: false } } } }),
|
||||
routesById,
|
||||
collector,
|
||||
);
|
||||
|
||||
expect(result.get(source)).toBe(undefined);
|
||||
|
||||
@@ -20,6 +20,7 @@ import {
|
||||
ExternalRouteRef,
|
||||
} from '@backstage/frontend-plugin-api';
|
||||
import { RouteRefsById } from './collectRouteIds';
|
||||
import { ErrorCollector } from '../wiring/createErrorCollector';
|
||||
import { Config } from '@backstage/config';
|
||||
import { JsonObject } from '@backstage/types';
|
||||
// eslint-disable-next-line @backstage/no-relative-monorepo-imports
|
||||
@@ -80,6 +81,7 @@ export function resolveRouteBindings(
|
||||
bindRoutes: ((context: { bind: CreateAppRouteBinder }) => void) | undefined,
|
||||
config: Config,
|
||||
routesById: RouteRefsById,
|
||||
collector: ErrorCollector,
|
||||
): Map<ExternalRouteRef, RouteRef | SubRouteRef> {
|
||||
const result = new Map<ExternalRouteRef, RouteRef | SubRouteRef>();
|
||||
const disabledExternalRefs = new Set<ExternalRouteRef>();
|
||||
@@ -93,7 +95,12 @@ export function resolveRouteBindings(
|
||||
for (const [key, value] of Object.entries(targetRoutes)) {
|
||||
const externalRoute = externalRoutes[key];
|
||||
if (!externalRoute) {
|
||||
throw new Error(`Key ${key} is not an existing external route`);
|
||||
collector.report({
|
||||
code: 'ROUTE_NOT_FOUND',
|
||||
message: `Key ${key} is not an existing external route`,
|
||||
context: { routeId: String(key) },
|
||||
});
|
||||
continue;
|
||||
}
|
||||
if (value) {
|
||||
result.set(externalRoute, value);
|
||||
@@ -112,16 +119,22 @@ export function resolveRouteBindings(
|
||||
if (bindings) {
|
||||
for (const [externalRefId, targetRefId] of Object.entries(bindings)) {
|
||||
if (!isValidTargetRefId(targetRefId)) {
|
||||
throw new Error(
|
||||
`Invalid config at app.routes.bindings['${externalRefId}'], value must be a non-empty string or false`,
|
||||
);
|
||||
collector.report({
|
||||
code: 'ROUTE_BINDING_INVALID_VALUE',
|
||||
message: `Invalid config at app.routes.bindings['${externalRefId}'], value must be a non-empty string or false`,
|
||||
context: { routeId: externalRefId },
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
const externalRef = routesById.externalRoutes.get(externalRefId);
|
||||
if (!externalRef) {
|
||||
throw new Error(
|
||||
`Invalid config at app.routes.bindings, '${externalRefId}' is not a valid external route`,
|
||||
);
|
||||
collector.report({
|
||||
code: 'ROUTE_NOT_FOUND',
|
||||
message: `Invalid config at app.routes.bindings, '${externalRefId}' is not a valid external route`,
|
||||
context: { routeId: externalRefId },
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
// Skip if binding was already defined in code
|
||||
@@ -134,9 +147,14 @@ export function resolveRouteBindings(
|
||||
} else {
|
||||
const targetRef = routesById.routes.get(targetRefId);
|
||||
if (!targetRef) {
|
||||
throw new Error(
|
||||
`Invalid config at app.routes.bindings['${externalRefId}'], '${targetRefId}' is not a valid route`,
|
||||
);
|
||||
collector.report({
|
||||
code: 'ROUTE_NOT_FOUND',
|
||||
message: `Invalid config at app.routes.bindings['${externalRefId}'], '${String(
|
||||
targetRefId,
|
||||
)}' is not a valid route`,
|
||||
context: { routeId: String(targetRefId) },
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
result.set(externalRef, targetRef);
|
||||
|
||||
@@ -66,6 +66,16 @@ export type AppErrorTypes = {
|
||||
API_EXTENSION_INVALID: {
|
||||
context: { node: AppNode };
|
||||
};
|
||||
// routing
|
||||
ROUTE_DUPLICATE: {
|
||||
context: { routeId: string };
|
||||
};
|
||||
ROUTE_BINDING_INVALID_VALUE: {
|
||||
context: { routeId: string };
|
||||
};
|
||||
ROUTE_NOT_FOUND: {
|
||||
context: { routeId: string };
|
||||
};
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -316,9 +316,9 @@ export function createSpecializedApp(options?: CreateSpecializedAppOptions): {
|
||||
const appBasePath = getBasePath(config);
|
||||
const appTreeApi = new AppTreeApiProxy(tree, appBasePath);
|
||||
|
||||
const routeRefsById = collectRouteIds(features);
|
||||
const routeRefsById = collectRouteIds(features, collector);
|
||||
const routeResolutionApi = new RouteResolutionApiProxy(
|
||||
resolveRouteBindings(options?.bindRoutes, config, routeRefsById),
|
||||
resolveRouteBindings(options?.bindRoutes, config, routeRefsById, collector),
|
||||
appBasePath,
|
||||
);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user