From 8b69319f29debb739e885bd6067e815c244d09c4 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Thu, 30 Nov 2023 09:43:45 +0100 Subject: [PATCH 1/7] refactor: do not use get T in component refs Signed-off-by: Camila Belo --- packages/frontend-plugin-api/src/components/ComponentRef.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/frontend-plugin-api/src/components/ComponentRef.tsx b/packages/frontend-plugin-api/src/components/ComponentRef.tsx index 2db85b9a42..d096fa34f3 100644 --- a/packages/frontend-plugin-api/src/components/ComponentRef.tsx +++ b/packages/frontend-plugin-api/src/components/ComponentRef.tsx @@ -34,10 +34,7 @@ export function createComponentRef(options: { const { id } = options; return { id, - get T(): T { - throw new Error(`tried to read ComponentRef.T of ${id}`); - }, - }; + } as ComponentRef; } const coreProgressComponentRef = createComponentRef({ From 08699d887392a1e47ac86474722fc91ef495ead8 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Thu, 30 Nov 2023 10:39:24 +0100 Subject: [PATCH 2/7] refactor: remove legacy component compat helper Signed-off-by: Camila Belo --- packages/app-next/src/App.tsx | 13 +--- .../examples/notFoundErrorPageExtension.tsx | 10 +-- packages/core-compat-api/api-report.md | 7 -- .../src/collectLegacyComponents.test.tsx | 66 ------------------- .../src/collectLegacyComponents.tsx | 50 -------------- packages/core-compat-api/src/index.ts | 1 - 6 files changed, 5 insertions(+), 142 deletions(-) delete mode 100644 packages/core-compat-api/src/collectLegacyComponents.test.tsx delete mode 100644 packages/core-compat-api/src/collectLegacyComponents.tsx diff --git a/packages/app-next/src/App.tsx b/packages/app-next/src/App.tsx index ccfeb61b58..d7ec30bf95 100644 --- a/packages/app-next/src/App.tsx +++ b/packages/app-next/src/App.tsx @@ -17,7 +17,7 @@ import React from 'react'; import { createApp } from '@backstage/frontend-app-api'; import { pagesPlugin } from './examples/pagesPlugin'; -import { CustomNotFoundErrorPage } from './examples/notFoundErrorPageExtension'; +import notFoundErrorPage from './examples/notFoundErrorPageExtension'; import graphiqlPlugin from '@backstage/plugin-graphiql/alpha'; import techRadarPlugin from '@backstage/plugin-tech-radar/alpha'; import userSettingsPlugin from '@backstage/plugin-user-settings/alpha'; @@ -33,10 +33,7 @@ import { } from '@backstage/frontend-plugin-api'; import techdocsPlugin from '@backstage/plugin-techdocs/alpha'; import { homePage } from './HomePage'; -import { - collectLegacyComponents, - collectLegacyRoutes, -} from '@backstage/core-compat-api'; +import { collectLegacyRoutes } from '@backstage/core-compat-api'; import { FlatRoutes } from '@backstage/core-app-api'; import { Route } from 'react-router'; import { CatalogImportPage } from '@backstage/plugin-catalog-import'; @@ -118,10 +115,6 @@ const collectedLegacyPlugins = collectLegacyRoutes( , ); -const legacyAppComponents = collectLegacyComponents({ - NotFoundErrorPage: CustomNotFoundErrorPage, -}); - const app = createApp({ features: [ graphiqlPlugin, @@ -137,7 +130,7 @@ const app = createApp({ scmAuthExtension, scmIntegrationApi, signInPage, - ...legacyAppComponents, + notFoundErrorPage, ], }), ], diff --git a/packages/app-next/src/examples/notFoundErrorPageExtension.tsx b/packages/app-next/src/examples/notFoundErrorPageExtension.tsx index 82acc25fb5..6cdaff978f 100644 --- a/packages/app-next/src/examples/notFoundErrorPageExtension.tsx +++ b/packages/app-next/src/examples/notFoundErrorPageExtension.tsx @@ -36,15 +36,9 @@ export function CustomNotFoundErrorPage() { > 404 - Bowie was unable to locate this page. Please contact your support team - if this page used to exist. + Unable to locate this page. Please contact your support team if this + page used to exist. - Backstage bowie + + ); + }, + }, }); diff --git a/packages/frontend-plugin-api/api-report.md b/packages/frontend-plugin-api/api-report.md index e8f4f16e97..e578c181fb 100644 --- a/packages/frontend-plugin-api/api-report.md +++ b/packages/frontend-plugin-api/api-report.md @@ -22,7 +22,6 @@ import { AuthProviderInfo } from '@backstage/core-plugin-api'; import { AuthRequestOptions } from '@backstage/core-plugin-api'; import { BackstageIdentityApi } from '@backstage/core-plugin-api'; import { BackstageIdentityResponse } from '@backstage/core-plugin-api'; -import { BackstagePlugin as BackstagePlugin_2 } from '@backstage/core-plugin-api'; import { BackstageUserIdentity } from '@backstage/core-plugin-api'; import { bitbucketAuthApiRef } from '@backstage/core-plugin-api'; import { bitbucketServerAuthApiRef } from '@backstage/core-plugin-api'; @@ -340,7 +339,7 @@ export const coreComponentsRefs: { // @public (undocumented) export type CoreErrorBoundaryFallbackComponent = ComponentType< PropsWithChildren<{ - plugin?: BackstagePlugin_2; + plugin?: BackstagePlugin; error: Error; resetError: () => void; }> diff --git a/packages/frontend-plugin-api/src/components/ErrorBoundary.tsx b/packages/frontend-plugin-api/src/components/ErrorBoundary.tsx index 1191b75a1d..e3765be386 100644 --- a/packages/frontend-plugin-api/src/components/ErrorBoundary.tsx +++ b/packages/frontend-plugin-api/src/components/ErrorBoundary.tsx @@ -15,34 +15,13 @@ */ import React, { Component, PropsWithChildren } from 'react'; -// TODO: Dependency on MUI should be removed from core packages -import { Button } from '@material-ui/core'; -import { ErrorPanel } from '@backstage/core-components'; import { BackstagePlugin } from '../wiring'; +import { CoreErrorBoundaryFallbackComponent } from '../types'; -type DefaultErrorBoundaryFallbackProps = PropsWithChildren<{ +type ErrorBoundaryProps = PropsWithChildren<{ plugin?: BackstagePlugin; - error: Error; - resetError: () => void; + fallback: CoreErrorBoundaryFallbackComponent; }>; - -const DefaultErrorBoundaryFallback = ({ - plugin, - error, - resetError, -}: DefaultErrorBoundaryFallbackProps) => { - const title = `Error in ${plugin?.id}`; - - return ( - - - - ); -}; - -type ErrorBoundaryProps = PropsWithChildren<{ plugin?: BackstagePlugin }>; type ErrorBoundaryState = { error?: Error }; /** @internal */ @@ -62,12 +41,11 @@ export class ErrorBoundary extends Component< render() { const { error } = this.state; - const { plugin, children } = this.props; + const { plugin, children, fallback: Fallback } = this.props; if (error) { - // TODO: use a configurable error boundary fallback return ( - - + {children} diff --git a/packages/frontend-plugin-api/src/types.ts b/packages/frontend-plugin-api/src/types.ts index f3f68c362c..3c79bcdf35 100644 --- a/packages/frontend-plugin-api/src/types.ts +++ b/packages/frontend-plugin-api/src/types.ts @@ -14,8 +14,8 @@ * limitations under the License. */ -import { BackstagePlugin } from '@backstage/core-plugin-api'; import { ComponentType, PropsWithChildren } from 'react'; +import { BackstagePlugin } from './wiring'; // TODO(Rugvip): This might be a quite useful utility type, maybe add to @backstage/types? /** From e5b00a94e648290eeb3e08e3127b3944dcf53428 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Thu, 30 Nov 2023 13:08:45 +0100 Subject: [PATCH 4/7] refactor: use unknown on component ref type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Philipp Hugenroth Co-authored-by: Fredrik Adelöw Co-authored-by: Patrik Oldsberg Signed-off-by: Camila Belo --- .../ComponentsApi/ComponentsApi.ts | 5 +- packages/frontend-plugin-api/api-report.md | 56 +++++++++---------- .../src/apis/definitions/ComponentsApi.ts | 3 +- .../src/components/ComponentRef.tsx | 20 +++---- .../src/components/ErrorBoundary.tsx | 6 +- .../extensions/createComponentExtension.tsx | 22 ++++---- packages/frontend-plugin-api/src/index.ts | 8 +-- packages/frontend-plugin-api/src/types.ts | 30 ++++------ .../src/wiring/coreExtensionData.ts | 4 +- 9 files changed, 75 insertions(+), 79 deletions(-) diff --git a/packages/frontend-app-api/src/apis/implementations/ComponentsApi/ComponentsApi.ts b/packages/frontend-app-api/src/apis/implementations/ComponentsApi/ComponentsApi.ts index 50d125e4ab..4d8bc47e24 100644 --- a/packages/frontend-app-api/src/apis/implementations/ComponentsApi/ComponentsApi.ts +++ b/packages/frontend-app-api/src/apis/implementations/ComponentsApi/ComponentsApi.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { ComponentType } from 'react'; import { ComponentRef, ComponentsApi } from '@backstage/frontend-plugin-api'; /** @@ -22,13 +23,13 @@ import { ComponentRef, ComponentsApi } from '@backstage/frontend-plugin-api'; * @internal */ export class DefaultComponentsApi implements ComponentsApi { - #components: Map, any>; + #components: Map, ComponentType>; constructor(components: Map, any>) { this.#components = components; } - getComponent(ref: ComponentRef): T { + getComponent(ref: ComponentRef): ComponentType { const impl = this.#components.get(ref); if (!impl) { throw new Error(`No implementation found for component ref ${ref}`); diff --git a/packages/frontend-plugin-api/api-report.md b/packages/frontend-plugin-api/api-report.md index e578c181fb..7342558560 100644 --- a/packages/frontend-plugin-api/api-report.md +++ b/packages/frontend-plugin-api/api-report.md @@ -286,7 +286,7 @@ export type CommonAnalyticsContext = { }; // @public (undocumented) -export type ComponentRef = { +export type ComponentRef = { id: string; T: T; }; @@ -294,7 +294,7 @@ export type ComponentRef = { // @public export interface ComponentsApi { // (undocumented) - getComponent(ref: ComponentRef): T; + getComponent(ref: ComponentRef): ComponentType; } // @public @@ -321,29 +321,29 @@ export interface ConfigurableExtensionDataRef< } // @public (undocumented) -export type CoreBootErrorPageComponent = ComponentType< - PropsWithChildren<{ - step: 'load-config' | 'load-chunk'; - error: Error; - }> ->; +export type CoreBootErrorPageProps = PropsWithChildren<{ + step: 'load-config' | 'load-chunk'; + error: Error; +}>; // @public (undocumented) export const coreComponentsRefs: { - progress: ComponentRef; - bootErrorPage: ComponentRef; - notFoundErrorPage: ComponentRef; - errorBoundaryFallback: ComponentRef; + progress: ComponentRef<{ + children?: ReactNode; + }>; + bootErrorPage: ComponentRef; + notFoundErrorPage: ComponentRef<{ + children?: ReactNode; + }>; + errorBoundaryFallback: ComponentRef; }; // @public (undocumented) -export type CoreErrorBoundaryFallbackComponent = ComponentType< - PropsWithChildren<{ - plugin?: BackstagePlugin; - error: Error; - resetError: () => void; - }> ->; +export type CoreErrorBoundaryFallbackProps = PropsWithChildren<{ + plugin?: BackstagePlugin; + error: Error; + resetError: () => void; +}>; // @public (undocumented) export const coreExtensionData: { @@ -356,20 +356,18 @@ export const coreExtensionData: { logoElements: ConfigurableExtensionDataRef; component: ConfigurableExtensionDataRef< { - ref: ComponentRef>; - impl: ComponentType; + ref: ComponentRef; + impl: ComponentType; }, {} >; }; // @public (undocumented) -export type CoreNotFoundErrorPageComponent = ComponentType< - PropsWithChildren<{}> ->; +export type CoreNotFoundErrorPageProps = PropsWithChildren<{}>; // @public (undocumented) -export type CoreProgressComponent = ComponentType>; +export type CoreProgressProps = PropsWithChildren<{}>; // @public (undocumented) export function createApiExtension< @@ -399,11 +397,11 @@ export { createApiRef }; // @public (undocumented) export function createComponentExtension< - TRef extends ComponentRef, + TProps extends {}, TConfig extends {}, TInputs extends AnyExtensionInputMap, >(options: { - ref: TRef; + ref: ComponentRef; name?: string; disabled?: boolean; inputs?: TInputs; @@ -413,13 +411,13 @@ export function createComponentExtension< lazy: (values: { config: TConfig; inputs: Expand>; - }) => Promise; + }) => Promise>; } | { sync: (values: { config: TConfig; inputs: Expand>; - }) => TRef['T']; + }) => ComponentType; }; }): ExtensionDefinition; diff --git a/packages/frontend-plugin-api/src/apis/definitions/ComponentsApi.ts b/packages/frontend-plugin-api/src/apis/definitions/ComponentsApi.ts index b55ed96549..dee17e7d97 100644 --- a/packages/frontend-plugin-api/src/apis/definitions/ComponentsApi.ts +++ b/packages/frontend-plugin-api/src/apis/definitions/ComponentsApi.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { ComponentType } from 'react'; import { createApiRef } from '@backstage/core-plugin-api'; import { ComponentRef } from '../../components'; @@ -24,7 +25,7 @@ import { ComponentRef } from '../../components'; */ export interface ComponentsApi { // TODO: Should component refs also provide the default implementation so that we're guaranteed to get a component? - getComponent(ref: ComponentRef): T; + getComponent(ref: ComponentRef): ComponentType; } /** diff --git a/packages/frontend-plugin-api/src/components/ComponentRef.tsx b/packages/frontend-plugin-api/src/components/ComponentRef.tsx index d096fa34f3..dd2e65a61f 100644 --- a/packages/frontend-plugin-api/src/components/ComponentRef.tsx +++ b/packages/frontend-plugin-api/src/components/ComponentRef.tsx @@ -15,20 +15,20 @@ */ import { - CoreBootErrorPageComponent, - CoreErrorBoundaryFallbackComponent, - CoreNotFoundErrorPageComponent, - CoreProgressComponent, + CoreBootErrorPageProps, + CoreErrorBoundaryFallbackProps, + CoreNotFoundErrorPageProps, + CoreProgressProps, } from '../types'; /** @public */ -export type ComponentRef = { +export type ComponentRef = { id: string; T: T; }; /** @public */ -export function createComponentRef(options: { +export function createComponentRef(options: { id: string; }): ComponentRef { const { id } = options; @@ -37,22 +37,22 @@ export function createComponentRef(options: { } as ComponentRef; } -const coreProgressComponentRef = createComponentRef({ +const coreProgressComponentRef = createComponentRef({ id: 'core.components.progress', }); const coreBootErrorPageComponentRef = - createComponentRef({ + createComponentRef({ id: 'core.components.bootErrorPage', }); const coreNotFoundErrorPageComponentRef = - createComponentRef({ + createComponentRef({ id: 'core.components.notFoundErrorPage', }); const coreErrorBoundaryFallbackComponentRef = - createComponentRef({ + createComponentRef({ id: 'core.components.errorBoundaryFallback', }); diff --git a/packages/frontend-plugin-api/src/components/ErrorBoundary.tsx b/packages/frontend-plugin-api/src/components/ErrorBoundary.tsx index e3765be386..be988dc14f 100644 --- a/packages/frontend-plugin-api/src/components/ErrorBoundary.tsx +++ b/packages/frontend-plugin-api/src/components/ErrorBoundary.tsx @@ -14,13 +14,13 @@ * limitations under the License. */ -import React, { Component, PropsWithChildren } from 'react'; +import React, { Component, ComponentType, PropsWithChildren } from 'react'; import { BackstagePlugin } from '../wiring'; -import { CoreErrorBoundaryFallbackComponent } from '../types'; +import { CoreErrorBoundaryFallbackProps } from '../types'; type ErrorBoundaryProps = PropsWithChildren<{ plugin?: BackstagePlugin; - fallback: CoreErrorBoundaryFallbackComponent; + fallback: ComponentType; }>; type ErrorBoundaryState = { error?: Error }; diff --git a/packages/frontend-plugin-api/src/extensions/createComponentExtension.tsx b/packages/frontend-plugin-api/src/extensions/createComponentExtension.tsx index 601ee81d11..816dee6054 100644 --- a/packages/frontend-plugin-api/src/extensions/createComponentExtension.tsx +++ b/packages/frontend-plugin-api/src/extensions/createComponentExtension.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { lazy } from 'react'; +import React, { lazy, ComponentType } from 'react'; import { AnyExtensionInputMap, ResolvedExtensionInputs, @@ -27,11 +27,11 @@ import { ExtensionBoundary, ComponentRef } from '../components'; /** @public */ export function createComponentExtension< - TRef extends ComponentRef, + TProps extends {}, TConfig extends {}, TInputs extends AnyExtensionInputMap, >(options: { - ref: TRef; + ref: ComponentRef; name?: string; disabled?: boolean; inputs?: TInputs; @@ -41,13 +41,13 @@ export function createComponentExtension< lazy: (values: { config: TConfig; inputs: Expand>; - }) => Promise; + }) => Promise>; } | { sync: (values: { config: TConfig; inputs: Expand>; - }) => TRef['T']; + }) => ComponentType; }; }) { return createExtension({ @@ -62,15 +62,17 @@ export function createComponentExtension< component: coreExtensionData.component, }, factory({ config, inputs, node }) { - let ExtensionComponent: TRef['T']; + let ExtensionComponent: ComponentType; if ('sync' in options.component) { ExtensionComponent = options.component.sync({ config, inputs }); } else { - const loader = options.component.lazy({ config, inputs }); + const lazyLoader = options.component.lazy; ExtensionComponent = lazy(() => - loader.then(component => ({ default: component })), - ); + lazyLoader({ config, inputs }).then(component => ({ + default: component, + })), + ) as unknown as ComponentType; } return { @@ -78,7 +80,7 @@ export function createComponentExtension< ref: options.ref, impl: props => ( - + ), }, diff --git a/packages/frontend-plugin-api/src/index.ts b/packages/frontend-plugin-api/src/index.ts index a38a7991fb..04cbddd336 100644 --- a/packages/frontend-plugin-api/src/index.ts +++ b/packages/frontend-plugin-api/src/index.ts @@ -31,8 +31,8 @@ export * from './apis/system'; export * from './wiring'; export type { - CoreProgressComponent, - CoreBootErrorPageComponent, - CoreNotFoundErrorPageComponent, - CoreErrorBoundaryFallbackComponent, + CoreProgressProps, + CoreBootErrorPageProps, + CoreNotFoundErrorPageProps, + CoreErrorBoundaryFallbackProps, } from './types'; diff --git a/packages/frontend-plugin-api/src/types.ts b/packages/frontend-plugin-api/src/types.ts index 3c79bcdf35..a69c483b5c 100644 --- a/packages/frontend-plugin-api/src/types.ts +++ b/packages/frontend-plugin-api/src/types.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { ComponentType, PropsWithChildren } from 'react'; +import { PropsWithChildren } from 'react'; import { BackstagePlugin } from './wiring'; // TODO(Rugvip): This might be a quite useful utility type, maybe add to @backstage/types? @@ -25,26 +25,20 @@ import { BackstagePlugin } from './wiring'; export type Expand = T extends infer O ? { [K in keyof O]: O[K] } : never; /** @public */ -export type CoreProgressComponent = ComponentType>; +export type CoreProgressProps = PropsWithChildren<{}>; /** @public */ -export type CoreBootErrorPageComponent = ComponentType< - PropsWithChildren<{ - step: 'load-config' | 'load-chunk'; - error: Error; - }> ->; +export type CoreBootErrorPageProps = PropsWithChildren<{ + step: 'load-config' | 'load-chunk'; + error: Error; +}>; /** @public */ -export type CoreNotFoundErrorPageComponent = ComponentType< - PropsWithChildren<{}> ->; +export type CoreNotFoundErrorPageProps = PropsWithChildren<{}>; /** @public */ -export type CoreErrorBoundaryFallbackComponent = ComponentType< - PropsWithChildren<{ - plugin?: BackstagePlugin; - error: Error; - resetError: () => void; - }> ->; +export type CoreErrorBoundaryFallbackProps = PropsWithChildren<{ + plugin?: BackstagePlugin; + error: Error; + resetError: () => void; +}>; diff --git a/packages/frontend-plugin-api/src/wiring/coreExtensionData.ts b/packages/frontend-plugin-api/src/wiring/coreExtensionData.ts index 9bdb4b23eb..47a77414a0 100644 --- a/packages/frontend-plugin-api/src/wiring/coreExtensionData.ts +++ b/packages/frontend-plugin-api/src/wiring/coreExtensionData.ts @@ -47,7 +47,7 @@ export const coreExtensionData = { theme: createExtensionDataRef('core.theme'), logoElements: createExtensionDataRef('core.logos'), component: createExtensionDataRef<{ - ref: ComponentRef>; - impl: ComponentType; + ref: ComponentRef; + impl: ComponentType; }>('component.ref'), }; From de595c955c18978a45aa53464f0ed99337072214 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Thu, 30 Nov 2023 16:19:30 +0100 Subject: [PATCH 5/7] refactor: use progress component on extention error boundary Signed-off-by: Camila Belo --- .../frontend-plugin-api/src/components/ExtensionBoundary.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx b/packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx index 1e946efba9..d830f665f0 100644 --- a/packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx +++ b/packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx @@ -64,6 +64,7 @@ export function ExtensionBoundary(props: ExtensionBoundaryProps) { const componentsApi = useApi(componentsApiRef); const plugin = node.spec.source; + const Progress = componentsApi.getComponent(coreComponentsRefs.progress); const fallback = componentsApi.getComponent( coreComponentsRefs.errorBoundaryFallback, ); @@ -75,7 +76,7 @@ export function ExtensionBoundary(props: ExtensionBoundaryProps) { }; return ( - + }> {children} From b7adf24bf9aceada423c478ed38bc57f61813ec5 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Thu, 30 Nov 2023 15:33:19 +0100 Subject: [PATCH 6/7] docs: add changelog files Signed-off-by: Camila Belo --- .changeset/brave-beers-happen.md | 5 +++++ .changeset/good-wolves-leave.md | 5 +++++ .changeset/sharp-dingos-learn.md | 5 +++++ 3 files changed, 15 insertions(+) create mode 100644 .changeset/brave-beers-happen.md create mode 100644 .changeset/good-wolves-leave.md create mode 100644 .changeset/sharp-dingos-learn.md diff --git a/.changeset/brave-beers-happen.md b/.changeset/brave-beers-happen.md new file mode 100644 index 0000000000..8a16ed4824 --- /dev/null +++ b/.changeset/brave-beers-happen.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-plugin-api': patch +--- + +**_BREAKING_**: update alpha component ref type to be more specific than any and use new plugin type for error boundary component extensions. diff --git a/.changeset/good-wolves-leave.md b/.changeset/good-wolves-leave.md new file mode 100644 index 0000000000..c2cde8bfed --- /dev/null +++ b/.changeset/good-wolves-leave.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-app-api': patch +--- + +**_BREAKING_**: Use the new plugin type for error boundary components. diff --git a/.changeset/sharp-dingos-learn.md b/.changeset/sharp-dingos-learn.md new file mode 100644 index 0000000000..9dbb7ed8f4 --- /dev/null +++ b/.changeset/sharp-dingos-learn.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-compat-api': patch +--- + +**_BREAKING_**: delete alpha DI compatibility helper for components, migrating components should be simple without an helper. From dde40de392ab598898c836048dbaacafa3d73fde Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Fri, 1 Dec 2023 13:50:01 +0100 Subject: [PATCH 7/7] refactor: apply review suggestions Signed-off-by: Camila Belo --- .changeset/brave-beers-happen.md | 2 +- .changeset/good-wolves-leave.md | 2 +- .changeset/sharp-dingos-learn.md | 2 +- .../examples/notFoundErrorPageExtension.tsx | 4 +-- .../src/extensions/CoreRoutes.tsx | 10 +++--- .../src/extensions/components.tsx | 13 +++----- .../src/wiring/createApp.test.tsx | 7 ++-- .../frontend-app-api/src/wiring/createApp.tsx | 2 -- packages/frontend-plugin-api/api-report.md | 33 ++++++++----------- .../src/apis/definitions/ComponentsApi.ts | 13 +++++++- .../src/components/ComponentRef.tsx | 9 +---- .../src/components/ExtensionBoundary.tsx | 17 +++------- .../src/components/index.ts | 2 +- packages/frontend-plugin-api/src/index.ts | 1 - packages/frontend-plugin-api/src/types.ts | 18 ++++------ .../src/wiring/coreExtensionData.ts | 2 +- yarn.lock | 6 ++-- 17 files changed, 60 insertions(+), 83 deletions(-) diff --git a/.changeset/brave-beers-happen.md b/.changeset/brave-beers-happen.md index 8a16ed4824..34bbb95666 100644 --- a/.changeset/brave-beers-happen.md +++ b/.changeset/brave-beers-happen.md @@ -2,4 +2,4 @@ '@backstage/frontend-plugin-api': patch --- -**_BREAKING_**: update alpha component ref type to be more specific than any and use new plugin type for error boundary component extensions. +Update alpha component ref type to be more specific than any, delete boot page component and use new plugin type for error boundary component extensions. diff --git a/.changeset/good-wolves-leave.md b/.changeset/good-wolves-leave.md index c2cde8bfed..51ac99a55a 100644 --- a/.changeset/good-wolves-leave.md +++ b/.changeset/good-wolves-leave.md @@ -2,4 +2,4 @@ '@backstage/frontend-app-api': patch --- -**_BREAKING_**: Use the new plugin type for error boundary components. +Use the new plugin type for error boundary components. diff --git a/.changeset/sharp-dingos-learn.md b/.changeset/sharp-dingos-learn.md index 9dbb7ed8f4..9447ca9698 100644 --- a/.changeset/sharp-dingos-learn.md +++ b/.changeset/sharp-dingos-learn.md @@ -2,4 +2,4 @@ '@backstage/core-compat-api': patch --- -**_BREAKING_**: delete alpha DI compatibility helper for components, migrating components should be simple without an helper. +Delete alpha DI compatibility helper for components, migrating components should be simple without a helper. diff --git a/packages/app-next/src/examples/notFoundErrorPageExtension.tsx b/packages/app-next/src/examples/notFoundErrorPageExtension.tsx index 6cdaff978f..06e9b50bb8 100644 --- a/packages/app-next/src/examples/notFoundErrorPageExtension.tsx +++ b/packages/app-next/src/examples/notFoundErrorPageExtension.tsx @@ -17,7 +17,7 @@ import React from 'react'; import { createComponentExtension, - coreComponentsRefs, + coreComponentRefs, } from '@backstage/frontend-plugin-api'; import { Box, Typography } from '@material-ui/core'; import { Button } from '@backstage/core-components'; @@ -51,6 +51,6 @@ export function CustomNotFoundErrorPage() { } export default createComponentExtension({ - ref: coreComponentsRefs.notFoundErrorPage, + ref: coreComponentRefs.notFoundErrorPage, component: { sync: () => CustomNotFoundErrorPage }, }); diff --git a/packages/frontend-app-api/src/extensions/CoreRoutes.tsx b/packages/frontend-app-api/src/extensions/CoreRoutes.tsx index 1b79231699..155a5b60fa 100644 --- a/packages/frontend-app-api/src/extensions/CoreRoutes.tsx +++ b/packages/frontend-app-api/src/extensions/CoreRoutes.tsx @@ -19,9 +19,8 @@ import { createExtension, coreExtensionData, createExtensionInput, - coreComponentsRefs, - useApi, - componentsApiRef, + coreComponentRefs, + useComponentRef, } from '@backstage/frontend-plugin-api'; import { useRoutes } from 'react-router-dom'; @@ -41,9 +40,8 @@ export const CoreRoutes = createExtension({ }, factory({ inputs }) { const Routes = () => { - const componentsApi = useApi(componentsApiRef); - const NotFoundErrorPage = componentsApi.getComponent( - coreComponentsRefs.notFoundErrorPage, + const NotFoundErrorPage = useComponentRef( + coreComponentRefs.notFoundErrorPage, ); const element = useRoutes([ diff --git a/packages/frontend-app-api/src/extensions/components.tsx b/packages/frontend-app-api/src/extensions/components.tsx index e13c1b2f3b..44d479f170 100644 --- a/packages/frontend-app-api/src/extensions/components.tsx +++ b/packages/frontend-app-api/src/extensions/components.tsx @@ -20,7 +20,7 @@ import { Button } from '@material-ui/core'; import { createComponentExtension, - coreComponentsRefs, + coreComponentRefs, } from '@backstage/frontend-plugin-api'; import { ErrorPanel } from '@backstage/core-components'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports @@ -28,22 +28,17 @@ import { components as defaultComponents } from '../../../app-defaults/src/defau // eslint-disable-next-line @backstage/no-relative-monorepo-imports export const DefaultProgressComponent = createComponentExtension({ - ref: coreComponentsRefs.progress, + ref: coreComponentRefs.progress, component: { sync: () => defaultComponents.Progress }, }); -export const DefaultBootErrorPageComponent = createComponentExtension({ - ref: coreComponentsRefs.bootErrorPage, - component: { sync: () => defaultComponents.BootErrorPage }, -}); - export const DefaultNotFoundErrorPageComponent = createComponentExtension({ - ref: coreComponentsRefs.notFoundErrorPage, + ref: coreComponentRefs.notFoundErrorPage, component: { sync: () => defaultComponents.NotFoundErrorPage }, }); export const DefaultErrorBoundaryComponent = createComponentExtension({ - ref: coreComponentsRefs.errorBoundaryFallback, + ref: coreComponentRefs.errorBoundaryFallback, component: { sync: () => props => { const { plugin, error, resetError } = props; diff --git a/packages/frontend-app-api/src/wiring/createApp.test.tsx b/packages/frontend-app-api/src/wiring/createApp.test.tsx index 2713a9743e..32773de7e1 100644 --- a/packages/frontend-app-api/src/wiring/createApp.test.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.test.tsx @@ -201,10 +201,9 @@ describe('createApp', () => { ] components [ - - - - + + + ] themes [ diff --git a/packages/frontend-app-api/src/wiring/createApp.tsx b/packages/frontend-app-api/src/wiring/createApp.tsx index f58dfb3159..dcfbd1faf2 100644 --- a/packages/frontend-app-api/src/wiring/createApp.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.tsx @@ -96,7 +96,6 @@ import { createAppTree } from '../tree'; import { DefaultProgressComponent, DefaultErrorBoundaryComponent, - DefaultBootErrorPageComponent, DefaultNotFoundErrorPageComponent, } from '../extensions/components'; import { AppNode } from '@backstage/frontend-plugin-api'; @@ -117,7 +116,6 @@ export const builtinExtensions = [ CoreLayout, DefaultProgressComponent, DefaultErrorBoundaryComponent, - DefaultBootErrorPageComponent, DefaultNotFoundErrorPageComponent, LightTheme, DarkTheme, diff --git a/packages/frontend-plugin-api/api-report.md b/packages/frontend-plugin-api/api-report.md index 7342558560..38f6583519 100644 --- a/packages/frontend-plugin-api/api-report.md +++ b/packages/frontend-plugin-api/api-report.md @@ -64,7 +64,6 @@ import { OpenIdConnectApi } from '@backstage/core-plugin-api'; import { PendingOAuthRequest } from '@backstage/core-plugin-api'; import { ProfileInfo } from '@backstage/core-plugin-api'; import { ProfileInfoApi } from '@backstage/core-plugin-api'; -import { PropsWithChildren } from 'react'; import { default as React_2 } from 'react'; import { ReactNode } from 'react'; import { SessionApi } from '@backstage/core-plugin-api'; @@ -321,29 +320,18 @@ export interface ConfigurableExtensionDataRef< } // @public (undocumented) -export type CoreBootErrorPageProps = PropsWithChildren<{ - step: 'load-config' | 'load-chunk'; - error: Error; -}>; - -// @public (undocumented) -export const coreComponentsRefs: { - progress: ComponentRef<{ - children?: ReactNode; - }>; - bootErrorPage: ComponentRef; - notFoundErrorPage: ComponentRef<{ - children?: ReactNode; - }>; +export const coreComponentRefs: { + progress: ComponentRef; + notFoundErrorPage: ComponentRef; errorBoundaryFallback: ComponentRef; }; // @public (undocumented) -export type CoreErrorBoundaryFallbackProps = PropsWithChildren<{ +export type CoreErrorBoundaryFallbackProps = { plugin?: BackstagePlugin; error: Error; resetError: () => void; -}>; +}; // @public (undocumented) export const coreExtensionData: { @@ -364,10 +352,12 @@ export const coreExtensionData: { }; // @public (undocumented) -export type CoreNotFoundErrorPageProps = PropsWithChildren<{}>; +export type CoreNotFoundErrorPageProps = { + children?: ReactNode; +}; // @public (undocumented) -export type CoreProgressProps = PropsWithChildren<{}>; +export type CoreProgressProps = {}; // @public (undocumented) export function createApiExtension< @@ -952,6 +942,11 @@ export { useApi }; export { useApiHolder }; +// @public +export function useComponentRef( + ref: ComponentRef, +): ComponentType; + // @public export function useRouteRef< TOptional extends boolean, diff --git a/packages/frontend-plugin-api/src/apis/definitions/ComponentsApi.ts b/packages/frontend-plugin-api/src/apis/definitions/ComponentsApi.ts index dee17e7d97..c5e9d2c44f 100644 --- a/packages/frontend-plugin-api/src/apis/definitions/ComponentsApi.ts +++ b/packages/frontend-plugin-api/src/apis/definitions/ComponentsApi.ts @@ -15,7 +15,7 @@ */ import { ComponentType } from 'react'; -import { createApiRef } from '@backstage/core-plugin-api'; +import { createApiRef, useApi } from '@backstage/core-plugin-api'; import { ComponentRef } from '../../components'; /** @@ -36,3 +36,14 @@ export interface ComponentsApi { export const componentsApiRef = createApiRef({ id: 'core.components', }); + +/** + * @public + * Returns the component associated with the given ref. + */ +export function useComponentRef( + ref: ComponentRef, +): ComponentType { + const componentsApi = useApi(componentsApiRef); + return componentsApi.getComponent(ref); +} diff --git a/packages/frontend-plugin-api/src/components/ComponentRef.tsx b/packages/frontend-plugin-api/src/components/ComponentRef.tsx index dd2e65a61f..50df9552ec 100644 --- a/packages/frontend-plugin-api/src/components/ComponentRef.tsx +++ b/packages/frontend-plugin-api/src/components/ComponentRef.tsx @@ -15,7 +15,6 @@ */ import { - CoreBootErrorPageProps, CoreErrorBoundaryFallbackProps, CoreNotFoundErrorPageProps, CoreProgressProps, @@ -41,11 +40,6 @@ const coreProgressComponentRef = createComponentRef({ id: 'core.components.progress', }); -const coreBootErrorPageComponentRef = - createComponentRef({ - id: 'core.components.bootErrorPage', - }); - const coreNotFoundErrorPageComponentRef = createComponentRef({ id: 'core.components.notFoundErrorPage', @@ -57,9 +51,8 @@ const coreErrorBoundaryFallbackComponentRef = }); /** @public */ -export const coreComponentsRefs = { +export const coreComponentRefs = { progress: coreProgressComponentRef, - bootErrorPage: coreBootErrorPageComponentRef, notFoundErrorPage: coreNotFoundErrorPageComponentRef, errorBoundaryFallback: coreErrorBoundaryFallbackComponentRef, }; diff --git a/packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx b/packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx index d830f665f0..db5c911cef 100644 --- a/packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx +++ b/packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx @@ -20,16 +20,12 @@ import React, { Suspense, useEffect, } from 'react'; -import { - AnalyticsContext, - useAnalytics, - useApi, -} from '@backstage/core-plugin-api'; +import { AnalyticsContext, useAnalytics } from '@backstage/core-plugin-api'; import { ErrorBoundary } from './ErrorBoundary'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { routableExtensionRenderedEvent } from '../../../core-plugin-api/src/analytics/Tracker'; -import { AppNode, componentsApiRef } from '../apis'; -import { coreComponentsRefs } from './ComponentRef'; +import { AppNode, useComponentRef } from '../apis'; +import { coreComponentRefs } from './ComponentRef'; type RouteTrackerProps = PropsWithChildren<{ disableTracking?: boolean; @@ -61,13 +57,10 @@ export interface ExtensionBoundaryProps { /** @public */ export function ExtensionBoundary(props: ExtensionBoundaryProps) { const { node, routable, children } = props; - const componentsApi = useApi(componentsApiRef); const plugin = node.spec.source; - const Progress = componentsApi.getComponent(coreComponentsRefs.progress); - const fallback = componentsApi.getComponent( - coreComponentsRefs.errorBoundaryFallback, - ); + const Progress = useComponentRef(coreComponentRefs.progress); + const fallback = useComponentRef(coreComponentRefs.errorBoundaryFallback); // Skipping "routeRef" attribute in the new system, the extension "id" should provide more insight const attributes = { diff --git a/packages/frontend-plugin-api/src/components/index.ts b/packages/frontend-plugin-api/src/components/index.ts index a7d947599f..893918059b 100644 --- a/packages/frontend-plugin-api/src/components/index.ts +++ b/packages/frontend-plugin-api/src/components/index.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -export { coreComponentsRefs, type ComponentRef } from './ComponentRef'; +export { coreComponentRefs, type ComponentRef } from './ComponentRef'; export { ExtensionBoundary, diff --git a/packages/frontend-plugin-api/src/index.ts b/packages/frontend-plugin-api/src/index.ts index 04cbddd336..72ebe34795 100644 --- a/packages/frontend-plugin-api/src/index.ts +++ b/packages/frontend-plugin-api/src/index.ts @@ -32,7 +32,6 @@ export * from './wiring'; export type { CoreProgressProps, - CoreBootErrorPageProps, CoreNotFoundErrorPageProps, CoreErrorBoundaryFallbackProps, } from './types'; diff --git a/packages/frontend-plugin-api/src/types.ts b/packages/frontend-plugin-api/src/types.ts index a69c483b5c..4d0fb1ac4b 100644 --- a/packages/frontend-plugin-api/src/types.ts +++ b/packages/frontend-plugin-api/src/types.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { PropsWithChildren } from 'react'; +import { ReactNode } from 'react'; import { BackstagePlugin } from './wiring'; // TODO(Rugvip): This might be a quite useful utility type, maybe add to @backstage/types? @@ -25,20 +25,16 @@ import { BackstagePlugin } from './wiring'; export type Expand = T extends infer O ? { [K in keyof O]: O[K] } : never; /** @public */ -export type CoreProgressProps = PropsWithChildren<{}>; +export type CoreProgressProps = {}; /** @public */ -export type CoreBootErrorPageProps = PropsWithChildren<{ - step: 'load-config' | 'load-chunk'; - error: Error; -}>; +export type CoreNotFoundErrorPageProps = { + children?: ReactNode; +}; /** @public */ -export type CoreNotFoundErrorPageProps = PropsWithChildren<{}>; - -/** @public */ -export type CoreErrorBoundaryFallbackProps = PropsWithChildren<{ +export type CoreErrorBoundaryFallbackProps = { plugin?: BackstagePlugin; error: Error; resetError: () => void; -}>; +}; diff --git a/packages/frontend-plugin-api/src/wiring/coreExtensionData.ts b/packages/frontend-plugin-api/src/wiring/coreExtensionData.ts index 47a77414a0..d52a672679 100644 --- a/packages/frontend-plugin-api/src/wiring/coreExtensionData.ts +++ b/packages/frontend-plugin-api/src/wiring/coreExtensionData.ts @@ -49,5 +49,5 @@ export const coreExtensionData = { component: createExtensionDataRef<{ ref: ComponentRef; impl: ComponentType; - }>('component.ref'), + }>('core.component'), }; diff --git a/yarn.lock b/yarn.lock index 0d8d076b45..9759cf8787 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3916,7 +3916,7 @@ __metadata: languageName: node linkType: hard -"@backstage/core-components@npm:^0.13.7, @backstage/core-components@npm:^0.13.8": +"@backstage/core-components@npm:^0.13.8": version: 0.13.8 resolution: "@backstage/core-components@npm:0.13.8" dependencies: @@ -4042,7 +4042,7 @@ __metadata: languageName: unknown linkType: soft -"@backstage/core-plugin-api@npm:^1.3.0, @backstage/core-plugin-api@npm:^1.5.0, @backstage/core-plugin-api@npm:^1.7.0, @backstage/core-plugin-api@npm:^1.8.0": +"@backstage/core-plugin-api@npm:^1.3.0, @backstage/core-plugin-api@npm:^1.5.0, @backstage/core-plugin-api@npm:^1.8.0": version: 1.8.0 resolution: "@backstage/core-plugin-api@npm:1.8.0" dependencies: @@ -5950,7 +5950,7 @@ __metadata: languageName: unknown linkType: soft -"@backstage/plugin-catalog-react@npm:^1.2.4, @backstage/plugin-catalog-react@npm:^1.8.5, @backstage/plugin-catalog-react@npm:^1.9.1": +"@backstage/plugin-catalog-react@npm:^1.2.4, @backstage/plugin-catalog-react@npm:^1.9.1": version: 1.9.1 resolution: "@backstage/plugin-catalog-react@npm:1.9.1" dependencies: