From d7c5d80c57ef054c9f302221047ce51d4caee987 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 10 Oct 2023 13:43:51 +0200 Subject: [PATCH] frontend-app-api: move root extension to be an input on core instead Signed-off-by: Patrik Oldsberg --- .changeset/slow-cameras-grab.md | 5 + .../frontend-app-api/src/extensions/Core.tsx | 16 ++- .../src/extensions/CoreLayout.tsx | 2 +- .../extractRouteInfoFromInstanceTree.test.ts | 4 +- .../extractRouteInfoFromInstanceTree.ts | 6 +- .../src/wiring/createApp.test.tsx | 104 ++++++------------ .../frontend-app-api/src/wiring/createApp.tsx | 38 ++----- .../frontend-app-api/src/wiring/parameters.ts | 16 +-- .../src/wiring/createPlugin.test.ts | 2 +- 9 files changed, 81 insertions(+), 112 deletions(-) create mode 100644 .changeset/slow-cameras-grab.md diff --git a/.changeset/slow-cameras-grab.md b/.changeset/slow-cameras-grab.md new file mode 100644 index 0000000000..d2ffaac73d --- /dev/null +++ b/.changeset/slow-cameras-grab.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-app-api': minor +--- + +The hidden `'root'` extension has been removed and has instead been made an input of the `'core'` extension. The checks for rejecting configuration of the `'root'` extension to rejects configuration of the `'core'` extension instead. diff --git a/packages/frontend-app-api/src/extensions/Core.tsx b/packages/frontend-app-api/src/extensions/Core.tsx index ec00059bf8..6efe3984dc 100644 --- a/packages/frontend-app-api/src/extensions/Core.tsx +++ b/packages/frontend-app-api/src/extensions/Core.tsx @@ -30,7 +30,19 @@ export const Core = createExtension({ themes: createExtensionInput({ theme: coreExtensionData.theme, }), + root: createExtensionInput( + { + element: coreExtensionData.reactElement, + }, + { singleton: true }, + ), + }, + output: { + root: coreExtensionData.reactElement, + }, + factory({ bind, inputs }) { + bind({ + root: inputs.root.element, + }); }, - output: {}, - factory() {}, }); diff --git a/packages/frontend-app-api/src/extensions/CoreLayout.tsx b/packages/frontend-app-api/src/extensions/CoreLayout.tsx index 9b0b8b25d1..bf0f382cfc 100644 --- a/packages/frontend-app-api/src/extensions/CoreLayout.tsx +++ b/packages/frontend-app-api/src/extensions/CoreLayout.tsx @@ -24,7 +24,7 @@ import { SidebarPage } from '@backstage/core-components'; export const CoreLayout = createExtension({ id: 'core.layout', - attachTo: { id: 'root', input: 'default' }, + attachTo: { id: 'core', input: 'root' }, inputs: { nav: createExtensionInput( { diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts index 17c224ab45..e70f588e0e 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts @@ -74,12 +74,12 @@ function routeInfoFromExtensions(extensions: Extension[]) { id: 'test', extensions, }); - const { rootInstances } = createInstances({ + const { coreInstance } = createInstances({ config: new MockConfigApi({}), features: [plugin], }); - return extractRouteInfoFromInstanceTree(rootInstances); + return extractRouteInfoFromInstanceTree(coreInstance); } function sortedEntries(map: Map): [RouteRef, T][] { diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts index 30305b0d2c..fa9f87dce0 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts @@ -42,7 +42,7 @@ export function joinPaths(...paths: string[]): string { return normalized; } -export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { +export function extractRouteInfoFromInstanceTree(core: ExtensionInstance): { routePaths: Map; routeParents: Map; routeObjects: BackstageRouteObject[]; @@ -151,9 +151,7 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { } } - for (const root of roots) { - visit(root); - } + visit(core); return { routePaths, routeParents, routeObjects }; } diff --git a/packages/frontend-app-api/src/wiring/createApp.test.tsx b/packages/frontend-app-api/src/wiring/createApp.test.tsx index 81937405fe..65f278b49b 100644 --- a/packages/frontend-app-api/src/wiring/createApp.test.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.test.tsx @@ -16,7 +16,6 @@ import { createExtension, - createExtensionInput, createExtensionOverrides, createPageExtension, createPlugin, @@ -27,7 +26,6 @@ import { screen } from '@testing-library/react'; import { MockConfigApi, renderWithEffects } from '@backstage/test-utils'; import React from 'react'; import { createRouteRef } from '@backstage/core-plugin-api'; -import { createExtensionInstance } from './createExtensionInstance'; const extBaseConfig = { id: 'test', @@ -37,12 +35,12 @@ const extBaseConfig = { }; describe('createInstances', () => { - it('throws an error when a root extension is parametrized', () => { + it('throws an error when a core extension is parametrized', () => { const config = new MockConfigApi({ app: { extensions: [ { - root: {}, + core: {}, }, ], }, @@ -54,18 +52,18 @@ describe('createInstances', () => { }), ]; expect(() => createInstances({ config, features })).toThrow( - "A 'root' extension configuration was detected, but the root extension is not configurable", + "A 'core' extension configuration was detected, but the core extension is not configurable", ); }); - it('throws an error when a root extension is overridden', () => { + it('throws an error when a core extension is overridden', () => { const config = new MockConfigApi({}); const features = [ createPlugin({ id: 'plugin', extensions: [ createExtension({ - id: 'root', + id: 'core', attachTo: { id: 'core.routes', input: 'route' }, inputs: {}, output: {}, @@ -75,7 +73,7 @@ describe('createInstances', () => { }), ]; expect(() => createInstances({ config, features })).toThrow( - "The following plugin(s) are overriding the 'root' extension which is forbidden: plugin", + "The following plugin(s) are overriding the 'core' extension which is forbidden: plugin", ); }); @@ -164,33 +162,14 @@ describe('createApp', () => { }); it('should log an app', () => { - const { rootInstances } = createInstances({ + const { coreInstance } = createInstances({ config: new MockConfigApi({}), features: [], }); - const root = createExtensionInstance({ - extension: createExtension({ - id: 'root', - attachTo: { id: '', input: '' }, - inputs: { - children: createExtensionInput({}), - }, - output: {}, - factory() {}, - }), - config: undefined, - attachments: new Map([['children', rootInstances]]), - }); - expect(String(root)).toMatchInlineSnapshot(` - " - children [ - - themes [ - - - ] - + expect(String(coreInstance)).toMatchInlineSnapshot(` + " + root [ content [ @@ -200,52 +179,24 @@ describe('createApp', () => { ] ] - " + themes [ + + + ] + " `); }); it('should serialize an app as JSON', () => { - const { rootInstances } = createInstances({ + const { coreInstance } = createInstances({ config: new MockConfigApi({}), features: [], }); - const root = createExtensionInstance({ - extension: createExtension({ - id: 'root', - attachTo: { id: '', input: '' }, - inputs: { - children: createExtensionInput({}), - }, - output: {}, - factory() {}, - }), - config: undefined, - attachments: new Map([['children', rootInstances]]), - }); - expect(JSON.parse(JSON.stringify(root))).toMatchInlineSnapshot(` + expect(JSON.parse(JSON.stringify(coreInstance))).toMatchInlineSnapshot(` { "attachments": { - "children": [ - { - "attachments": { - "themes": [ - { - "id": "themes.light", - "output": [ - "core.theme", - ], - }, - { - "id": "themes.dark", - "output": [ - "core.theme", - ], - }, - ], - }, - "id": "core", - }, + "root": [ { "attachments": { "content": [ @@ -271,8 +222,25 @@ describe('createApp', () => { ], }, ], + "themes": [ + { + "id": "themes.light", + "output": [ + "core.theme", + ], + }, + { + "id": "themes.dark", + "output": [ + "core.theme", + ], + }, + ], }, - "id": "root", + "id": "core", + "output": [ + "core.reactElement", + ], } `); }); diff --git a/packages/frontend-app-api/src/wiring/createApp.tsx b/packages/frontend-app-api/src/wiring/createApp.tsx index 873e774ad8..3999cad738 100644 --- a/packages/frontend-app-api/src/wiring/createApp.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.tsx @@ -250,13 +250,11 @@ export function createInstances(options: { return newInstance; } - const rootConfigs = attachmentMap.get('root')?.get('default') ?? []; - - const rootInstances = rootConfigs.map(instanceParams => - createInstance(instanceParams), + const coreInstance = createInstance( + extensionParams.find(p => p.extension.id === 'core')!, ); - return { instances, rootInstances }; + return { coreInstance, instances }; } /** @public */ @@ -286,41 +284,29 @@ export function createApp(options: { ]), ); - const { rootInstances } = createInstances({ + const { coreInstance } = createInstances({ features: allFeatures, config, }); - const routeInfo = extractRouteInfoFromInstanceTree(rootInstances); - - const coreInstance = rootInstances.find(({ id }) => id === 'core'); - if (!coreInstance) { - throw Error('Unable to find core extension instance'); - } - - const apiHolder = createApiHolder(coreInstance, config); - const appContext = createLegacyAppContext( allFeatures.filter( (f): f is BackstagePlugin => f.$$type === '@backstage/BackstagePlugin', ), ); - const rootElements = rootInstances - .map(e => ( - - {e.getData(coreExtensionData.reactElement)} - - )) - .filter((x): x is JSX.Element => !!x); - const App = () => ( - + - + {/* TODO: set base path using the logic from AppRouter */} - {rootElements} + + {coreInstance.getData(coreExtensionData.reactElement)} + diff --git a/packages/frontend-app-api/src/wiring/parameters.ts b/packages/frontend-app-api/src/wiring/parameters.ts index e81d5f74cb..911f12f416 100644 --- a/packages/frontend-app-api/src/wiring/parameters.ts +++ b/packages/frontend-app-api/src/wiring/parameters.ts @@ -235,13 +235,13 @@ export function mergeExtensionParameters(options: { override => toInternalExtensionOverrides(override).extensions, ); - // Prevent root override - if (pluginExtensions.some(({ id }) => id === 'root')) { - const rootPluginIds = pluginExtensions - .filter(({ id }) => id === 'root') + // Prevent core override + if (pluginExtensions.some(({ id }) => id === 'core')) { + const pluginIds = pluginExtensions + .filter(({ id }) => id === 'core') .map(({ source }) => source.id); throw new Error( - `The following plugin(s) are overriding the 'root' extension which is forbidden: ${rootPluginIds.join( + `The following plugin(s) are overriding the 'core' extension which is forbidden: ${pluginIds.join( ',', )}`, ); @@ -352,10 +352,10 @@ export function mergeExtensionParameters(options: { for (const overrideParam of parameters) { const extensionId = overrideParam.id; - // Prevent root parametrization - if (extensionId === 'root') { + // Prevent core parametrization + if (extensionId === 'core') { throw new Error( - "A 'root' extension configuration was detected, but the root extension is not configurable", + "A 'core' extension configuration was detected, but the core extension is not configurable", ); } diff --git a/packages/frontend-plugin-api/src/wiring/createPlugin.test.ts b/packages/frontend-plugin-api/src/wiring/createPlugin.test.ts index 07841b3a9a..7f73c45544 100644 --- a/packages/frontend-plugin-api/src/wiring/createPlugin.test.ts +++ b/packages/frontend-plugin-api/src/wiring/createPlugin.test.ts @@ -85,7 +85,7 @@ const TechDocsPage = createExtension({ const outputExtension = createExtension({ id: 'test.output', - attachTo: { id: 'root', input: 'default' }, + attachTo: { id: 'core', input: 'root' }, inputs: { names: createExtensionInput({ name: nameExtensionDataRef,