From 9b89b82f66b500dabc5493a51d2fd1144e4828a4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 18 Jul 2024 12:08:15 +0200 Subject: [PATCH] frontend-plugin-api: infer ExtensionBoundary routable prop from outputs Signed-off-by: Patrik Oldsberg --- .changeset/early-trees-dance.md | 5 ++ .changeset/mighty-dolls-retire.md | 5 ++ .../architecture/03-extensions.md | 4 +- packages/frontend-plugin-api/api-report.md | 1 - .../src/components/ExtensionBoundary.test.tsx | 72 ++++++++++++++++--- .../src/components/ExtensionBoundary.tsx | 14 +++- .../src/extensions/createPageExtension.tsx | 2 +- plugins/catalog-react/src/alpha.tsx | 2 +- 8 files changed, 91 insertions(+), 14 deletions(-) create mode 100644 .changeset/early-trees-dance.md create mode 100644 .changeset/mighty-dolls-retire.md diff --git a/.changeset/early-trees-dance.md b/.changeset/early-trees-dance.md new file mode 100644 index 0000000000..5422bd2872 --- /dev/null +++ b/.changeset/early-trees-dance.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-plugin-api': patch +--- + +The `ExtensionBoundary` now by default infers whether its routable from whether it outputs a route path. diff --git a/.changeset/mighty-dolls-retire.md b/.changeset/mighty-dolls-retire.md new file mode 100644 index 0000000000..b31c443a38 --- /dev/null +++ b/.changeset/mighty-dolls-retire.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-react': patch +--- + +Internal refactor to remove unnecessary `routable` prop in the implementation of the `createEntityContentExtension` alpha export. diff --git a/docs/frontend-system/architecture/03-extensions.md b/docs/frontend-system/architecture/03-extensions.md index 19345e0b26..cdc73a17ec 100644 --- a/docs/frontend-system/architecture/03-extensions.md +++ b/docs/frontend-system/architecture/03-extensions.md @@ -337,7 +337,7 @@ Similar to plugins the `ErrorBoundary` for extension allows to pass in a fallbac ### Analytics -Analytics information are provided through the `AnalyticsContext`, which will give `extensionId` & `pluginId` as context to analytics event fired inside of the extension. Additionally `RouteTracker` will capture an analytics event for routable extension to inform which extension metadata gets associated with a navigation event when the route navigated to is a gathered `mountPoint`. +Analytics information are provided through the `AnalyticsContext`, which will give `extensionId` & `pluginId` as context to analytics event fired inside of the extension. Additionally `RouteTracker` will capture an analytics event for routable extension to inform which extension metadata gets associated with a navigation event when the route navigated to is a gathered `mountPoint`. Whether an extension is routable is inferred from its outputs, but you can also explicitly control this behavior by passing the `routable` prop to `ExtensionBoundary`. The `ExtensionBoundary` can be used like the following in an extension creator: @@ -359,7 +359,7 @@ export function createSomeExtension< path: config.path, routeRef: options.routeRef, element: ( - + ), diff --git a/packages/frontend-plugin-api/api-report.md b/packages/frontend-plugin-api/api-report.md index 3e19573d95..d50ef0dd03 100644 --- a/packages/frontend-plugin-api/api-report.md +++ b/packages/frontend-plugin-api/api-report.md @@ -844,7 +844,6 @@ export interface ExtensionBoundaryProps { children: ReactNode; // (undocumented) node: AppNode; - // (undocumented) routable?: boolean; } diff --git a/packages/frontend-plugin-api/src/components/ExtensionBoundary.test.tsx b/packages/frontend-plugin-api/src/components/ExtensionBoundary.test.tsx index 66fb3901fc..1f12f127e8 100644 --- a/packages/frontend-plugin-api/src/components/ExtensionBoundary.test.tsx +++ b/packages/frontend-plugin-api/src/components/ExtensionBoundary.test.tsx @@ -15,15 +15,24 @@ */ import React, { useEffect } from 'react'; -import { screen, waitFor } from '@testing-library/react'; -import { MockAnalyticsApi, TestApiProvider } from '@backstage/test-utils'; +import { act, screen, waitFor } from '@testing-library/react'; +import { + MockAnalyticsApi, + TestApiProvider, + withLogCollector, +} from '@backstage/test-utils'; import { ExtensionBoundary } from './ExtensionBoundary'; import { coreExtensionData, createExtension } from '../wiring'; -import { analyticsApiRef, useAnalytics } from '@backstage/core-plugin-api'; +import { + analyticsApiRef, + createApiFactory, + useAnalytics, +} from '@backstage/core-plugin-api'; import { createRouteRef } from '../routing'; import { createExtensionTester } from '@backstage/frontend-test-utils'; +import { createApiExtension } from '../extensions'; -const wrapInBoundaryExtension = (element: JSX.Element) => { +const wrapInBoundaryExtension = (element?: JSX.Element) => { const routeRef = createRouteRef(); return createExtension({ name: 'test', @@ -54,12 +63,25 @@ describe('ExtensionBoundary', () => { }); it('should show app error component when an error is thrown', async () => { - const error = 'Something went wrong'; + const errorMsg = 'Something went wrong'; const ErrorComponent = () => { - throw new Error(error); + throw new Error(errorMsg); }; - createExtensionTester(wrapInBoundaryExtension()).render(); - await waitFor(() => expect(screen.getByText(error)).toBeInTheDocument()); + const { error } = await withLogCollector(['error'], async () => { + createExtensionTester( + wrapInBoundaryExtension(), + ).render(); + await waitFor(() => + expect(screen.getByText(errorMsg)).toBeInTheDocument(), + ); + }); + expect(error).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + message: expect.stringContaining(errorMsg), + }), + ]), + ); }); it('should wrap children with analytics context', async () => { @@ -97,4 +119,38 @@ describe('ExtensionBoundary', () => { }); }); }); + + // TODO(Rugvip): It's annoying to test the inverse of this currently, because the extension tester overrides the subject to always output a path + it('should emit analytics events if routable', async () => { + const Emitter = () => { + const analytics = useAnalytics(); + useEffect(() => { + analytics.captureEvent('dummy', 'dummy'); + }); + return null; + }; + const analyticsApiMock = new MockAnalyticsApi(); + + await act(async () => { + createExtensionTester(wrapInBoundaryExtension()) + .add( + createApiExtension({ + factory: createApiFactory(analyticsApiRef, analyticsApiMock), + }), + ) + .render(); + }); + + expect(analyticsApiMock.getEvents()).toEqual([ + expect.objectContaining({ + action: 'navigate', + subject: '/', + context: expect.objectContaining({ + pluginId: 'root', + extensionId: 'test', + }), + }), + expect.objectContaining({ action: 'dummy' }), + ]); + }); }); diff --git a/packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx b/packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx index 768352b76e..b6269484b0 100644 --- a/packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx +++ b/packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx @@ -26,6 +26,7 @@ import { ErrorBoundary } from './ErrorBoundary'; import { routableExtensionRenderedEvent } from '../../../core-plugin-api/src/analytics/Tracker'; import { AppNode, useComponentRef } from '../apis'; import { coreComponentRefs } from './coreComponentRefs'; +import { coreExtensionData } from '../wiring'; type RouteTrackerProps = PropsWithChildren<{ disableTracking?: boolean; @@ -50,6 +51,11 @@ const RouteTracker = (props: RouteTrackerProps) => { /** @public */ export interface ExtensionBoundaryProps { node: AppNode; + /** + * This explicitly marks the extension as routable for the purpose of + * capturing analytics events. If not provided, the extension boundary will be + * marked as routable if it outputs a routePath. + */ routable?: boolean; children: ReactNode; } @@ -58,6 +64,10 @@ export interface ExtensionBoundaryProps { export function ExtensionBoundary(props: ExtensionBoundaryProps) { const { node, routable, children } = props; + const doesOutputRoutePath = Boolean( + node.instance?.getData(coreExtensionData.routePath), + ); + const plugin = node.spec.source; const Progress = useComponentRef(coreComponentRefs.progress); const fallback = useComponentRef(coreComponentRefs.errorBoundaryFallback); @@ -72,7 +82,9 @@ export function ExtensionBoundary(props: ExtensionBoundaryProps) { }> - {children} + + {children} + diff --git a/packages/frontend-plugin-api/src/extensions/createPageExtension.tsx b/packages/frontend-plugin-api/src/extensions/createPageExtension.tsx index dfe549f32b..cd434a3ffc 100644 --- a/packages/frontend-plugin-api/src/extensions/createPageExtension.tsx +++ b/packages/frontend-plugin-api/src/extensions/createPageExtension.tsx @@ -87,7 +87,7 @@ export function createPageExtension< path: config.path, routeRef: options.routeRef, element: ( - + ), diff --git a/plugins/catalog-react/src/alpha.tsx b/plugins/catalog-react/src/alpha.tsx index b1650d1f5b..85712b358a 100644 --- a/plugins/catalog-react/src/alpha.tsx +++ b/plugins/catalog-react/src/alpha.tsx @@ -166,7 +166,7 @@ export function createEntityContentExtension< title: config.title, routeRef: options.routeRef, element: ( - + ),