From 8f7bb05169d148751fc66e235dfe77ab0315dee0 Mon Sep 17 00:00:00 2001 From: Vadim Dalecky Date: Mon, 20 Apr 2020 15:04:14 +0200 Subject: [PATCH] [Discuss] Remove expressions plugin's dependency on inspector plugin (#63841) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: 💡 use inspector service in visualizations to open it * refactor: 💡 remove expressions plugin dependency on inspector * test: 💍 fix Jest mock * fix: 🐛 remove Inspectore from Expressions plugin dependency inf * docs: ✏️ add JSDocs for createStartServicesGetter() method * test: 💍 fix TypeScript errors in expressions mocks --- src/plugins/expressions/kibana.json | 3 +- src/plugins/expressions/public/loader.ts | 16 ++----- src/plugins/expressions/public/mocks.tsx | 3 -- src/plugins/expressions/public/plugin.ts | 9 +--- .../public/react_expression_renderer.tsx | 3 +- src/plugins/expressions/public/render.ts | 3 +- src/plugins/expressions/public/services.ts | 3 -- .../core/create_start_service_getter.ts | 42 +++++++++++++++++++ src/plugins/visualizations/kibana.json | 2 +- .../create_vis_embeddable_from_object.ts | 4 +- .../public/embeddable/visualize_embeddable.ts | 17 ++++++-- .../visualize_embeddable_factory.tsx | 13 ++++-- src/plugins/visualizations/public/mocks.ts | 5 ++- src/plugins/visualizations/public/plugin.ts | 34 ++++++++++----- 14 files changed, 107 insertions(+), 50 deletions(-) diff --git a/src/plugins/expressions/kibana.json b/src/plugins/expressions/kibana.json index cba693dd4bc20..5d2112103e94d 100644 --- a/src/plugins/expressions/kibana.json +++ b/src/plugins/expressions/kibana.json @@ -4,7 +4,6 @@ "server": true, "ui": true, "requiredPlugins": [ - "bfetch", - "inspector" + "bfetch" ] } diff --git a/src/plugins/expressions/public/loader.ts b/src/plugins/expressions/public/loader.ts index fbe2f37c648d6..418ff6fdf8614 100644 --- a/src/plugins/expressions/public/loader.ts +++ b/src/plugins/expressions/public/loader.ts @@ -19,13 +19,14 @@ import { BehaviorSubject, Observable, Subject } from 'rxjs'; import { filter, map } from 'rxjs/operators'; -import { Adapters, InspectorSession } from '../../inspector/public'; -import { ExpressionRenderHandler } from './render'; +import { Adapters } from '../../inspector/public'; import { IExpressionLoaderParams } from './types'; import { ExpressionAstExpression } from '../common'; -import { getInspector, getExpressionsService } from './services'; import { ExecutionContract } from '../common/execution/execution_contract'; +import { ExpressionRenderHandler } from './render'; +import { getExpressionsService } from './services'; + type Data = any; export class ExpressionLoader { @@ -120,15 +121,6 @@ export class ExpressionLoader { return this.renderHandler.getElement(); } - openInspector(title: string): InspectorSession | undefined { - const inspector = this.inspect(); - if (inspector) { - return getInspector().open(inspector, { - title, - }); - } - } - inspect(): Adapters | undefined { return this.execution ? (this.execution.inspect() as Adapters) : undefined; } diff --git a/src/plugins/expressions/public/mocks.tsx b/src/plugins/expressions/public/mocks.tsx index cb7089f814643..b8f2f693e9c77 100644 --- a/src/plugins/expressions/public/mocks.tsx +++ b/src/plugins/expressions/public/mocks.tsx @@ -22,7 +22,6 @@ import { ExpressionsSetup, ExpressionsStart, plugin as pluginInitializer } from /* eslint-disable */ import { coreMock } from '../../../core/public/mocks'; -import { inspectorPluginMock } from '../../inspector/public/mocks'; import { bfetchPluginMock } from '../../bfetch/public/mocks'; /* eslint-enable */ @@ -89,7 +88,6 @@ const createPlugin = async () => { const plugin = pluginInitializer(pluginInitializerContext); const setup = await plugin.setup(coreSetup, { bfetch: bfetchPluginMock.createSetupContract(), - inspector: inspectorPluginMock.createSetupContract(), }); return { @@ -101,7 +99,6 @@ const createPlugin = async () => { doStart: async () => await plugin.start(coreStart, { bfetch: bfetchPluginMock.createStartContract(), - inspector: inspectorPluginMock.createStartContract(), }), }; }; diff --git a/src/plugins/expressions/public/plugin.ts b/src/plugins/expressions/public/plugin.ts index 7c0de271b7706..720c3b701d504 100644 --- a/src/plugins/expressions/public/plugin.ts +++ b/src/plugins/expressions/public/plugin.ts @@ -29,11 +29,9 @@ import { ExpressionsServiceStart, ExecutionContext, } from '../common'; -import { Setup as InspectorSetup, Start as InspectorStart } from '../../inspector/public'; import { BfetchPublicSetup, BfetchPublicStart } from '../../bfetch/public'; import { setCoreStart, - setInspector, setInterpreter, setRenderersRegistry, setNotifications, @@ -45,12 +43,10 @@ import { render, ExpressionRenderHandler } from './render'; export interface ExpressionsSetupDeps { bfetch: BfetchPublicSetup; - inspector: InspectorSetup; } export interface ExpressionsStartDeps { bfetch: BfetchPublicStart; - inspector: InspectorStart; } export interface ExpressionsSetup extends ExpressionsServiceSetup { @@ -120,7 +116,7 @@ export class ExpressionsPublicPlugin }); } - public setup(core: CoreSetup, { inspector, bfetch }: ExpressionsSetupDeps): ExpressionsSetup { + public setup(core: CoreSetup, { bfetch }: ExpressionsSetupDeps): ExpressionsSetup { this.configureExecutor(core); const { expressions } = this; @@ -180,9 +176,8 @@ export class ExpressionsPublicPlugin return Object.freeze(setup); } - public start(core: CoreStart, { inspector, bfetch }: ExpressionsStartDeps): ExpressionsStart { + public start(core: CoreStart, { bfetch }: ExpressionsStartDeps): ExpressionsStart { setCoreStart(core); - setInspector(inspector); setNotifications(core.notifications); const { expressions } = this; diff --git a/src/plugins/expressions/public/react_expression_renderer.tsx b/src/plugins/expressions/public/react_expression_renderer.tsx index 242a49c6d6639..2c99f173c9f33 100644 --- a/src/plugins/expressions/public/react_expression_renderer.tsx +++ b/src/plugins/expressions/public/react_expression_renderer.tsx @@ -17,8 +17,7 @@ * under the License. */ -import { useRef, useEffect, useState, useLayoutEffect } from 'react'; -import React from 'react'; +import React, { useRef, useEffect, useState, useLayoutEffect } from 'react'; import classNames from 'classnames'; import { Subscription } from 'rxjs'; import { filter } from 'rxjs/operators'; diff --git a/src/plugins/expressions/public/render.ts b/src/plugins/expressions/public/render.ts index ad4d16bcd1323..4aaf0da60fc60 100644 --- a/src/plugins/expressions/public/render.ts +++ b/src/plugins/expressions/public/render.ts @@ -21,10 +21,11 @@ import * as Rx from 'rxjs'; import { Observable } from 'rxjs'; import { filter } from 'rxjs/operators'; import { RenderError, RenderErrorHandlerFnType, IExpressionLoaderParams } from './types'; -import { getRenderersRegistry } from './services'; import { renderErrorHandler as defaultRenderErrorHandler } from './render_error_handler'; import { IInterpreterRenderHandlers, ExpressionAstExpression } from '../common'; +import { getRenderersRegistry } from './services'; + export type IExpressionRendererExtraHandlers = Record; export interface ExpressionRenderHandlerParams { diff --git a/src/plugins/expressions/public/services.ts b/src/plugins/expressions/public/services.ts index a203e87414571..016456c956666 100644 --- a/src/plugins/expressions/public/services.ts +++ b/src/plugins/expressions/public/services.ts @@ -20,14 +20,11 @@ import { NotificationsStart } from 'kibana/public'; import { createKibanaUtilsCore, createGetterSetter } from '../../kibana_utils/public'; import { ExpressionInterpreter } from './types'; -import { Start as IInspector } from '../../inspector/public'; import { ExpressionsSetup } from './plugin'; import { ExpressionsService } from '../common'; export const { getCoreStart, setCoreStart } = createKibanaUtilsCore(); -export const [getInspector, setInspector] = createGetterSetter('Inspector'); - export const [getInterpreter, setInterpreter] = createGetterSetter( 'Interpreter' ); diff --git a/src/plugins/kibana_utils/public/core/create_start_service_getter.ts b/src/plugins/kibana_utils/public/core/create_start_service_getter.ts index e507d1ae778e5..5e385eb5ed473 100644 --- a/src/plugins/kibana_utils/public/core/create_start_service_getter.ts +++ b/src/plugins/kibana_utils/public/core/create_start_service_getter.ts @@ -30,6 +30,48 @@ export type StartServicesGetter = () = OwnContract >; +/** + * Use this utility to create a synchronous *start* service getter in *setup* + * life-cycle of your plugin. + * + * Below is a usage example in a Kibana plugin. + * + * ```ts + * export interface MyPluginStartDeps { + * data: DataPublicPluginStart; + * expressions: ExpressionsStart; + * inspector: InspectorStart; + * uiActions: UiActionsStart; + * } + * + * class MyPlugin implements Plugin { + * setup(core: CoreSetup, plugins) { + * const start = createStartServicesGetter(core.getStartServices); + * plugins.expressions.registerFunction(myExpressionFunction(start)); + * } + * + * start(core, plugins: MyPluginStartDeps) { + * + * } + * } + * ``` + * + * In `myExpressionFunction` you can make sure you are picking only the dependencies + * your function needs using the `Pick` type. + * + * ```ts + * const myExpressionFunction = + * (start: StartServicesGetter>) => { + * + * start().plugins.indexPatterns.something(123); + * } + * ``` + * + * @param accessor Asynchronous start service accessor provided by platform. + * @returns Returns a function which synchronously returns *start* core services + * and plugin contracts. If you call this function before the *start* life-cycle + * has started it will throw. + */ export const createStartServicesGetter = ( accessor: StartServicesAccessor ): StartServicesGetter => { diff --git a/src/plugins/visualizations/kibana.json b/src/plugins/visualizations/kibana.json index cd22b1375ae1b..f3f9cbd8341ec 100644 --- a/src/plugins/visualizations/kibana.json +++ b/src/plugins/visualizations/kibana.json @@ -3,5 +3,5 @@ "version": "kibana", "server": true, "ui": true, - "requiredPlugins": ["data", "expressions", "uiActions", "embeddable", "usageCollection"] + "requiredPlugins": ["data", "expressions", "uiActions", "embeddable", "usageCollection", "inspector"] } diff --git a/src/plugins/visualizations/public/embeddable/create_vis_embeddable_from_object.ts b/src/plugins/visualizations/public/embeddable/create_vis_embeddable_from_object.ts index bf2d174f594b2..8e51bd4ac5d4f 100644 --- a/src/plugins/visualizations/public/embeddable/create_vis_embeddable_from_object.ts +++ b/src/plugins/visualizations/public/embeddable/create_vis_embeddable_from_object.ts @@ -28,8 +28,9 @@ import { getTimeFilter, getCapabilities, } from '../services'; +import { VisualizeEmbeddableFactoryDeps } from './visualize_embeddable_factory'; -export const createVisEmbeddableFromObject = async ( +export const createVisEmbeddableFromObject = (deps: VisualizeEmbeddableFactoryDeps) => async ( vis: Vis, input: Partial & { id: string }, parent?: IContainer @@ -58,6 +59,7 @@ export const createVisEmbeddableFromObject = async ( indexPatterns, editUrl, editable, + deps, }, input, parent diff --git a/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts b/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts index e64d200251797..ffb028ff131b3 100644 --- a/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts +++ b/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts @@ -42,6 +42,7 @@ import { buildPipeline } from '../legacy/build_pipeline'; import { Vis } from '../vis'; import { getExpressions, getUiActions } from '../services'; import { VIS_EVENT_TO_TRIGGER } from './events'; +import { VisualizeEmbeddableFactoryDeps } from './visualize_embeddable_factory'; const getKeys = (o: T): Array => Object.keys(o) as Array; @@ -50,6 +51,7 @@ export interface VisualizeEmbeddableConfiguration { indexPatterns?: IIndexPattern[]; editUrl: string; editable: boolean; + deps: VisualizeEmbeddableFactoryDeps; } export interface VisualizeInput extends EmbeddableInput { @@ -84,10 +86,11 @@ export class VisualizeEmbeddable extends Embeddable { - if (this.handler) { - return this.handler.openInspector(this.getTitle() || ''); - } + if (!this.handler) return; + + const adapters = this.handler.inspect(); + if (!adapters) return; + + this.deps.start().plugins.inspector.open(adapters, { + title: this.getTitle() || '', + }); }; /** diff --git a/src/plugins/visualizations/public/embeddable/visualize_embeddable_factory.tsx b/src/plugins/visualizations/public/embeddable/visualize_embeddable_factory.tsx index 4b7d01ae3b246..6ab1c98645988 100644 --- a/src/plugins/visualizations/public/embeddable/visualize_embeddable_factory.tsx +++ b/src/plugins/visualizations/public/embeddable/visualize_embeddable_factory.tsx @@ -25,7 +25,7 @@ import { EmbeddableOutput, ErrorEmbeddable, IContainer, -} from '../../../../plugins/embeddable/public'; +} from '../../../embeddable/public'; import { DisabledLabEmbeddable } from './disabled_lab_embeddable'; import { VisualizeEmbeddable, VisualizeInput, VisualizeOutput } from './visualize_embeddable'; import { VISUALIZE_EMBEDDABLE_TYPE } from './constants'; @@ -39,11 +39,17 @@ import { import { showNewVisModal } from '../wizard'; import { convertToSerializedVis } from '../saved_visualizations/_saved_vis'; import { createVisEmbeddableFromObject } from './create_vis_embeddable_from_object'; +import { StartServicesGetter } from '../../../kibana_utils/public'; +import { VisualizationsStartDeps } from '../plugin'; interface VisualizationAttributes extends SavedObjectAttributes { visState: string; } +export interface VisualizeEmbeddableFactoryDeps { + start: StartServicesGetter>; +} + export class VisualizeEmbeddableFactory implements EmbeddableFactoryDefinition< @@ -79,7 +85,8 @@ export class VisualizeEmbeddableFactory return visType.stage !== 'experimental'; }, }; - constructor() {} + + constructor(private readonly deps: VisualizeEmbeddableFactoryDeps) {} public async isEditable() { return getCapabilities().visualize.save as boolean; @@ -101,7 +108,7 @@ export class VisualizeEmbeddableFactory try { const savedObject = await savedVisualizations.get(savedObjectId); const vis = new Vis(savedObject.visState.type, await convertToSerializedVis(savedObject)); - return createVisEmbeddableFromObject(vis, input, parent); + return createVisEmbeddableFromObject(this.deps)(vis, input, parent); } catch (e) { console.error(e); // eslint-disable-line no-console return new ErrorEmbeddable(e, input, parent); diff --git a/src/plugins/visualizations/public/mocks.ts b/src/plugins/visualizations/public/mocks.ts index 2aa346423297a..d6eeffdb01459 100644 --- a/src/plugins/visualizations/public/mocks.ts +++ b/src/plugins/visualizations/public/mocks.ts @@ -26,6 +26,7 @@ import { expressionsPluginMock } from '../../../plugins/expressions/public/mocks import { dataPluginMock } from '../../../plugins/data/public/mocks'; import { usageCollectionPluginMock } from '../../../plugins/usage_collection/public/mocks'; import { uiActionsPluginMock } from '../../../plugins/ui_actions/public/mocks'; +import { inspectorPluginMock } from '../../../plugins/inspector/public/mocks'; const createSetupContract = (): VisualizationsSetup => ({ createBaseVisualization: jest.fn(), @@ -53,14 +54,16 @@ const createInstance = async () => { const setup = plugin.setup(coreMock.createSetup(), { data: dataPluginMock.createSetupContract(), - expressions: expressionsPluginMock.createSetupContract(), embeddable: embeddablePluginMock.createSetupContract(), + expressions: expressionsPluginMock.createSetupContract(), + inspector: inspectorPluginMock.createSetupContract(), usageCollection: usageCollectionPluginMock.createSetupContract(), }); const doStart = () => plugin.start(coreMock.createStart(), { data: dataPluginMock.createStartContract(), expressions: expressionsPluginMock.createStartContract(), + inspector: inspectorPluginMock.createStartContract(), uiActions: uiActionsPluginMock.createStartContract(), }); diff --git a/src/plugins/visualizations/public/plugin.ts b/src/plugins/visualizations/public/plugin.ts index 8fcb84b19a9be..b3e8c9b5b61b3 100644 --- a/src/plugins/visualizations/public/plugin.ts +++ b/src/plugins/visualizations/public/plugin.ts @@ -43,18 +43,23 @@ import { VisualizeEmbeddableFactory, createVisEmbeddableFromObject, } from './embeddable'; -import { ExpressionsSetup, ExpressionsStart } from '../../../plugins/expressions/public'; -import { EmbeddableSetup } from '../../../plugins/embeddable/public'; +import { ExpressionsSetup, ExpressionsStart } from '../../expressions/public'; +import { EmbeddableSetup } from '../../embeddable/public'; import { visualization as visualizationFunction } from './expressions/visualization_function'; import { visualization as visualizationRenderer } from './expressions/visualization_renderer'; import { range as rangeExpressionFunction } from './expression_functions/range'; import { visDimension as visDimensionExpressionFunction } from './expression_functions/vis_dimension'; import { DataPublicPluginSetup, DataPublicPluginStart } from '../../../plugins/data/public'; -import { UsageCollectionSetup } from '../../../plugins/usage_collection/public'; +import { + Setup as InspectorSetup, + Start as InspectorStart, +} from '../../../plugins/inspector/public'; +import { UsageCollectionSetup } from '../../usage_collection/public'; +import { createStartServicesGetter, StartServicesGetter } from '../../kibana_utils/public'; import { createSavedVisLoader, SavedVisualizationsLoader } from './saved_visualizations'; import { SerializedVis, Vis } from './vis'; import { showNewVisModal } from './wizard'; -import { UiActionsStart } from '../../../plugins/ui_actions/public'; +import { UiActionsStart } from '../../ui_actions/public'; import { convertFromSerializedVis, convertToSerializedVis, @@ -74,19 +79,21 @@ export interface VisualizationsStart extends TypesStart { convertToSerializedVis: typeof convertToSerializedVis; convertFromSerializedVis: typeof convertFromSerializedVis; showNewVisModal: typeof showNewVisModal; - __LEGACY: { createVisEmbeddableFromObject: typeof createVisEmbeddableFromObject }; + __LEGACY: { createVisEmbeddableFromObject: ReturnType }; } export interface VisualizationsSetupDeps { - expressions: ExpressionsSetup; + data: DataPublicPluginSetup; embeddable: EmbeddableSetup; + expressions: ExpressionsSetup; + inspector: InspectorSetup; usageCollection: UsageCollectionSetup; - data: DataPublicPluginSetup; } export interface VisualizationsStartDeps { data: DataPublicPluginStart; expressions: ExpressionsStart; + inspector: InspectorStart; uiActions: UiActionsStart; } @@ -107,13 +114,16 @@ export class VisualizationsPlugin VisualizationsStartDeps > { private readonly types: TypesService = new TypesService(); + private getStartServicesOrDie?: StartServicesGetter; constructor(initializerContext: PluginInitializerContext) {} public setup( - core: CoreSetup, + core: CoreSetup, { expressions, embeddable, usageCollection, data }: VisualizationsSetupDeps ): VisualizationsSetup { + const start = (this.getStartServicesOrDie = createStartServicesGetter(core.getStartServices)); + setUISettings(core.uiSettings); setUsageCollector(usageCollection); @@ -122,7 +132,7 @@ export class VisualizationsPlugin expressions.registerFunction(rangeExpressionFunction); expressions.registerFunction(visDimensionExpressionFunction); - const embeddableFactory = new VisualizeEmbeddableFactory(); + const embeddableFactory = new VisualizeEmbeddableFactory({ start }); embeddable.registerEmbeddableFactory(VISUALIZE_EMBEDDABLE_TYPE, embeddableFactory); return { @@ -171,7 +181,11 @@ export class VisualizationsPlugin convertToSerializedVis, convertFromSerializedVis, savedVisualizationsLoader, - __LEGACY: { createVisEmbeddableFromObject }, + __LEGACY: { + createVisEmbeddableFromObject: createVisEmbeddableFromObject({ + start: this.getStartServicesOrDie!, + }), + }, }; }