Extensions: Simplify the configure() function (#64958)

refactor: stop passing the extension to the `configure()` function
This commit is contained in:
Levente Balogh 2023-03-20 10:41:15 +01:00 committed by GitHub
parent 46cba5f993
commit 788e4a7f19
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 33 additions and 70 deletions

View File

@ -74,7 +74,7 @@ export type AppPluginExtensionLinkConfig<C extends object = object> = {
description: string;
placement: string;
path: string;
configure?: (extension: AppPluginExtensionLink, context?: C) => Partial<AppPluginExtensionLink> | undefined;
configure?: (context?: C) => Partial<AppPluginExtensionLink> | undefined;
};
export type AppPluginExtensionCommandConfig<C extends object = object> = {
@ -82,7 +82,7 @@ export type AppPluginExtensionCommandConfig<C extends object = object> = {
description: string;
placement: string;
handler: (context?: C, helpers?: AppPluginExtensionCommandHelpers) => void;
configure?: (extension: AppPluginExtensionCommand, context?: C) => Partial<AppPluginExtensionCommand> | undefined;
configure?: (context?: C) => Partial<AppPluginExtensionCommand> | undefined;
};
export class AppPlugin<T extends KeyValue = KeyValue> extends GrafanaPlugin<AppPluginMeta<T>> {

View File

@ -20,9 +20,12 @@ export function getPluginExtensions<T extends object = {}>(
const extensions = configureFuncs.reduce<PluginExtension[]>((result, configure) => {
const extension = configure(context);
// If the configure() function returns `undefined`, the extension is not displayed
if (extension) {
result.push(extension);
}
return result;
}, []);

View File

@ -13,11 +13,6 @@ describe('error handling for extensions', () => {
});
const context = {};
const extension: AppPluginExtensionLink = {
title: 'Go to page one',
description: 'Will navigate the user to page one',
path: `/a/${pluginId}/one`,
};
it('should return configured link if configure is successful', () => {
const configureWithErrorHandling = errorHandler(() => {
@ -26,7 +21,7 @@ describe('error handling for extensions', () => {
};
});
const configured = configureWithErrorHandling(extension, context);
const configured = configureWithErrorHandling(context);
expect(configured).toEqual({
title: 'This is a new title',
@ -38,7 +33,7 @@ describe('error handling for extensions', () => {
throw new Error();
});
const configured = configureWithErrorHandling(extension, context);
const configured = configureWithErrorHandling(context);
expect(configured).toBeUndefined();
});
@ -47,7 +42,7 @@ describe('error handling for extensions', () => {
const promisebased = (async () => {}) as ConfigureFunc<AppPluginExtensionLink>;
const configureWithErrorHandling = errorHandler(promisebased);
const configured = configureWithErrorHandling(extension, context);
const configured = configureWithErrorHandling(context);
expect(configured).toBeUndefined();
});
@ -56,7 +51,7 @@ describe('error handling for extensions', () => {
const objectbased = {} as ConfigureFunc<AppPluginExtensionLink>;
const configureWithErrorHandling = errorHandler(objectbased);
const configured = configureWithErrorHandling(extension, context);
const configured = configureWithErrorHandling(context);
expect(configured).toBeUndefined();
});
@ -65,7 +60,7 @@ describe('error handling for extensions', () => {
const returnString = (() => '') as ConfigureFunc<AppPluginExtensionLink>;
const configureWithErrorHandling = errorHandler(returnString);
const configured = configureWithErrorHandling(extension, context);
const configured = configureWithErrorHandling(context);
expect(configured).toBeUndefined();
});
@ -74,7 +69,7 @@ describe('error handling for extensions', () => {
const returnUndefined = () => undefined;
const configureWithErrorHandling = errorHandler(returnUndefined);
const configured = configureWithErrorHandling(extension, context);
const configured = configureWithErrorHandling(context);
expect(configured).toBeUndefined();
});

View File

@ -12,14 +12,14 @@ export function handleErrorsInConfigure<T>(options: Options) {
const { pluginId, title, logger } = options;
return (configure: ConfigureFunc<T>): ConfigureFunc<T> => {
return function handleErrors(extension, context) {
return function handleErrors(context) {
try {
if (!isFunction(configure)) {
logger(`[Plugins] ${pluginId} provided invalid configuration function for extension '${title}'.`);
return;
}
const result = configure(extension, context);
const result = configure(context);
if (result instanceof Promise) {
logger(
`[Plugins] ${pluginId} provided an unsupported async/promise-based configureation function for extension '${title}'.`

View File

@ -8,15 +8,15 @@ import { PluginExtensionRegistry } from '@grafana/runtime';
import { createPluginExtensionRegistry } from './registryFactory';
const validateLink = jest.fn((configure, extension, context) => configure?.(extension, context));
const configureErrorHandler = jest.fn((configure, extension, context) => configure?.(extension, context));
const validateLink = jest.fn((configure, context) => configure?.(context));
const configureErrorHandler = jest.fn((configure, context) => configure?.(context));
const commandErrorHandler = jest.fn((configure, context) => configure?.(context));
jest.mock('./errorHandling', () => ({
...jest.requireActual('./errorHandling'),
handleErrorsInConfigure: jest.fn(() => {
return jest.fn((configure) => {
return jest.fn((extension, context) => configureErrorHandler(configure, extension, context));
return jest.fn((context) => configureErrorHandler(configure, context));
});
}),
handleErrorsInHandler: jest.fn(() => {
@ -30,7 +30,7 @@ jest.mock('./validateLink', () => ({
...jest.requireActual('./validateLink'),
createLinkValidator: jest.fn(() => {
return jest.fn((configure) => {
return jest.fn((extension, context) => validateLink(configure, extension, context));
return jest.fn((context) => validateLink(configure, context));
});
}),
}));
@ -205,15 +205,10 @@ describe('createPluginExtensionRegistry()', () => {
const [configure] = registry[linkConfig.placement];
const context = {};
const configurable = {
title: linkConfig.title,
description: linkConfig.description,
path: linkConfig.path,
};
configure(context);
expect(validateLink).toBeCalledWith(expect.any(Function), configurable, context);
expect(validateLink).toBeCalledWith(expect.any(Function), context);
});
it('should wrap configure function with extension error handling', () => {
@ -232,15 +227,10 @@ describe('createPluginExtensionRegistry()', () => {
const [configure] = registry[linkConfig.placement];
const context = {};
const configurable = {
title: linkConfig.title,
description: linkConfig.description,
path: linkConfig.path,
};
configure(context);
expect(configureErrorHandler).toBeCalledWith(expect.any(Function), configurable, context);
expect(configureErrorHandler).toBeCalledWith(expect.any(Function), context);
});
it('should return undefined if returned by the provided extension config', () => {
@ -394,15 +384,11 @@ describe('createPluginExtensionRegistry()', () => {
const [configure] = registry[commandConfig1.placement];
const context = {};
const configurable = {
title: commandConfig1.title,
description: commandConfig2.description,
};
configure(context);
// The error handler is wrapping (decorating) the configure function, so it can provide standard error messages
expect(configureErrorHandler).toBeCalledWith(expect.any(Function), configurable, context);
expect(configureErrorHandler).toBeCalledWith(expect.any(Function), context);
});
it('should return undefined if returned by the provided extension config', () => {

View File

@ -86,13 +86,7 @@ function createCommandRegistryItem(
const handler = catchErrorsInHandler(handlerWithHelpers);
const extensionFactory = createCommandFactory(pluginId, config, handler);
const configurable: AppPluginExtensionCommand = {
title: config.title,
description: config.description,
};
const mapper = mapToConfigure<PluginExtensionCommand, AppPluginExtensionCommand>(extensionFactory, configurable);
const mapper = mapToConfigure<PluginExtensionCommand, AppPluginExtensionCommand>(extensionFactory);
const catchErrorsInConfigure = handleErrorsInConfigure<AppPluginExtensionCommand>(options);
return mapper(catchErrorsInConfigure(configure));
@ -110,14 +104,7 @@ function createLinkRegistryItem(
const options = { pluginId: pluginId, title: config.title, logger: console.warn };
const extensionFactory = createLinkFactory(pluginId, config);
const configurable: AppPluginExtensionLink = {
title: config.title,
description: config.description,
path: config.path,
};
const mapper = mapToConfigure<PluginExtensionLink, AppPluginExtensionLink>(extensionFactory, configurable);
const mapper = mapToConfigure<PluginExtensionLink, AppPluginExtensionLink>(extensionFactory);
const withConfigureErrorHandling = handleErrorsInConfigure<AppPluginExtensionLink>(options);
const validateLink = createLinkValidator(options);
@ -125,7 +112,7 @@ function createLinkRegistryItem(
}
function createLinkFactory(pluginId: string, config: AppPluginExtensionLinkConfig) {
return (override: Partial<AppPluginExtensionLink>, context?: object): PluginExtensionLink => {
return (override: Partial<AppPluginExtensionLink>): PluginExtensionLink => {
const title = override?.title ?? config.title;
const description = override?.description ?? config.description;
const path = override?.path ?? config.path;
@ -160,16 +147,15 @@ function createCommandFactory(
}
function mapToConfigure<T extends PluginExtension, C>(
commandFactory: (override: Partial<C>, context?: object) => T | undefined,
configurable: C
extensionFactory: (override: Partial<C>, context?: object) => T | undefined
): (configure: ConfigureFunc<C>) => PluginExtensionRegistryItem<T> {
return (configure) => {
return function mapToExtension(context?: object): T | undefined {
const override = configure(configurable, context);
const override = configure(context);
if (!override) {
return undefined;
}
return commandFactory(override, context);
return extensionFactory(override, context);
};
};
}

View File

@ -1,4 +1,4 @@
import type { AppPluginExtensionCommandConfig } from '@grafana/data';
export type CommandHandlerFunc = AppPluginExtensionCommandConfig['handler'];
export type ConfigureFunc<T> = (extension: T, context?: object) => Partial<T> | undefined;
export type ConfigureFunc<T> = (context?: object) => Partial<T> | undefined;

View File

@ -1,5 +1,3 @@
import type { AppPluginExtensionLink } from '@grafana/data';
import { createLinkValidator } from './validateLink';
describe('extension link validator', () => {
@ -11,11 +9,6 @@ describe('extension link validator', () => {
});
const context = {};
const extension: AppPluginExtensionLink = {
title: 'Go to page one',
description: 'Will navigate the user to page one',
path: `/a/${pluginId}/one`,
};
it('should return link configuration if path is valid', () => {
const configureWithValidation = validator(() => {
@ -24,7 +17,7 @@ describe('extension link validator', () => {
};
});
const configured = configureWithValidation(extension, context);
const configured = configureWithValidation(context);
expect(configured).toEqual({
path: `/a/${pluginId}/other`,
});
@ -37,7 +30,7 @@ describe('extension link validator', () => {
};
});
const configured = configureWithValidation(extension, context);
const configured = configureWithValidation(context);
expect(configured).toEqual({ title: 'Go to page two' });
});
@ -48,7 +41,7 @@ describe('extension link validator', () => {
};
});
const configured = configureWithValidation(extension, context);
const configured = configureWithValidation(context);
expect(configured).toBeUndefined();
});
@ -57,7 +50,7 @@ describe('extension link validator', () => {
return undefined;
});
const configured = configureWithValidation(extension, context);
const configured = configureWithValidation(context);
expect(configured).toBeUndefined();
});
});

View File

@ -14,8 +14,8 @@ export function createLinkValidator(options: Options) {
const { pluginId, title, logger } = options;
return (configure: ConfigureFunc<AppPluginExtensionLink>): ConfigureFunc<AppPluginExtensionLink> => {
return function validateLink(link, context) {
const configured = configure(link, context);
return function validateLink(context) {
const configured = configure(context);
if (!isString(configured?.path)) {
return configured;