Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Discuss] Remove expressions plugin's dependency on inspector plugin #63841

Merged
merged 7 commits into from
Apr 20, 2020
3 changes: 1 addition & 2 deletions src/plugins/expressions/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
"server": true,
"ui": true,
"requiredPlugins": [
"bfetch",
"inspector"
"bfetch"
]
}
16 changes: 4 additions & 12 deletions src/plugins/expressions/public/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand Down
3 changes: 0 additions & 3 deletions src/plugins/expressions/public/mocks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand Down Expand Up @@ -89,7 +88,6 @@ const createPlugin = async () => {
const plugin = pluginInitializer(pluginInitializerContext);
const setup = await plugin.setup(coreSetup, {
bfetch: bfetchPluginMock.createSetupContract(),
inspector: inspectorPluginMock.createSetupContract(),
});

return {
Expand All @@ -101,7 +99,6 @@ const createPlugin = async () => {
doStart: async () =>
await plugin.start(coreStart, {
bfetch: bfetchPluginMock.createStartContract(),
inspector: inspectorPluginMock.createStartContract(),
}),
};
};
Expand Down
9 changes: 2 additions & 7 deletions src/plugins/expressions/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions src/plugins/expressions/public/react_expression_renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/expressions/public/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, any>;

export interface ExpressionRenderHandlerParams {
Expand Down
3 changes: 0 additions & 3 deletions src/plugins/expressions/public/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<IInspector>('Inspector');

export const [getInterpreter, setInterpreter] = createGetterSetter<ExpressionInterpreter>(
'Interpreter'
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,48 @@ export type StartServicesGetter<Plugins = unknown, OwnContract = unknown> = () =
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<MyPluginStartDeps>, 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<Pick<MyPluginStartDeps, 'data'>>) => {
*
* 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 = <TPluginsStart extends object, TStart>(
accessor: StartServicesAccessor<TPluginsStart, TStart>
): StartServicesGetter<TPluginsStart, TStart> => {
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/visualizations/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
"version": "kibana",
"server": true,
"ui": true,
"requiredPlugins": ["data", "expressions", "uiActions", "embeddable", "usageCollection"]
"requiredPlugins": ["data", "expressions", "uiActions", "embeddable", "usageCollection", "inspector"]
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<VisualizeInput> & { id: string },
parent?: IContainer
Expand Down Expand Up @@ -58,6 +59,7 @@ export const createVisEmbeddableFromObject = async (
indexPatterns,
editUrl,
editable,
deps,
},
input,
parent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <T extends {}>(o: T): Array<keyof T> => Object.keys(o) as Array<keyof T>;

Expand All @@ -50,6 +51,7 @@ export interface VisualizeEmbeddableConfiguration {
indexPatterns?: IIndexPattern[];
editUrl: string;
editable: boolean;
deps: VisualizeEmbeddableFactoryDeps;
}

export interface VisualizeInput extends EmbeddableInput {
Expand Down Expand Up @@ -84,10 +86,11 @@ export class VisualizeEmbeddable extends Embeddable<VisualizeInput, VisualizeOut
public readonly type = VISUALIZE_EMBEDDABLE_TYPE;
private autoRefreshFetchSubscription: Subscription;
private abortController?: AbortController;
private readonly deps: VisualizeEmbeddableFactoryDeps;

constructor(
timefilter: TimefilterContract,
{ vis, editUrl, indexPatterns, editable }: VisualizeEmbeddableConfiguration,
{ vis, editUrl, indexPatterns, editable, deps }: VisualizeEmbeddableConfiguration,
initialInput: VisualizeInput,
parent?: IContainer
) {
Expand All @@ -102,6 +105,7 @@ export class VisualizeEmbeddable extends Embeddable<VisualizeInput, VisualizeOut
},
parent
);
this.deps = deps;
this.timefilter = timefilter;
this.vis = vis;
this.vis.uiState.on('change', this.uiStateChangeHandler);
Expand All @@ -128,9 +132,14 @@ export class VisualizeEmbeddable extends Embeddable<VisualizeInput, VisualizeOut
};

public openInspector = () => {
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() || '',
});
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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<Pick<VisualizationsStartDeps, 'inspector'>>;
}

export class VisualizeEmbeddableFactory
implements
EmbeddableFactoryDefinition<
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion src/plugins/visualizations/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
});

Expand Down
Loading