From fb9b5e7bec0ab015fb0b3072c0968da19b54606a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 26 Jan 2024 13:14:38 +0100 Subject: [PATCH] frontend-app-api: key components by id instead of ref + tests Signed-off-by: Patrik Oldsberg --- .changeset/smart-numbers-call.md | 5 + .../DefaultComponentsApi.test.tsx | 111 ++++++++++++++++++ ...mponentsApi.ts => DefaultComponentsApi.ts} | 28 ++++- .../implementations/ComponentsApi/index.ts | 2 +- .../src/tree/resolveAppNodeSpecs.ts | 17 ++- .../frontend-app-api/src/wiring/createApp.tsx | 16 +-- 6 files changed, 153 insertions(+), 26 deletions(-) create mode 100644 .changeset/smart-numbers-call.md create mode 100644 packages/frontend-app-api/src/apis/implementations/ComponentsApi/DefaultComponentsApi.test.tsx rename packages/frontend-app-api/src/apis/implementations/ComponentsApi/{ComponentsApi.ts => DefaultComponentsApi.ts} (58%) diff --git a/.changeset/smart-numbers-call.md b/.changeset/smart-numbers-call.md new file mode 100644 index 0000000000..24a2053259 --- /dev/null +++ b/.changeset/smart-numbers-call.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-app-api': patch +--- + +The default `ComponentsApi` implementation now uses the `ComponentRef` ID as the component key, rather than the reference instance. This fixes a bug where duplicate installations of `@backstage/frontend-plugin-api` would break the app. diff --git a/packages/frontend-app-api/src/apis/implementations/ComponentsApi/DefaultComponentsApi.test.tsx b/packages/frontend-app-api/src/apis/implementations/ComponentsApi/DefaultComponentsApi.test.tsx new file mode 100644 index 0000000000..7d62103727 --- /dev/null +++ b/packages/frontend-app-api/src/apis/implementations/ComponentsApi/DefaultComponentsApi.test.tsx @@ -0,0 +1,111 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { + coreExtensionData, + createComponentExtension, + createComponentRef, + createExtension, + createExtensionOverrides, +} from '@backstage/frontend-plugin-api'; +import { resolveAppNodeSpecs } from '../../../tree/resolveAppNodeSpecs'; +import { resolveAppTree } from '../../../tree/resolveAppTree'; +import { App } from '../../../extensions/App'; +import { DefaultComponentsApi } from './DefaultComponentsApi'; +import { render, screen } from '@testing-library/react'; +import { instantiateAppNodeTree } from '../../../tree/instantiateAppNodeTree'; + +const testRefA = createComponentRef({ id: 'test.a' }); +const testRefB1 = createComponentRef({ id: 'test.b' }); +const testRefB2 = createComponentRef({ id: 'test.b' }); + +const baseOverrides = createExtensionOverrides({ + extensions: [ + App, + createExtension({ + namespace: 'app', + name: 'root', + attachTo: { id: 'app', input: 'root' }, + output: { + element: coreExtensionData.reactElement, + }, + factory() { + return { + element:
root
, + }; + }, + }), + ], +}); + +describe('DefaultComponentsApi', () => { + it('should provide components', () => { + const tree = resolveAppTree( + 'app', + resolveAppNodeSpecs({ + features: [ + baseOverrides, + createExtensionOverrides({ + extensions: [ + createComponentExtension({ + ref: testRefA, + loader: { sync: () => () =>
test.a
}, + }), + ], + }), + ], + }), + ); + instantiateAppNodeTree(tree.root); + const api = DefaultComponentsApi.fromTree(tree); + + const ComponentA = api.getComponent(testRefA); + render(); + + expect(screen.getByText('test.a')).toBeInTheDocument(); + }); + + it('should key extension refs by ID', () => { + const tree = resolveAppTree( + 'app', + resolveAppNodeSpecs({ + features: [ + baseOverrides, + createExtensionOverrides({ + extensions: [ + createComponentExtension({ + ref: testRefB1, + loader: { sync: () => () =>
test.b
}, + }), + ], + }), + ], + }), + ); + instantiateAppNodeTree(tree.root); + const api = DefaultComponentsApi.fromTree(tree); + + const ComponentB1 = api.getComponent(testRefB1); + const ComponentB2 = api.getComponent(testRefB2); + + expect(ComponentB1).toBe(ComponentB2); + + render(); + + expect(screen.getByText('test.b')).toBeInTheDocument(); + }); +}); diff --git a/packages/frontend-app-api/src/apis/implementations/ComponentsApi/ComponentsApi.ts b/packages/frontend-app-api/src/apis/implementations/ComponentsApi/DefaultComponentsApi.ts similarity index 58% rename from packages/frontend-app-api/src/apis/implementations/ComponentsApi/ComponentsApi.ts rename to packages/frontend-app-api/src/apis/implementations/ComponentsApi/DefaultComponentsApi.ts index 4d8bc47e24..572b6fc3c4 100644 --- a/packages/frontend-app-api/src/apis/implementations/ComponentsApi/ComponentsApi.ts +++ b/packages/frontend-app-api/src/apis/implementations/ComponentsApi/DefaultComponentsApi.ts @@ -15,7 +15,12 @@ */ import { ComponentType } from 'react'; -import { ComponentRef, ComponentsApi } from '@backstage/frontend-plugin-api'; +import { + AppTree, + ComponentRef, + ComponentsApi, + createComponentExtension, +} from '@backstage/frontend-plugin-api'; /** * Implementation for the {@linkComponentApi} @@ -23,14 +28,29 @@ import { ComponentRef, ComponentsApi } from '@backstage/frontend-plugin-api'; * @internal */ export class DefaultComponentsApi implements ComponentsApi { - #components: Map, ComponentType>; + #components: Map>; - constructor(components: Map, any>) { + static fromTree(tree: AppTree) { + const componentEntries = tree.root.edges.attachments + .get('components') + ?.reduce((map, e) => { + const data = e.instance?.getData( + createComponentExtension.componentDataRef, + ); + if (data) { + map.set(data.ref.id, data.impl); + } + return map; + }, new Map()); + return new DefaultComponentsApi(componentEntries ?? new Map()); + } + + constructor(components: Map) { this.#components = components; } getComponent(ref: ComponentRef): ComponentType { - const impl = this.#components.get(ref); + const impl = this.#components.get(ref.id); if (!impl) { throw new Error(`No implementation found for component ref ${ref}`); } diff --git a/packages/frontend-app-api/src/apis/implementations/ComponentsApi/index.ts b/packages/frontend-app-api/src/apis/implementations/ComponentsApi/index.ts index f04059c54f..18604f15de 100644 --- a/packages/frontend-app-api/src/apis/implementations/ComponentsApi/index.ts +++ b/packages/frontend-app-api/src/apis/implementations/ComponentsApi/index.ts @@ -14,4 +14,4 @@ * limitations under the License. */ -export { DefaultComponentsApi } from './ComponentsApi'; +export { DefaultComponentsApi } from './DefaultComponentsApi'; diff --git a/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.ts b/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.ts index 8fd9d44638..f056e7a49e 100644 --- a/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.ts +++ b/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.ts @@ -31,17 +31,22 @@ import { toInternalExtension } from '../../../frontend-plugin-api/src/wiring/res /** @internal */ export function resolveAppNodeSpecs(options: { - features: FrontendFeature[]; - builtinExtensions: Extension[]; - parameters: Array; + features?: FrontendFeature[]; + builtinExtensions?: Extension[]; + parameters?: Array; forbidden?: Set; }): AppNodeSpec[] { - const { builtinExtensions, parameters, forbidden = new Set() } = options; + const { + builtinExtensions = [], + parameters = [], + forbidden = new Set(), + features = [], + } = options; - const plugins = options.features.filter( + const plugins = features.filter( (f): f is BackstagePlugin => f.$$type === '@backstage/BackstagePlugin', ); - const overrides = options.features.filter( + const overrides = features.filter( (f): f is ExtensionOverrides => f.$$type === '@backstage/ExtensionOverrides', ); diff --git a/packages/frontend-app-api/src/wiring/createApp.tsx b/packages/frontend-app-api/src/wiring/createApp.tsx index 635e36aace..591e1cbf25 100644 --- a/packages/frontend-app-api/src/wiring/createApp.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.tsx @@ -19,11 +19,9 @@ import { ConfigReader } from '@backstage/config'; import { AppTree, appTreeApiRef, - ComponentRef, componentsApiRef, coreExtensionData, createApiExtension, - createComponentExtension, createThemeExtension, createTranslationExtension, FrontendFeature, @@ -373,22 +371,10 @@ function createApiHolder( factory: () => routeResolutionApi, }); - const componentsExtensions = - tree.root.edges.attachments - .get('components') - ?.map(e => e.instance?.getData(createComponentExtension.componentDataRef)) - .filter(x => !!x) ?? []; - - const componentsMap = componentsExtensions.reduce( - (components, component) => - component ? components.set(component.ref, component?.impl) : components, - new Map, any>(), - ); - factoryRegistry.register('static', { api: componentsApiRef, deps: {}, - factory: () => new DefaultComponentsApi(componentsMap), + factory: () => DefaultComponentsApi.fromTree(tree), }); factoryRegistry.register('static', {