From 831493278fba15a010a7610792ba35887d95298b Mon Sep 17 00:00:00 2001 From: Levente Balogh Date: Tue, 10 Sep 2024 10:42:07 +0200 Subject: [PATCH] UI Extensions: Share the registries using a React context (#92014) * feat: add a context for the extension registries * feat: add a provider for registries to `AppWrapper` * feat(extensions): add a read-only registry version * feat: share the registry for exposed components using the context * fix: tests * feat: share the registry for added components using the context * feat: share the addedLinks registry using react context * use read-only registry versions --- public/app/AppWrapper.tsx | 21 +-- public/app/app.ts | 24 ++-- .../plugins/components/AppRootPage.test.tsx | 7 +- .../plugins/components/AppRootPage.tsx | 31 ++++- .../extensions/ExtensionRegistriesContext.tsx | 55 ++++++++ .../registry/AddedComponentsRegistry.test.ts | 57 ++++++++ .../registry/AddedComponentsRegistry.ts | 20 ++- .../registry/AddedLinksRegistry.test.ts | 60 +++++++++ .../extensions/registry/AddedLinksRegistry.ts | 20 ++- .../ExportedComponentsRegistry.test.ts | 58 +++++++++ .../registry/ExposedComponentsRegistry.ts | 20 ++- .../plugins/extensions/registry/Registry.ts | 42 ++++-- .../extensions/usePluginComponent.test.tsx | 62 ++++++--- .../plugins/extensions/usePluginComponent.tsx | 35 +++-- .../extensions/usePluginComponents.test.tsx | 57 ++++---- .../extensions/usePluginComponents.tsx | 59 ++++----- .../extensions/usePluginLinks.test.tsx | 45 +++---- .../plugins/extensions/usePluginLinks.tsx | 122 +++++++++--------- 18 files changed, 561 insertions(+), 234 deletions(-) create mode 100644 public/app/features/plugins/extensions/ExtensionRegistriesContext.tsx diff --git a/public/app/AppWrapper.tsx b/public/app/AppWrapper.tsx index 0cbe1f21d78..768a0ff0dff 100644 --- a/public/app/AppWrapper.tsx +++ b/public/app/AppWrapper.tsx @@ -19,6 +19,7 @@ import { sidecarService } from './core/services/SidecarService'; import { contextSrv } from './core/services/context_srv'; import { ThemeProvider } from './core/utils/ConfigProvider'; import { LiveConnectionWarning } from './features/live/LiveConnectionWarning'; +import { ExtensionRegistriesProvider } from './features/plugins/extensions/ExtensionRegistriesContext'; import { ExperimentalSplitPaneRouterWrapper, RouterWrapper } from './routes/RoutesWrapper'; interface AppWrapperProps { @@ -110,15 +111,17 @@ export class AppWrapper extends Component { > -
- {config.featureToggles.appSidecar ? ( - - ) : ( - - )} - - -
+ +
+ {config.featureToggles.appSidecar ? ( + + ) : ( + + )} + + +
+ diff --git a/public/app/app.ts b/public/app/app.ts index bc5dd608787..64d22d7a504 100644 --- a/public/app/app.ts +++ b/public/app/app.ts @@ -84,10 +84,11 @@ import { PanelRenderer } from './features/panel/components/PanelRenderer'; import { DatasourceSrv } from './features/plugins/datasource_srv'; import { createPluginExtensionsGetter } from './features/plugins/extensions/getPluginExtensions'; import { setupPluginExtensionRegistries } from './features/plugins/extensions/registry/setup'; -import { createUsePluginComponent } from './features/plugins/extensions/usePluginComponent'; -import { createUsePluginComponents } from './features/plugins/extensions/usePluginComponents'; +import { PluginExtensionRegistries } from './features/plugins/extensions/registry/types'; +import { usePluginComponent } from './features/plugins/extensions/usePluginComponent'; +import { usePluginComponents } from './features/plugins/extensions/usePluginComponents'; import { createUsePluginExtensions } from './features/plugins/extensions/usePluginExtensions'; -import { createUsePluginLinks } from './features/plugins/extensions/usePluginLinks'; +import { usePluginLinks } from './features/plugins/extensions/usePluginLinks'; import { importPanelPlugin, syncGetPanelPlugin } from './features/plugins/importPanelPlugin'; import { preloadPlugins } from './features/plugins/pluginPreloader'; import { QueryRunner } from './features/query/state/QueryRunner'; @@ -124,6 +125,7 @@ if (process.env.NODE_ENV === 'development') { export class GrafanaApp { context!: GrafanaContextType; + pluginExtensionsRegistries!: PluginExtensionRegistries; async init() { try { @@ -210,7 +212,7 @@ export class GrafanaApp { initWindowRuntime(); // Initialize plugin extensions - const pluginExtensionsRegistries = setupPluginExtensionRegistries(); + this.pluginExtensionsRegistries = setupPluginExtensionRegistries(); if (contextSrv.user.orgRole !== '') { // The "cloud-home-app" is registering banners once it's loaded, and this can cause a rerender in the AppChrome if it's loaded after the Grafana app init. @@ -219,15 +221,15 @@ export class GrafanaApp { const awaitedAppPlugins = Object.values(config.apps).filter((app) => awaitedAppPluginIds.includes(app.id)); const appPlugins = Object.values(config.apps).filter((app) => !awaitedAppPluginIds.includes(app.id)); - preloadPlugins(appPlugins, pluginExtensionsRegistries); - await preloadPlugins(awaitedAppPlugins, pluginExtensionsRegistries, 'frontend_awaited_plugins_preload'); + preloadPlugins(appPlugins, this.pluginExtensionsRegistries); + await preloadPlugins(awaitedAppPlugins, this.pluginExtensionsRegistries, 'frontend_awaited_plugins_preload'); } - setPluginLinksHook(createUsePluginLinks(pluginExtensionsRegistries.addedLinksRegistry)); - setPluginExtensionGetter(createPluginExtensionsGetter(pluginExtensionsRegistries)); - setPluginExtensionsHook(createUsePluginExtensions(pluginExtensionsRegistries)); - setPluginComponentHook(createUsePluginComponent(pluginExtensionsRegistries.exposedComponentsRegistry)); - setPluginComponentsHook(createUsePluginComponents(pluginExtensionsRegistries.addedComponentsRegistry)); + setPluginExtensionGetter(createPluginExtensionsGetter(this.pluginExtensionsRegistries)); + setPluginExtensionsHook(createUsePluginExtensions(this.pluginExtensionsRegistries)); + setPluginLinksHook(usePluginLinks); + setPluginComponentHook(usePluginComponent); + setPluginComponentsHook(usePluginComponents); // initialize chrome service const queryParams = locationService.getSearchObject(); diff --git a/public/app/features/plugins/components/AppRootPage.test.tsx b/public/app/features/plugins/components/AppRootPage.test.tsx index db244781749..028cde06a29 100644 --- a/public/app/features/plugins/components/AppRootPage.test.tsx +++ b/public/app/features/plugins/components/AppRootPage.test.tsx @@ -11,6 +11,8 @@ import { RouteDescriptor } from 'app/core/navigation/types'; import { contextSrv } from 'app/core/services/context_srv'; import { Echo } from 'app/core/services/echo/Echo'; +import { ExtensionRegistriesProvider } from '../extensions/ExtensionRegistriesContext'; +import { setupPluginExtensionRegistries } from '../extensions/registry/setup'; import { getPluginSettings } from '../pluginSettings'; import { importAppPlugin } from '../plugin_loader'; @@ -85,6 +87,7 @@ async function renderUnderRouter(page = '') { appPluginNavItem.parentItem = appsSection; + const registries = setupPluginExtensionRegistries(); const pagePath = page ? `/${page}` : ''; const route = { component: () => , @@ -96,7 +99,9 @@ async function renderUnderRouter(page = '') { render( - } /> + + } /> + ); } diff --git a/public/app/features/plugins/components/AppRootPage.tsx b/public/app/features/plugins/components/AppRootPage.tsx index c01bb2f4941..9eabe95f823 100644 --- a/public/app/features/plugins/components/AppRootPage.tsx +++ b/public/app/features/plugins/components/AppRootPage.tsx @@ -24,6 +24,12 @@ import { appEvents, contextSrv } from 'app/core/core'; import { getNotFoundNav, getWarningNav, getExceptionNav } from 'app/core/navigation/errorModels'; import { getMessageFromError } from 'app/core/utils/errors'; +import { + ExtensionRegistriesProvider, + useAddedLinksRegistry, + useAddedComponentsRegistry, + useExposedComponentsRegistry, +} from '../extensions/ExtensionRegistriesContext'; import { getPluginSettings } from '../pluginSettings'; import { importAppPlugin } from '../plugin_loader'; import { buildPluginSectionNav, pluginsLogger } from '../utils'; @@ -49,6 +55,9 @@ interface State { const initialState: State = { loading: true, loadingError: false, pluginNav: null, plugin: null }; export function AppRootPage({ pluginId, pluginNavSection }: Props) { + const addedLinksRegistry = useAddedLinksRegistry(); + const addedComponentsRegistry = useAddedComponentsRegistry(); + const exposedComponentsRegistry = useExposedComponentsRegistry(); const match = useRouteMatch(); const location = useLocation(); const [state, dispatch] = useReducer(stateSlice.reducer, initialState); @@ -89,13 +98,21 @@ export function AppRootPage({ pluginId, pluginNavSection }: Props) { const pluginRoot = plugin.root && ( - + + + ); diff --git a/public/app/features/plugins/extensions/ExtensionRegistriesContext.tsx b/public/app/features/plugins/extensions/ExtensionRegistriesContext.tsx new file mode 100644 index 00000000000..fd7c146c0ba --- /dev/null +++ b/public/app/features/plugins/extensions/ExtensionRegistriesContext.tsx @@ -0,0 +1,55 @@ +import { PropsWithChildren, createContext, useContext } from 'react'; + +import { AddedComponentsRegistry } from 'app/features/plugins/extensions/registry/AddedComponentsRegistry'; +import { AddedLinksRegistry } from 'app/features/plugins/extensions/registry/AddedLinksRegistry'; +import { ExposedComponentsRegistry } from 'app/features/plugins/extensions/registry/ExposedComponentsRegistry'; + +import { PluginExtensionRegistries } from './registry/types'; + +export interface ExtensionRegistriesContextType { + registries: PluginExtensionRegistries; +} + +// Using a different context for each registry to avoid unnecessary re-renders +export const AddedLinksRegistryContext = createContext(undefined); +export const AddedComponentsRegistryContext = createContext(undefined); +export const ExposedComponentsRegistryContext = createContext(undefined); + +export function useAddedLinksRegistry(): AddedLinksRegistry { + const context = useContext(AddedLinksRegistryContext); + if (!context) { + throw new Error('No `AddedLinksRegistryContext` found.'); + } + return context; +} + +export function useAddedComponentsRegistry(): AddedComponentsRegistry { + const context = useContext(AddedComponentsRegistryContext); + if (!context) { + throw new Error('No `AddedComponentsRegistryContext` found.'); + } + return context; +} + +export function useExposedComponentsRegistry(): ExposedComponentsRegistry { + const context = useContext(ExposedComponentsRegistryContext); + if (!context) { + throw new Error('No `ExposedComponentsRegistryContext` found.'); + } + return context; +} + +export const ExtensionRegistriesProvider = ({ + registries, + children, +}: PropsWithChildren) => { + return ( + + + + {children} + + + + ); +}; diff --git a/public/app/features/plugins/extensions/registry/AddedComponentsRegistry.test.ts b/public/app/features/plugins/extensions/registry/AddedComponentsRegistry.test.ts index 5d5263b5e95..ac0dc845a0a 100644 --- a/public/app/features/plugins/extensions/registry/AddedComponentsRegistry.test.ts +++ b/public/app/features/plugins/extensions/registry/AddedComponentsRegistry.test.ts @@ -2,6 +2,7 @@ import React from 'react'; import { firstValueFrom } from 'rxjs'; import { AddedComponentsRegistry } from './AddedComponentsRegistry'; +import { MSG_CANNOT_REGISTER_READ_ONLY } from './Registry'; describe('AddedComponentsRegistry', () => { const consoleWarn = jest.fn(); @@ -377,4 +378,60 @@ describe('AddedComponentsRegistry', () => { const currentState = await registry.getState(); expect(Object.keys(currentState)).toHaveLength(0); }); + + it('should not be possible to register a component on a read-only registry', async () => { + const registry = new AddedComponentsRegistry(); + const readOnlyRegistry = registry.readOnly(); + + expect(() => { + readOnlyRegistry.register({ + pluginId: 'grafana-basic-app', + configs: [ + { + title: 'Component 1 title', + description: '', + targets: ['grafana/alerting/home'], + component: () => React.createElement('div', null, 'Hello World1'), + }, + ], + }); + }).toThrow(MSG_CANNOT_REGISTER_READ_ONLY); + + const currentState = await readOnlyRegistry.getState(); + expect(Object.keys(currentState)).toHaveLength(0); + }); + + it('should pass down fresh registrations to the read-only version of the registry', async () => { + const pluginId = 'grafana-basic-app'; + const registry = new AddedComponentsRegistry(); + const readOnlyRegistry = registry.readOnly(); + const subscribeCallback = jest.fn(); + let readOnlyState; + + // Should have no extensions registered in the beginning + readOnlyState = await readOnlyRegistry.getState(); + expect(Object.keys(readOnlyState)).toHaveLength(0); + + readOnlyRegistry.asObservable().subscribe(subscribeCallback); + + // Register an extension to the original (writable) registry + registry.register({ + pluginId, + configs: [ + { + title: 'Component 1 title', + description: 'Component 1 description', + targets: ['grafana/alerting/home'], + component: () => React.createElement('div', null, 'Hello World1'), + }, + ], + }); + + // The read-only registry should have received the new extension + readOnlyState = await readOnlyRegistry.getState(); + expect(Object.keys(readOnlyState)).toHaveLength(1); + + expect(subscribeCallback).toHaveBeenCalledTimes(2); + expect(Object.keys(subscribeCallback.mock.calls[1][0])).toEqual(['grafana/alerting/home']); + }); }); diff --git a/public/app/features/plugins/extensions/registry/AddedComponentsRegistry.ts b/public/app/features/plugins/extensions/registry/AddedComponentsRegistry.ts index 954e49b0f34..f95f3167fe6 100644 --- a/public/app/features/plugins/extensions/registry/AddedComponentsRegistry.ts +++ b/public/app/features/plugins/extensions/registry/AddedComponentsRegistry.ts @@ -1,3 +1,5 @@ +import { ReplaySubject } from 'rxjs'; + import { PluginExtensionAddedComponentConfig } from '@grafana/data'; import { logWarning, wrapWithPluginContext } from '../utils'; @@ -21,10 +23,13 @@ export class AddedComponentsRegistry extends Registry< AddedComponentRegistryItem[], PluginExtensionAddedComponentConfig > { - constructor(initialState: RegistryType = {}) { - super({ - initialState, - }); + constructor( + options: { + registrySubject?: ReplaySubject>; + initialState?: RegistryType; + } = {} + ) { + super(options); } mapToRegistry( @@ -83,4 +88,11 @@ export class AddedComponentsRegistry extends Registry< return registry; } + + // Returns a read-only version of the registry. + readOnly() { + return new AddedComponentsRegistry({ + registrySubject: this.registrySubject, + }); + } } diff --git a/public/app/features/plugins/extensions/registry/AddedLinksRegistry.test.ts b/public/app/features/plugins/extensions/registry/AddedLinksRegistry.test.ts index 1135d68e661..a77eddb2a7d 100644 --- a/public/app/features/plugins/extensions/registry/AddedLinksRegistry.test.ts +++ b/public/app/features/plugins/extensions/registry/AddedLinksRegistry.test.ts @@ -1,6 +1,7 @@ import { firstValueFrom } from 'rxjs'; import { AddedLinksRegistry } from './AddedLinksRegistry'; +import { MSG_CANNOT_REGISTER_READ_ONLY } from './Registry'; describe('AddedLinksRegistry', () => { const consoleWarn = jest.fn(); @@ -520,4 +521,63 @@ describe('AddedLinksRegistry', () => { const registry = subscribeCallback.mock.calls[0][0]; expect(registry).toEqual({}); }); + + it('should not be possible to register a link on a read-only registry', async () => { + const pluginId = 'grafana-basic-app'; + const registry = new AddedLinksRegistry(); + const readOnlyRegistry = registry.readOnly(); + + expect(() => { + readOnlyRegistry.register({ + pluginId, + configs: [ + { + title: 'Link 2', + description: 'Link 2 description', + path: `/a/${pluginId}/declare-incident`, + targets: 'plugins/myorg-basic-app/start', + configure: jest.fn().mockReturnValue({}), + }, + ], + }); + }).toThrow(MSG_CANNOT_REGISTER_READ_ONLY); + + const currentState = await readOnlyRegistry.getState(); + expect(Object.keys(currentState)).toHaveLength(0); + }); + + it('should pass down fresh registrations to the read-only version of the registry', async () => { + const pluginId = 'grafana-basic-app'; + const registry = new AddedLinksRegistry(); + const readOnlyRegistry = registry.readOnly(); + const subscribeCallback = jest.fn(); + let readOnlyState; + + // Should have no extensions registered in the beginning + readOnlyState = await readOnlyRegistry.getState(); + expect(Object.keys(readOnlyState)).toHaveLength(0); + + readOnlyRegistry.asObservable().subscribe(subscribeCallback); + + // Register an extension to the original (writable) registry + registry.register({ + pluginId, + configs: [ + { + title: 'Link 2', + description: 'Link 2 description', + path: `/a/${pluginId}/declare-incident`, + targets: 'plugins/myorg-basic-app/start', + configure: jest.fn().mockReturnValue({}), + }, + ], + }); + + // The read-only registry should have received the new extension + readOnlyState = await readOnlyRegistry.getState(); + expect(Object.keys(readOnlyState)).toHaveLength(1); + + expect(subscribeCallback).toHaveBeenCalledTimes(2); + expect(Object.keys(subscribeCallback.mock.calls[1][0])).toEqual(['plugins/myorg-basic-app/start']); + }); }); diff --git a/public/app/features/plugins/extensions/registry/AddedLinksRegistry.ts b/public/app/features/plugins/extensions/registry/AddedLinksRegistry.ts index 30b4ebbdfcc..d547f426258 100644 --- a/public/app/features/plugins/extensions/registry/AddedLinksRegistry.ts +++ b/public/app/features/plugins/extensions/registry/AddedLinksRegistry.ts @@ -1,3 +1,5 @@ +import { ReplaySubject } from 'rxjs'; + import { IconName, PluginExtensionAddedLinkConfig } from '@grafana/data'; import { PluginAddedLinksConfigureFunc, PluginExtensionEventHelpers } from '@grafana/data/src/types/pluginExtensions'; @@ -25,10 +27,13 @@ export type AddedLinkRegistryItem = { }; export class AddedLinksRegistry extends Registry { - constructor(initialState: RegistryType = {}) { - super({ - initialState, - }); + constructor( + options: { + registrySubject?: ReplaySubject>; + initialState?: RegistryType; + } = {} + ) { + super(options); } mapToRegistry( @@ -95,4 +100,11 @@ export class AddedLinksRegistry extends Registry { const consoleWarn = jest.fn(); @@ -339,4 +340,61 @@ describe('ExposedComponentsRegistry', () => { const currentState = await registry.getState(); expect(Object.keys(currentState)).toHaveLength(0); }); + + it('should not be possible to register a component on a read-only registry', async () => { + const pluginId = 'grafana-basic-app'; + const registry = new ExposedComponentsRegistry(); + const readOnlyRegistry = registry.readOnly(); + + expect(() => { + readOnlyRegistry.register({ + pluginId, + configs: [ + { + id: `${pluginId}/hello-world/v1`, + title: 'not important', + description: 'not important', + component: () => React.createElement('div', null, 'Hello World1'), + }, + ], + }); + }).toThrow(MSG_CANNOT_REGISTER_READ_ONLY); + + const currentState = await readOnlyRegistry.getState(); + expect(Object.keys(currentState)).toHaveLength(0); + }); + + it('should pass down fresh registrations to the read-only version of the registry', async () => { + const pluginId = 'grafana-basic-app'; + const registry = new ExposedComponentsRegistry(); + const readOnlyRegistry = registry.readOnly(); + const subscribeCallback = jest.fn(); + let readOnlyState; + + // Should have no extensions registered in the beginning + readOnlyState = await readOnlyRegistry.getState(); + expect(Object.keys(readOnlyState)).toHaveLength(0); + + readOnlyRegistry.asObservable().subscribe(subscribeCallback); + + // Register an extension to the original (writable) registry + registry.register({ + pluginId, + configs: [ + { + id: `${pluginId}/hello-world/v1`, + title: 'not important', + description: 'not important', + component: () => React.createElement('div', null, 'Hello World1'), + }, + ], + }); + + // The read-only registry should have received the new extension + readOnlyState = await readOnlyRegistry.getState(); + expect(Object.keys(readOnlyState)).toHaveLength(1); + + expect(subscribeCallback).toHaveBeenCalledTimes(2); + expect(Object.keys(subscribeCallback.mock.calls[1][0])).toEqual([`${pluginId}/hello-world/v1`]); + }); }); diff --git a/public/app/features/plugins/extensions/registry/ExposedComponentsRegistry.ts b/public/app/features/plugins/extensions/registry/ExposedComponentsRegistry.ts index 40cba5bf44b..90bbca13c73 100644 --- a/public/app/features/plugins/extensions/registry/ExposedComponentsRegistry.ts +++ b/public/app/features/plugins/extensions/registry/ExposedComponentsRegistry.ts @@ -1,3 +1,5 @@ +import { ReplaySubject } from 'rxjs'; + import { PluginExtensionExposedComponentConfig } from '@grafana/data'; import { logWarning } from '../utils'; @@ -16,10 +18,13 @@ export class ExposedComponentsRegistry extends Registry< ExposedComponentRegistryItem, PluginExtensionExposedComponentConfig > { - constructor(initialState: RegistryType = {}) { - super({ - initialState, - }); + constructor( + options: { + registrySubject?: ReplaySubject>; + initialState?: RegistryType; + } = {} + ) { + super(options); } mapToRegistry( @@ -68,4 +73,11 @@ export class ExposedComponentsRegistry extends Registry< return registry; } + + // Returns a read-only version of the registry. + readOnly() { + return new ExposedComponentsRegistry({ + registrySubject: this.registrySubject, + }); + } } diff --git a/public/app/features/plugins/extensions/registry/Registry.ts b/public/app/features/plugins/extensions/registry/Registry.ts index 2d1cb2d895b..33470990e37 100644 --- a/public/app/features/plugins/extensions/registry/Registry.ts +++ b/public/app/features/plugins/extensions/registry/Registry.ts @@ -2,6 +2,8 @@ import { Observable, ReplaySubject, Subject, firstValueFrom, map, scan, startWit import { deepFreeze } from '../utils'; +export const MSG_CANNOT_REGISTER_READ_ONLY = 'Cannot register to a read-only registry'; + export type PluginExtensionConfigs = { pluginId: string; configs: T[]; @@ -9,27 +11,39 @@ export type PluginExtensionConfigs = { export type RegistryType = Record; -type ConstructorOptions = { - initialState: RegistryType; -}; - // This is the base-class used by the separate specific registries. export abstract class Registry { + // Used in cases when we would like to pass a read-only registry to plugin. + // In these cases we are passing in the `registrySubject` to the constructor. + // (If TRUE `initialState` is ignored.) + private isReadOnly: boolean; + // This is the subject that receives extension configs for a loaded plugin. private resultSubject: Subject>; - private registrySubject: ReplaySubject>; + // This is the subject that we expose. + // (It will buffer the last value on the stream - the registry - and emit it to new subscribers immediately.) + protected registrySubject: ReplaySubject>; - constructor(options: ConstructorOptions) { - const { initialState } = options; + constructor(options: { + registrySubject?: ReplaySubject>; + initialState?: RegistryType; + }) { this.resultSubject = new Subject>(); - // This is the subject that we expose. - // (It will buffer the last value on the stream - the registry - and emit it to new subscribers immediately.) - this.registrySubject = new ReplaySubject>(1); + this.isReadOnly = false; + // If the registry subject (observable) is provided, it means that all the registry updates are taken care of outside of this class -> it is read-only. + if (options.registrySubject) { + this.registrySubject = options.registrySubject; + this.isReadOnly = true; + + return; + } + + this.registrySubject = new ReplaySubject>(1); this.resultSubject .pipe( - scan(this.mapToRegistry, initialState), + scan(this.mapToRegistry, options.initialState ?? {}), // Emit an empty registry to start the stream (it is only going to do it once during construction, and then just passes down the values) - startWith(initialState), + startWith(options.initialState ?? {}), map((registry) => deepFreeze(registry)) ) // Emitting the new registry to `this.registrySubject` @@ -42,6 +56,10 @@ export abstract class Registry { ): RegistryType; register(result: PluginExtensionConfigs): void { + if (this.isReadOnly) { + throw new Error(MSG_CANNOT_REGISTER_READ_ONLY); + } + this.resultSubject.next(result); } diff --git a/public/app/features/plugins/extensions/usePluginComponent.test.tsx b/public/app/features/plugins/extensions/usePluginComponent.test.tsx index 46b81ad3325..8b5e644e040 100644 --- a/public/app/features/plugins/extensions/usePluginComponent.test.tsx +++ b/public/app/features/plugins/extensions/usePluginComponent.test.tsx @@ -1,8 +1,13 @@ -import { act, render, screen } from '@testing-library/react'; +import { act, render, screen, waitFor } from '@testing-library/react'; import { renderHook } from '@testing-library/react-hooks'; -import { ExposedComponentsRegistry } from './registry/ExposedComponentsRegistry'; -import { createUsePluginComponent } from './usePluginComponent'; +import { ExtensionRegistriesProvider } from './ExtensionRegistriesContext'; +import { setupPluginExtensionRegistries } from './registry/setup'; +import { PluginExtensionRegistries } from './registry/types'; +import { usePluginComponent } from './usePluginComponent'; +import * as utils from './utils'; + +const wrapWithPluginContext = jest.spyOn(utils, 'wrapWithPluginContext'); jest.mock('app/features/plugins/pluginSettings', () => ({ getPluginSettings: jest.fn().mockResolvedValue({ @@ -16,15 +21,21 @@ jest.mock('app/features/plugins/pluginSettings', () => ({ })); describe('usePluginComponent()', () => { - let registry: ExposedComponentsRegistry; + let registries: PluginExtensionRegistries; + let wrapper: ({ children }: { children: React.ReactNode }) => JSX.Element; beforeEach(() => { - registry = new ExposedComponentsRegistry(); + registries = setupPluginExtensionRegistries(); + + wrapWithPluginContext.mockClear(); + + wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} + ); }); it('should return null if there are no component exposed for the id', () => { - const usePluginComponent = createUsePluginComponent(registry); - const { result } = renderHook(() => usePluginComponent('foo/bar')); + const { result } = renderHook(() => usePluginComponent('foo/bar'), { wrapper }); expect(result.current.component).toEqual(null); expect(result.current.isLoading).toEqual(false); @@ -34,13 +45,12 @@ describe('usePluginComponent()', () => { const id = 'my-app-plugin/foo/bar/v1'; const pluginId = 'my-app-plugin'; - registry.register({ + registries.exposedComponentsRegistry.register({ pluginId, configs: [{ id, title: 'not important', description: 'not important', component: () =>
Hello World
}], }); - const usePluginComponent = createUsePluginComponent(registry); - const { result } = renderHook(() => usePluginComponent(id)); + const { result } = renderHook(() => usePluginComponent(id), { wrapper }); const Component = result.current.component; act(() => { @@ -55,8 +65,7 @@ describe('usePluginComponent()', () => { it('should dynamically update when component is registered to the registry', async () => { const id = 'my-app-plugin/foo/bar/v1'; const pluginId = 'my-app-plugin'; - const usePluginComponent = createUsePluginComponent(registry); - const { result, rerender } = renderHook(() => usePluginComponent(id)); + const { result, rerender } = renderHook(() => usePluginComponent(id), { wrapper }); // No extensions yet expect(result.current.component).toBeNull(); @@ -64,7 +73,7 @@ describe('usePluginComponent()', () => { // Add extensions to the registry act(() => { - registry.register({ + registries.exposedComponentsRegistry.register({ pluginId, configs: [ { @@ -91,12 +100,27 @@ describe('usePluginComponent()', () => { expect(await screen.findByText('Hello World')).toBeVisible(); }); - it('should only render the hook once', () => { - const spy = jest.spyOn(registry, 'asObservable'); - const id = 'my-app-plugin/foo/bar'; - const usePluginComponent = createUsePluginComponent(registry); + it('should only render the hook once', async () => { + const pluginId = 'my-app-plugin'; + const id = `${pluginId}/foo/v1`; - renderHook(() => usePluginComponent(id)); - expect(spy).toHaveBeenCalledTimes(1); + // Add extensions to the registry + act(() => { + registries.exposedComponentsRegistry.register({ + pluginId, + configs: [ + { + id, + title: 'not important', + description: 'not important', + component: () =>
Hello World
, + }, + ], + }); + }); + + expect(wrapWithPluginContext).toHaveBeenCalledTimes(0); + renderHook(() => usePluginComponent(id), { wrapper }); + await waitFor(() => expect(wrapWithPluginContext).toHaveBeenCalledTimes(1)); }); }); diff --git a/public/app/features/plugins/extensions/usePluginComponent.tsx b/public/app/features/plugins/extensions/usePluginComponent.tsx index 8985d182aea..7dfeb3e1f67 100644 --- a/public/app/features/plugins/extensions/usePluginComponent.tsx +++ b/public/app/features/plugins/extensions/usePluginComponent.tsx @@ -3,31 +3,28 @@ import { useObservable } from 'react-use'; import { UsePluginComponentResult } from '@grafana/runtime'; -import { ExposedComponentsRegistry } from './registry/ExposedComponentsRegistry'; +import { useExposedComponentsRegistry } from './ExtensionRegistriesContext'; import { wrapWithPluginContext } from './utils'; // Returns a component exposed by a plugin. // (Exposed components can be defined in plugins by calling .exposeComponent() on the AppPlugin instance.) -export function createUsePluginComponent(registry: ExposedComponentsRegistry) { - const observableRegistry = registry.asObservable(); - - return function usePluginComponent(id: string): UsePluginComponentResult { - const registry = useObservable(observableRegistry); - - return useMemo(() => { - if (!registry?.[id]) { - return { - isLoading: false, - component: null, - }; - } - - const registryItem = registry[id]; +export function usePluginComponent(id: string): UsePluginComponentResult { + const registry = useExposedComponentsRegistry(); + const registryState = useObservable(registry.asObservable()); + return useMemo(() => { + if (!registryState?.[id]) { return { isLoading: false, - component: wrapWithPluginContext(registryItem.pluginId, registryItem.component), + component: null, }; - }, [id, registry]); - }; + } + + const registryItem = registryState[id]; + + return { + isLoading: false, + component: wrapWithPluginContext(registryItem.pluginId, registryItem.component), + }; + }, [id, registryState]); } diff --git a/public/app/features/plugins/extensions/usePluginComponents.test.tsx b/public/app/features/plugins/extensions/usePluginComponents.test.tsx index 1e40a1a4b21..a9855f8006a 100644 --- a/public/app/features/plugins/extensions/usePluginComponents.test.tsx +++ b/public/app/features/plugins/extensions/usePluginComponents.test.tsx @@ -1,8 +1,13 @@ import { act, render, screen } from '@testing-library/react'; import { renderHook } from '@testing-library/react-hooks'; -import { AddedComponentsRegistry } from './registry/AddedComponentsRegistry'; -import { createUsePluginComponents } from './usePluginComponents'; +import { ExtensionRegistriesProvider } from './ExtensionRegistriesContext'; +import { setupPluginExtensionRegistries } from './registry/setup'; +import { PluginExtensionRegistries } from './registry/types'; +import { usePluginComponents } from './usePluginComponents'; +import * as utils from './utils'; + +const wrapWithPluginContext = jest.spyOn(utils, 'wrapWithPluginContext'); jest.mock('app/features/plugins/pluginSettings', () => ({ getPluginSettings: jest.fn().mockResolvedValue({ @@ -16,18 +21,26 @@ jest.mock('app/features/plugins/pluginSettings', () => ({ })); describe('usePluginComponents()', () => { - let registry: AddedComponentsRegistry; + let registries: PluginExtensionRegistries; + let wrapper: ({ children }: { children: React.ReactNode }) => JSX.Element; beforeEach(() => { - registry = new AddedComponentsRegistry(); + registries = setupPluginExtensionRegistries(); + + wrapWithPluginContext.mockClear(); + + wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} + ); }); it('should return an empty array if there are no extensions registered for the extension point', () => { - const usePluginComponents = createUsePluginComponents(registry); - const { result } = renderHook(() => - usePluginComponents({ - extensionPointId: 'foo/bar', - }) + const { result } = renderHook( + () => + usePluginComponents({ + extensionPointId: 'foo/bar', + }), + { wrapper } ); expect(result.current.components).toEqual([]); @@ -37,7 +50,7 @@ describe('usePluginComponents()', () => { const extensionPointId = 'plugins/foo/bar/v1'; const pluginId = 'my-app-plugin'; - registry.register({ + registries.addedComponentsRegistry.register({ pluginId, configs: [ { @@ -61,8 +74,7 @@ describe('usePluginComponents()', () => { ], }); - const usePluginComponents = createUsePluginComponents(registry); - const { result } = renderHook(() => usePluginComponents({ extensionPointId })); + const { result } = renderHook(() => usePluginComponents({ extensionPointId }), { wrapper }); expect(result.current.components.length).toBe(2); @@ -77,15 +89,14 @@ describe('usePluginComponents()', () => { it('should dynamically update the extensions registered for a certain extension point', () => { const extensionPointId = 'plugins/foo/bar/v1'; const pluginId = 'my-app-plugin'; - const usePluginComponents = createUsePluginComponents(registry); - let { result, rerender } = renderHook(() => usePluginComponents({ extensionPointId })); + let { result, rerender } = renderHook(() => usePluginComponents({ extensionPointId }), { wrapper }); // No extensions yet expect(result.current.components.length).toBe(0); // Add extensions to the registry act(() => { - registry.register({ + registries.addedComponentsRegistry.register({ pluginId, configs: [ { @@ -116,20 +127,12 @@ describe('usePluginComponents()', () => { expect(result.current.components.length).toBe(2); }); - it('should only render the hook once', () => { - const spy = jest.spyOn(registry, 'asObservable'); - const extensionPointId = 'plugins/foo/bar'; - const usePluginComponents = createUsePluginComponents(registry); - - renderHook(() => usePluginComponents({ extensionPointId })); - expect(spy).toHaveBeenCalledTimes(1); - }); - it('should honour the limitPerPlugin arg if its set', () => { const extensionPointId = 'plugins/foo/bar/v1'; const plugins = ['my-app-plugin1', 'my-app-plugin2', 'my-app-plugin3']; - const usePluginComponents = createUsePluginComponents(registry); - let { result, rerender } = renderHook(() => usePluginComponents({ extensionPointId, limitPerPlugin: 2 })); + let { result, rerender } = renderHook(() => usePluginComponents({ extensionPointId, limitPerPlugin: 2 }), { + wrapper, + }); // No extensions yet expect(result.current.components.length).toBe(0); @@ -137,7 +140,7 @@ describe('usePluginComponents()', () => { // Add extensions to the registry act(() => { for (let pluginId of plugins) { - registry.register({ + registries.addedComponentsRegistry.register({ pluginId, configs: [ { diff --git a/public/app/features/plugins/extensions/usePluginComponents.tsx b/public/app/features/plugins/extensions/usePluginComponents.tsx index b2ad9804d6b..bb19a09608f 100644 --- a/public/app/features/plugins/extensions/usePluginComponents.tsx +++ b/public/app/features/plugins/extensions/usePluginComponents.tsx @@ -6,42 +6,39 @@ import { UsePluginComponentsResult, } from '@grafana/runtime/src/services/pluginExtensions/getPluginExtensions'; -import { AddedComponentsRegistry } from './registry/AddedComponentsRegistry'; +import { useAddedComponentsRegistry } from './ExtensionRegistriesContext'; // Returns an array of component extensions for the given extension point -export function createUsePluginComponents(registry: AddedComponentsRegistry) { - const observableRegistry = registry.asObservable(); +export function usePluginComponents({ + limitPerPlugin, + extensionPointId, +}: UsePluginComponentOptions): UsePluginComponentsResult { + const registry = useAddedComponentsRegistry(); + const registryState = useObservable(registry.asObservable()); - return function usePluginComponents({ - limitPerPlugin, - extensionPointId, - }: UsePluginComponentOptions): UsePluginComponentsResult { - const registry = useObservable(observableRegistry); + return useMemo(() => { + const components: Array> = []; + const extensionsByPlugin: Record = {}; - return useMemo(() => { - const components: Array> = []; - const extensionsByPlugin: Record = {}; + for (const registryItem of registryState?.[extensionPointId] ?? []) { + const { pluginId } = registryItem; - for (const registryItem of registry?.[extensionPointId] ?? []) { - const { pluginId } = registryItem; - - // Only limit if the `limitPerPlugin` is set - if (limitPerPlugin && extensionsByPlugin[pluginId] >= limitPerPlugin) { - continue; - } - - if (extensionsByPlugin[pluginId] === undefined) { - extensionsByPlugin[pluginId] = 0; - } - - components.push(registryItem.component as React.ComponentType); - extensionsByPlugin[pluginId] += 1; + // Only limit if the `limitPerPlugin` is set + if (limitPerPlugin && extensionsByPlugin[pluginId] >= limitPerPlugin) { + continue; } - return { - isLoading: false, - components, - }; - }, [extensionPointId, limitPerPlugin, registry]); - }; + if (extensionsByPlugin[pluginId] === undefined) { + extensionsByPlugin[pluginId] = 0; + } + + components.push(registryItem.component as React.ComponentType); + extensionsByPlugin[pluginId] += 1; + } + + return { + isLoading: false, + components, + }; + }, [extensionPointId, limitPerPlugin, registryState]); } diff --git a/public/app/features/plugins/extensions/usePluginLinks.test.tsx b/public/app/features/plugins/extensions/usePluginLinks.test.tsx index ceefed61393..c061932ac73 100644 --- a/public/app/features/plugins/extensions/usePluginLinks.test.tsx +++ b/public/app/features/plugins/extensions/usePluginLinks.test.tsx @@ -1,8 +1,10 @@ import { act } from '@testing-library/react'; import { renderHook } from '@testing-library/react-hooks'; -import { AddedLinksRegistry } from './registry/AddedLinksRegistry'; -import { createUsePluginLinks } from './usePluginLinks'; +import { ExtensionRegistriesProvider } from './ExtensionRegistriesContext'; +import { setupPluginExtensionRegistries } from './registry/setup'; +import { PluginExtensionRegistries } from './registry/types'; +import { usePluginLinks } from './usePluginLinks'; jest.mock('app/features/plugins/pluginSettings', () => ({ getPluginSettings: jest.fn().mockResolvedValue({ @@ -16,18 +18,24 @@ jest.mock('app/features/plugins/pluginSettings', () => ({ })); describe('usePluginLinks()', () => { - let registry: AddedLinksRegistry; + let registries: PluginExtensionRegistries; + let wrapper: ({ children }: { children: React.ReactNode }) => JSX.Element; beforeEach(() => { - registry = new AddedLinksRegistry(); + registries = setupPluginExtensionRegistries(); + + wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} + ); }); it('should return an empty array if there are no link extensions registered for the extension point', () => { - const usePluginComponents = createUsePluginLinks(registry); - const { result } = renderHook(() => - usePluginComponents({ - extensionPointId: 'foo/bar', - }) + const { result } = renderHook( + () => + usePluginLinks({ + extensionPointId: 'foo/bar', + }), + { wrapper } ); expect(result.current.links).toEqual([]); @@ -37,7 +45,7 @@ describe('usePluginLinks()', () => { const extensionPointId = 'plugins/foo/bar/v1'; const pluginId = 'my-app-plugin'; - registry.register({ + registries.addedLinksRegistry.register({ pluginId, configs: [ { @@ -61,8 +69,7 @@ describe('usePluginLinks()', () => { ], }); - const usePluginExtensions = createUsePluginLinks(registry); - const { result } = renderHook(() => usePluginExtensions({ extensionPointId })); + const { result } = renderHook(() => usePluginLinks({ extensionPointId }), { wrapper }); expect(result.current.links.length).toBe(2); expect(result.current.links[0].title).toBe('1'); @@ -72,15 +79,14 @@ describe('usePluginLinks()', () => { it('should dynamically update the extensions registered for a certain extension point', () => { const extensionPointId = 'plugins/foo/bar/v1'; const pluginId = 'my-app-plugin'; - const usePluginExtensions = createUsePluginLinks(registry); - let { result, rerender } = renderHook(() => usePluginExtensions({ extensionPointId })); + let { result, rerender } = renderHook(() => usePluginLinks({ extensionPointId }), { wrapper }); // No extensions yet expect(result.current.links.length).toBe(0); // Add extensions to the registry act(() => { - registry.register({ + registries.addedLinksRegistry.register({ pluginId, configs: [ { @@ -106,13 +112,4 @@ describe('usePluginLinks()', () => { expect(result.current.links[0].title).toBe('1'); expect(result.current.links[1].title).toBe('2'); }); - - it('should only render the hook once', () => { - const addedLinksRegistrySpy = jest.spyOn(registry, 'asObservable'); - const extensionPointId = 'plugins/foo/bar'; - const usePluginLinks = createUsePluginLinks(registry); - - renderHook(() => usePluginLinks({ extensionPointId })); - expect(addedLinksRegistrySpy).toHaveBeenCalledTimes(1); - }); }); diff --git a/public/app/features/plugins/extensions/usePluginLinks.tsx b/public/app/features/plugins/extensions/usePluginLinks.tsx index be8f3f5bf03..723ea6cd9bf 100644 --- a/public/app/features/plugins/extensions/usePluginLinks.tsx +++ b/public/app/features/plugins/extensions/usePluginLinks.tsx @@ -8,7 +8,7 @@ import { UsePluginLinksResult, } from '@grafana/runtime/src/services/pluginExtensions/getPluginExtensions'; -import { AddedLinksRegistry } from './registry/AddedLinksRegistry'; +import { useAddedLinksRegistry } from './ExtensionRegistriesContext'; import { generateExtensionId, getLinkExtensionOnClick, @@ -18,69 +18,67 @@ import { } from './utils'; // Returns an array of component extensions for the given extension point -export function createUsePluginLinks(registry: AddedLinksRegistry) { - const observableRegistry = registry.asObservable(); - - return function usePluginLinks({ - limitPerPlugin, - extensionPointId, - context, - }: UsePluginLinksOptions): UsePluginLinksResult { - const registry = useObservable(observableRegistry); - - return useMemo(() => { - if (!registry || !registry[extensionPointId]) { - return { - isLoading: false, - links: [], - }; - } - const frozenContext = context ? getReadOnlyProxy(context) : {}; - const extensions: PluginExtensionLink[] = []; - const extensionsByPlugin: Record = {}; - - for (const addedLink of registry[extensionPointId] ?? []) { - const { pluginId } = addedLink; - // Only limit if the `limitPerPlugin` is set - if (limitPerPlugin && extensionsByPlugin[pluginId] >= limitPerPlugin) { - continue; - } - - if (extensionsByPlugin[pluginId] === undefined) { - extensionsByPlugin[pluginId] = 0; - } - - // Run the configure() function with the current context, and apply the ovverides - const overrides = getLinkExtensionOverrides(pluginId, addedLink, frozenContext); - - // configure() returned an `undefined` -> hide the extension - if (addedLink.configure && overrides === undefined) { - continue; - } - - const path = overrides?.path || addedLink.path; - const extension: PluginExtensionLink = { - id: generateExtensionId(pluginId, extensionPointId, addedLink.title), - type: PluginExtensionTypes.link, - pluginId: pluginId, - onClick: getLinkExtensionOnClick(pluginId, extensionPointId, addedLink, frozenContext), - - // Configurable properties - icon: overrides?.icon || addedLink.icon, - title: overrides?.title || addedLink.title, - description: overrides?.description || addedLink.description, - path: isString(path) ? getLinkExtensionPathWithTracking(pluginId, path, extensionPointId) : undefined, - category: overrides?.category || addedLink.category, - }; - - extensions.push(extension); - extensionsByPlugin[pluginId] += 1; - } +export function usePluginLinks({ + limitPerPlugin, + extensionPointId, + context, +}: UsePluginLinksOptions): UsePluginLinksResult { + const registry = useAddedLinksRegistry(); + const registryState = useObservable(registry.asObservable()); + return useMemo(() => { + if (!registryState || !registryState[extensionPointId]) { return { isLoading: false, - links: extensions, + links: [], }; - }, [context, extensionPointId, limitPerPlugin, registry]); - }; + } + + const frozenContext = context ? getReadOnlyProxy(context) : {}; + const extensions: PluginExtensionLink[] = []; + const extensionsByPlugin: Record = {}; + + for (const addedLink of registryState[extensionPointId] ?? []) { + const { pluginId } = addedLink; + // Only limit if the `limitPerPlugin` is set + if (limitPerPlugin && extensionsByPlugin[pluginId] >= limitPerPlugin) { + continue; + } + + if (extensionsByPlugin[pluginId] === undefined) { + extensionsByPlugin[pluginId] = 0; + } + + // Run the configure() function with the current context, and apply the ovverides + const overrides = getLinkExtensionOverrides(pluginId, addedLink, frozenContext); + + // configure() returned an `undefined` -> hide the extension + if (addedLink.configure && overrides === undefined) { + continue; + } + + const path = overrides?.path || addedLink.path; + const extension: PluginExtensionLink = { + id: generateExtensionId(pluginId, extensionPointId, addedLink.title), + type: PluginExtensionTypes.link, + pluginId: pluginId, + onClick: getLinkExtensionOnClick(pluginId, extensionPointId, addedLink, frozenContext), + + // Configurable properties + icon: overrides?.icon || addedLink.icon, + title: overrides?.title || addedLink.title, + description: overrides?.description || addedLink.description, + path: isString(path) ? getLinkExtensionPathWithTracking(pluginId, path, extensionPointId) : undefined, + category: overrides?.category || addedLink.category, + }; + + extensions.push(extension); + extensionsByPlugin[pluginId] += 1; + } + + return { + isLoading: false, + links: extensions, + }; + }, [context, extensionPointId, limitPerPlugin, registryState]); }