From 66af240ea692929df76d8f817e96595d648c44d6 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Thu, 19 Oct 2023 14:54:35 +0200 Subject: [PATCH] refactor(frontend-plugin-api): merge boundary and suspense components Signed-off-by: Camila Belo --- packages/frontend-plugin-api/api-report.md | 13 +---- .../src/components/ErrorBoundary.tsx | 3 +- .../src/components/ExtensionBoundary.test.tsx | 6 +-- .../src/components/ExtensionBoundary.tsx | 47 ++++++++++++++----- .../src/components/index.ts | 5 -- .../src/extensions/createPageExtension.tsx | 41 ++++------------ plugins/catalog/src/alpha.tsx | 5 +- plugins/search-react/src/alpha.tsx | 17 +++---- 8 files changed, 57 insertions(+), 80 deletions(-) diff --git a/packages/frontend-plugin-api/api-report.md b/packages/frontend-plugin-api/api-report.md index b6f85e757b..ac8d164fae 100644 --- a/packages/frontend-plugin-api/api-report.md +++ b/packages/frontend-plugin-api/api-report.md @@ -340,7 +340,7 @@ export interface ExtensionBoundaryProps { // (undocumented) id: string; // (undocumented) - routeRef?: RouteRef; + routable?: boolean; // (undocumented) source?: BackstagePlugin; } @@ -416,17 +416,6 @@ export interface ExtensionOverridesOptions { extensions: Extension[]; } -// @public (undocumented) -export function ExtensionSuspense( - props: ExtensionSuspenseProps, -): React_2.JSX.Element; - -// @public (undocumented) -export interface ExtensionSuspenseProps { - // (undocumented) - children: ReactNode; -} - // @public export interface ExternalRouteRef< TParams extends AnyRouteRefParams = AnyRouteRefParams, diff --git a/packages/frontend-plugin-api/src/components/ErrorBoundary.tsx b/packages/frontend-plugin-api/src/components/ErrorBoundary.tsx index f9ce9c9809..1191b75a1d 100644 --- a/packages/frontend-plugin-api/src/components/ErrorBoundary.tsx +++ b/packages/frontend-plugin-api/src/components/ErrorBoundary.tsx @@ -15,9 +15,10 @@ */ import React, { Component, PropsWithChildren } from 'react'; +// TODO: Dependency on MUI should be removed from core packages import { Button } from '@material-ui/core'; -import { BackstagePlugin } from '../wiring'; import { ErrorPanel } from '@backstage/core-components'; +import { BackstagePlugin } from '../wiring'; type DefaultErrorBoundaryFallbackProps = PropsWithChildren<{ plugin?: BackstagePlugin; diff --git a/packages/frontend-plugin-api/src/components/ExtensionBoundary.test.tsx b/packages/frontend-plugin-api/src/components/ExtensionBoundary.test.tsx index 150ace717c..704cafda2d 100644 --- a/packages/frontend-plugin-api/src/components/ExtensionBoundary.test.tsx +++ b/packages/frontend-plugin-api/src/components/ExtensionBoundary.test.tsx @@ -33,7 +33,6 @@ import { analyticsApiRef, useAnalytics } from '@backstage/core-plugin-api'; import { createApp } from '@backstage/frontend-app-api'; import { JsonObject } from '@backstage/types'; import { createRouteRef } from '../routing'; -import { toInternalRouteRef } from '../routing/RouteRef'; function renderExtensionInTestApp( extension: Extension, @@ -59,7 +58,6 @@ function renderExtensionInTestApp( const wrapInBoundaryExtension = (element: JSX.Element) => { const id = 'plugin.extension'; const routeRef = createRouteRef(); - toInternalRouteRef(routeRef).setId(id); return createExtension({ id, attachTo: { id: 'core.routes', input: 'routes' }, @@ -73,7 +71,7 @@ const wrapInBoundaryExtension = (element: JSX.Element) => { routeRef, path: '/', element: ( - + {element} ), @@ -128,7 +126,7 @@ describe('ExtensionBoundary', () => { subject, context: { extension: 'plugin.extension', - routeRef: 'plugin.extension', + routeRef: 'unknown', }, }), ); diff --git a/packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx b/packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx index 796908c619..a9ea297216 100644 --- a/packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx +++ b/packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx @@ -14,36 +14,59 @@ * limitations under the License. */ -import React, { ReactNode } from 'react'; -import { AnalyticsContext } from '@backstage/core-plugin-api'; +import React, { PropsWithChildren, ReactNode, useEffect } from 'react'; +import { AnalyticsContext, useAnalytics } from '@backstage/core-plugin-api'; import { BackstagePlugin } from '../wiring'; -import { RouteRef } from '../routing'; import { ErrorBoundary } from './ErrorBoundary'; -import { toInternalRouteRef } from '../routing/RouteRef'; +import { ExtensionSuspense } from './ExtensionSuspense'; +// eslint-disable-next-line @backstage/no-relative-monorepo-imports +import { routableExtensionRenderedEvent } from '../../../core-plugin-api/src/analytics/Tracker'; + +type RouteTrackerProps = PropsWithChildren<{ + disableTracking?: boolean; +}>; + +const RouteTracker = (props: RouteTrackerProps) => { + const { disableTracking, children } = props; + const analytics = useAnalytics(); + + // This event, never exposed to end-users of the analytics API, + // helps inform which extension metadata gets associated with a + // navigation event when the route navigated to is a gathered + // mountpoint. + useEffect(() => { + if (disableTracking) return; + analytics.captureEvent(routableExtensionRenderedEvent, ''); + }, [analytics, disableTracking]); + + return <>{children}; +}; /** @public */ export interface ExtensionBoundaryProps { id: string; source?: BackstagePlugin; - routeRef?: RouteRef; + routable?: boolean; children: ReactNode; } /** @public */ export function ExtensionBoundary(props: ExtensionBoundaryProps) { - const { id, source, routeRef, children } = props; + const { id, source, routable, children } = props; + // Skipping "routeRef" attribute in the new system, the extension "id" should provide more insight const attributes = { extension: id, pluginId: source?.id, - routeRef: routeRef - ? toInternalRouteRef(routeRef).getDescription() - : undefined, }; return ( - - {children} - + + + + {children} + + + ); } diff --git a/packages/frontend-plugin-api/src/components/index.ts b/packages/frontend-plugin-api/src/components/index.ts index fecd7f36cc..7056a59271 100644 --- a/packages/frontend-plugin-api/src/components/index.ts +++ b/packages/frontend-plugin-api/src/components/index.ts @@ -18,8 +18,3 @@ export { ExtensionBoundary, type ExtensionBoundaryProps, } from './ExtensionBoundary'; - -export { - ExtensionSuspense, - type ExtensionSuspenseProps, -} from './ExtensionSuspense'; diff --git a/packages/frontend-plugin-api/src/extensions/createPageExtension.tsx b/packages/frontend-plugin-api/src/extensions/createPageExtension.tsx index ad7db1d222..f05182149b 100644 --- a/packages/frontend-plugin-api/src/extensions/createPageExtension.tsx +++ b/packages/frontend-plugin-api/src/extensions/createPageExtension.tsx @@ -14,11 +14,8 @@ * limitations under the License. */ -import React, { lazy, useEffect } from 'react'; -import { useAnalytics } from '@backstage/core-plugin-api'; -// eslint-disable-next-line @backstage/no-relative-monorepo-imports -import { routableExtensionRenderedEvent } from '../../../core-plugin-api/src/analytics/Tracker'; -import { ExtensionBoundary, ExtensionSuspense } from '../components'; +import React, { lazy } from 'react'; +import { ExtensionBoundary } from '../components'; import { createSchemaFromZod, PortableSchema } from '../schema'; import { coreExtensionData, @@ -58,9 +55,7 @@ export function createPageExtension< }) => Promise; }, ): Extension { - const { id, routeRef } = options; - - const attachTo = options.attachTo ?? { id: 'core.routes', input: 'routes' }; + const { id } = options; const configSchema = 'configSchema' in options @@ -71,7 +66,7 @@ export function createPageExtension< return createExtension({ id, - attachTo, + attachTo: options.attachTo ?? { id: 'core.routes', input: 'routes' }, configSchema, inputs: options.inputs, disabled: options.disabled, @@ -81,36 +76,18 @@ export function createPageExtension< routeRef: coreExtensionData.routeRef.optional(), }, factory({ bind, config, inputs, source }) { - const { path } = config; - - const PageComponent = lazy(() => + const ExtensionComponent = lazy(() => options .loader({ config, inputs }) .then(element => ({ default: () => element })), ); - const ExtensionComponent = () => { - const analytics = useAnalytics(); - - // This event, never exposed to end-users of the analytics API, - // helps inform which extension metadata gets associated with a - // navigation event when the route navigated to is a gathered - // mountpoint. - useEffect(() => { - analytics.captureEvent(routableExtensionRenderedEvent, ''); - }, [analytics]); - - return ; - }; - bind({ - path, - routeRef, + path: config.path, + routeRef: options.routeRef, element: ( - - - - + + ), }); diff --git a/plugins/catalog/src/alpha.tsx b/plugins/catalog/src/alpha.tsx index e9cb083978..31551c1930 100644 --- a/plugins/catalog/src/alpha.tsx +++ b/plugins/catalog/src/alpha.tsx @@ -36,7 +36,6 @@ import { PortableSchema, ExtensionBoundary, createExtensionInput, - ExtensionSuspense, } from '@backstage/frontend-plugin-api'; import { AsyncEntityProvider, @@ -117,9 +116,7 @@ export function createCatalogFilterExtension< bind({ element: ( - - - + ), }); diff --git a/plugins/search-react/src/alpha.tsx b/plugins/search-react/src/alpha.tsx index 322bd3e67b..f937ef99cc 100644 --- a/plugins/search-react/src/alpha.tsx +++ b/plugins/search-react/src/alpha.tsx @@ -20,7 +20,6 @@ import { ListItemProps } from '@material-ui/core'; import { ExtensionBoundary, - ExtensionSuspense, PortableSchema, createExtension, createExtensionDataRef, @@ -120,15 +119,13 @@ export function createSearchResultListItemExtension< predicate: options.predicate, component: props => ( - - - - - + + + ), },