From da8cba44f474e8bc76cc613c30e13161987bbfcc Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 8 Jun 2021 18:40:14 +0200 Subject: [PATCH 1/2] core-plugin-api: apply extension fixes and deprecate extensions in core-app-api Signed-off-by: Patrik Oldsberg --- .changeset/olive-singers-sparkle.md | 5 + .changeset/pink-bags-tickle.md | 5 + .../src/extensions/extensions.test.tsx | 75 ------------ .../src/extensions/extensions.tsx | 115 ++---------------- .../src/extensions/extensions.tsx | 70 ++++++++--- 5 files changed, 73 insertions(+), 197 deletions(-) create mode 100644 .changeset/olive-singers-sparkle.md create mode 100644 .changeset/pink-bags-tickle.md delete mode 100644 packages/core-app-api/src/extensions/extensions.test.tsx diff --git a/.changeset/olive-singers-sparkle.md b/.changeset/olive-singers-sparkle.md new file mode 100644 index 0000000000..14e45867a0 --- /dev/null +++ b/.changeset/olive-singers-sparkle.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-plugin-api': patch +--- + +Apply fixes to the extension creation API that were mistakenly applied to `@backstage/core-app-api` instead. diff --git a/.changeset/pink-bags-tickle.md b/.changeset/pink-bags-tickle.md new file mode 100644 index 0000000000..9c600459e7 --- /dev/null +++ b/.changeset/pink-bags-tickle.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-app-api': patch +--- + +Deprecate and disable the extension creation methods, which were added to this package by mistake and should only exist within `@backstage/core-plugin-api`. diff --git a/packages/core-app-api/src/extensions/extensions.test.tsx b/packages/core-app-api/src/extensions/extensions.test.tsx deleted file mode 100644 index e8770dddbb..0000000000 --- a/packages/core-app-api/src/extensions/extensions.test.tsx +++ /dev/null @@ -1,75 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * 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 { createPlugin, createRouteRef } from '@backstage/core-plugin-api'; -import { getComponentData } from './componentData'; -import { - createComponentExtension, - createReactExtension, - createRoutableExtension, -} from './extensions'; - -const plugin = createPlugin({ - id: 'my-plugin', -}); - -describe('extensions', () => { - it('should create a react extension with component data', () => { - const Component = () =>
; - - const extension = createReactExtension({ - component: { - sync: Component, - }, - data: { - myData: { foo: 'bar' }, - }, - }); - - const ExtensionComponent = plugin.provide(extension); - const element = ; - - expect(getComponentData(element, 'core.plugin')).toBe(plugin); - expect(getComponentData(element, 'myData')).toEqual({ foo: 'bar' }); - }); - - it('should create react extensions of different types', () => { - const Component = () =>
; - const routeRef = createRouteRef({ id: 'foo' }); - - const extension1 = createComponentExtension({ - component: { - sync: Component, - }, - }); - - const extension2 = createRoutableExtension({ - component: () => Promise.resolve(Component), - mountPoint: routeRef, - }); - - const ExtensionComponent1 = plugin.provide(extension1); - const ExtensionComponent2 = plugin.provide(extension2); - - const element1 = ; - const element2 = ; - - expect(getComponentData(element1, 'core.plugin')).toBe(plugin); - expect(getComponentData(element2, 'core.plugin')).toBe(plugin); - expect(getComponentData(element2, 'core.mountPoint')).toBe(routeRef); - }); -}); diff --git a/packages/core-app-api/src/extensions/extensions.tsx b/packages/core-app-api/src/extensions/extensions.tsx index 8eab3b905f..a9121c4d15 100644 --- a/packages/core-app-api/src/extensions/extensions.tsx +++ b/packages/core-app-api/src/extensions/extensions.tsx @@ -14,15 +14,7 @@ * limitations under the License. */ -import React, { lazy, Suspense } from 'react'; -import { attachComponentData } from './componentData'; -import { - Extension, - BackstagePlugin, - RouteRef, - useRouteRef, - useApp, -} from '@backstage/core-plugin-api'; +import { Extension, RouteRef } from '@backstage/core-plugin-api'; type ComponentLoader = | { @@ -32,114 +24,31 @@ type ComponentLoader = sync: T; }; -// We do not use ComponentType as the return type, since it doesn't let us convey the children prop. -// ComponentType inserts children as an optional prop whether the inner component accepts it or not, -// making it impossible to make the usage of children type safe. +const ERROR_MESSAGE = 'Import this from @backstage/core-plugin-api'; + +/** @deprecated Import from @backstage/core-plugin-api instead */ export function createRoutableExtension< T extends (props: any) => JSX.Element | null ->(options: { +>(_options: { component: () => Promise; mountPoint: RouteRef; }): Extension { - const { component, mountPoint } = options; - return createReactExtension({ - component: { - lazy: () => - component().then( - InnerComponent => { - const RoutableExtensionWrapper: any = (props: any) => { - // Validate that the routing is wired up correctly in the App.tsx - try { - useRouteRef(mountPoint); - } catch (error) { - if (error?.message.startsWith('No path for ')) { - throw new Error( - `Routable extension component with mount point ${mountPoint} was not discovered in the app element tree. ` + - 'Routable extension components may not be rendered by other components and must be ' + - 'directly available as an element within the App provider component.', - ); - } - throw error; - } - return ; - }; - - const componentName = - (InnerComponent as { displayName?: string }).displayName || - InnerComponent.name || - 'LazyComponent'; - - RoutableExtensionWrapper.displayName = `RoutableExtension(${componentName})`; - - return RoutableExtensionWrapper as T; - }, - error => { - const RoutableExtensionWrapper: any = (_: any) => { - const app = useApp(); - const { BootErrorPage } = app.getComponents(); - - return ; - }; - return RoutableExtensionWrapper; - }, - ), - }, - data: { - 'core.mountPoint': mountPoint, - }, - }); + throw new Error(ERROR_MESSAGE); } -// We do not use ComponentType as the return type, since it doesn't let us convey the children prop. -// ComponentType inserts children as an optional prop whether the inner component accepts it or not, -// making it impossible to make the usage of children type safe. +/** @deprecated Import from @backstage/core-plugin-api instead */ export function createComponentExtension< T extends (props: any) => JSX.Element | null ->(options: { component: ComponentLoader }): Extension { - const { component } = options; - return createReactExtension({ component }); +>(_options: { component: ComponentLoader }): Extension { + throw new Error(ERROR_MESSAGE); } -// We do not use ComponentType as the return type, since it doesn't let us convey the children prop. -// ComponentType inserts children as an optional prop whether the inner component accepts it or not, -// making it impossible to make the usage of children type safe. +/** @deprecated Import from @backstage/core-plugin-api instead */ export function createReactExtension< T extends (props: any) => JSX.Element | null ->(options: { +>(_options: { component: ComponentLoader; data?: Record; }): Extension { - const { data = {} } = options; - - let Component: T; - if ('lazy' in options.component) { - const lazyLoader = options.component.lazy; - Component = (lazy(() => - lazyLoader().then(component => ({ default: component })), - ) as unknown) as T; - } else { - Component = options.component.sync; - } - const componentName = - (Component as { displayName?: string }).displayName || - Component.name || - 'Component'; - - return { - expose(plugin: BackstagePlugin) { - const Result: any = (props: any) => ( - - - - ); - - attachComponentData(Result, 'core.plugin', plugin); - for (const [key, value] of Object.entries(data)) { - attachComponentData(Result, key, value); - } - - Result.displayName = `Extension(${componentName})`; - return Result; - }, - }; + throw new Error(ERROR_MESSAGE); } diff --git a/packages/core-plugin-api/src/extensions/extensions.tsx b/packages/core-plugin-api/src/extensions/extensions.tsx index e56b6901dd..d6597ba8a9 100644 --- a/packages/core-plugin-api/src/extensions/extensions.tsx +++ b/packages/core-plugin-api/src/extensions/extensions.tsx @@ -15,6 +15,7 @@ */ import React, { lazy, Suspense } from 'react'; +import { useApp } from '../app'; import { RouteRef, useRouteRef } from '../routing'; import { attachComponentData } from './componentData'; import { Extension, BackstagePlugin } from '../plugin/types'; @@ -27,8 +28,11 @@ type ComponentLoader = sync: T; }; +// We do not use ComponentType as the return type, since it doesn't let us convey the children prop. +// ComponentType inserts children as an optional prop whether the inner component accepts it or not, +// making it impossible to make the usage of children type safe. export function createRoutableExtension< - T extends (props: any) => JSX.Element + T extends (props: any) => JSX.Element | null >(options: { component: () => Promise; mountPoint: RouteRef; @@ -37,22 +41,44 @@ export function createRoutableExtension< return createReactExtension({ component: { lazy: () => - component().then(InnerComponent => { - const RoutableExtensionWrapper = ((props: any) => { - // Validate that the routing is wired up correctly in the App.tsx - try { - useRouteRef(mountPoint); - } catch { - throw new Error( - 'Routable extension component was not discovered in the app element tree. ' + - 'Routable extension components may not be rendered by other components and must be ' + - 'directly available as an element within the App provider component.', - ); - } - return ; - }) as T; - return RoutableExtensionWrapper; - }), + component().then( + InnerComponent => { + const RoutableExtensionWrapper: any = (props: any) => { + // Validate that the routing is wired up correctly in the App.tsx + try { + useRouteRef(mountPoint); + } catch (error) { + if (error?.message.startsWith('No path for ')) { + throw new Error( + `Routable extension component with mount point ${mountPoint} was not discovered in the app element tree. ` + + 'Routable extension components may not be rendered by other components and must be ' + + 'directly available as an element within the App provider component.', + ); + } + throw error; + } + return ; + }; + + const componentName = + (InnerComponent as { displayName?: string }).displayName || + InnerComponent.name || + 'LazyComponent'; + + RoutableExtensionWrapper.displayName = `RoutableExtension(${componentName})`; + + return RoutableExtensionWrapper as T; + }, + error => { + const RoutableExtensionWrapper: any = (_: any) => { + const app = useApp(); + const { BootErrorPage } = app.getComponents(); + + return ; + }; + return RoutableExtensionWrapper; + }, + ), }, data: { 'core.mountPoint': mountPoint, @@ -60,15 +86,21 @@ export function createRoutableExtension< }); } +// We do not use ComponentType as the return type, since it doesn't let us convey the children prop. +// ComponentType inserts children as an optional prop whether the inner component accepts it or not, +// making it impossible to make the usage of children type safe. export function createComponentExtension< - T extends (props: any) => JSX.Element + T extends (props: any) => JSX.Element | null >(options: { component: ComponentLoader }): Extension { const { component } = options; return createReactExtension({ component }); } +// We do not use ComponentType as the return type, since it doesn't let us convey the children prop. +// ComponentType inserts children as an optional prop whether the inner component accepts it or not, +// making it impossible to make the usage of children type safe. export function createReactExtension< - T extends (props: any) => JSX.Element + T extends (props: any) => JSX.Element | null >(options: { component: ComponentLoader; data?: Record; From 6e266402f47e355408a0c214d60dfe59eb0bcd6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 8 Jun 2021 21:14:34 +0200 Subject: [PATCH 2/2] update api-reports and tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/core-app-api/src/app/App.test.tsx | 2 +- packages/core-app-api/src/plugins/collectors.test.tsx | 5 +++-- packages/core-app-api/src/routing/RoutingProvider.test.tsx | 2 +- packages/core-app-api/src/routing/collectors.test.tsx | 3 ++- packages/core-plugin-api/api-report.md | 6 +++--- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/core-app-api/src/app/App.test.tsx b/packages/core-app-api/src/app/App.test.tsx index c282ba6b7f..cde3af9456 100644 --- a/packages/core-app-api/src/app/App.test.tsx +++ b/packages/core-app-api/src/app/App.test.tsx @@ -20,7 +20,6 @@ import { lightTheme } from '@backstage/theme'; import { render, screen } from '@testing-library/react'; import React, { PropsWithChildren } from 'react'; import { BrowserRouter, Routes } from 'react-router-dom'; -import { createRoutableExtension } from '../extensions'; import { defaultAppIcons } from './icons'; import { configApiRef, @@ -31,6 +30,7 @@ import { createExternalRouteRef, createRouteRef, createSubRouteRef, + createRoutableExtension, } from '@backstage/core-plugin-api'; import { generateBoundRoutes, PrivateAppImpl } from './App'; diff --git a/packages/core-app-api/src/plugins/collectors.test.tsx b/packages/core-app-api/src/plugins/collectors.test.tsx index 220d89690f..46c19ec7bf 100644 --- a/packages/core-app-api/src/plugins/collectors.test.tsx +++ b/packages/core-app-api/src/plugins/collectors.test.tsx @@ -15,11 +15,12 @@ */ import React, { PropsWithChildren } from 'react'; -import { createPlugin, createRouteRef } from '@backstage/core-plugin-api'; import { + createPlugin, + createRouteRef, createRoutableExtension, createComponentExtension, -} from '../extensions'; +} from '@backstage/core-plugin-api'; import { MemoryRouter, Routes, Route } from 'react-router-dom'; import { traverseElementTree, diff --git a/packages/core-app-api/src/routing/RoutingProvider.test.tsx b/packages/core-app-api/src/routing/RoutingProvider.test.tsx index 39528fe3a9..8f8ffc820b 100644 --- a/packages/core-app-api/src/routing/RoutingProvider.test.tsx +++ b/packages/core-app-api/src/routing/RoutingProvider.test.tsx @@ -25,7 +25,6 @@ import { render } from '@testing-library/react'; import { renderHook } from '@testing-library/react-hooks'; import { VersionedValue } from '../lib/versionedValues'; import { getGlobalSingleton } from '../lib/globalObject'; -import { createRoutableExtension } from '../extensions'; import { childDiscoverer, routeElementDiscoverer, @@ -34,6 +33,7 @@ import { import { createPlugin, useRouteRef, + createRoutableExtension, createRouteRef, createExternalRouteRef, RouteRef, diff --git a/packages/core-app-api/src/routing/collectors.test.tsx b/packages/core-app-api/src/routing/collectors.test.tsx index 40b57bf25a..50c8ec1772 100644 --- a/packages/core-app-api/src/routing/collectors.test.tsx +++ b/packages/core-app-api/src/routing/collectors.test.tsx @@ -27,11 +27,12 @@ import { routeElementDiscoverer, } from '../extensions/traversal'; import { + createRoutableExtension, createRouteRef, createPlugin, RouteRef, } from '@backstage/core-plugin-api'; -import { attachComponentData, createRoutableExtension } from '../extensions'; +import { attachComponentData } from '../extensions'; import { MemoryRouter, Routes, Route } from 'react-router-dom'; const MockComponent = ({ children }: PropsWithChildren<{ path?: string }>) => ( diff --git a/packages/core-plugin-api/api-report.md b/packages/core-plugin-api/api-report.md index dcf53d8aed..686d096b7f 100644 --- a/packages/core-plugin-api/api-report.md +++ b/packages/core-plugin-api/api-report.md @@ -172,7 +172,7 @@ export function createApiFactory(api: ApiRef, instan export function createApiRef(config: ApiRefConfig): ApiRef; // @public (undocumented) -export function createComponentExtension JSX.Element>(options: { +export function createComponentExtension JSX.Element | null>(options: { component: ComponentLoader; }): Extension; @@ -189,13 +189,13 @@ export function createExternalRouteRef(config: PluginConfig): BackstagePlugin; // @public (undocumented) -export function createReactExtension JSX.Element>(options: { +export function createReactExtension JSX.Element | null>(options: { component: ComponentLoader; data?: Record; }): Extension; // @public (undocumented) -export function createRoutableExtension JSX.Element>(options: { +export function createRoutableExtension JSX.Element | null>(options: { component: () => Promise; mountPoint: RouteRef; }): Extension;