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
2 changes: 0 additions & 2 deletions src/plugins/expressions/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import { Setup as InspectorSetup, Start as InspectorStart } from '../../inspecto
import { BfetchPublicSetup, BfetchPublicStart } from '../../bfetch/public';
import {
setCoreStart,
setInspector,
setInterpreter,
setRenderersRegistry,
setNotifications,
Expand Down Expand Up @@ -182,7 +181,6 @@ export class ExpressionsPublicPlugin

public start(core: CoreStart, { inspector, bfetch }: ExpressionsStartDeps): ExpressionsStart {
streamich marked this conversation as resolved.
Show resolved Hide resolved
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
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
34 changes: 24 additions & 10 deletions src/plugins/visualizations/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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<typeof createVisEmbeddableFromObject> };
}

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;
}

Expand All @@ -107,13 +114,16 @@ export class VisualizationsPlugin
VisualizationsStartDeps
> {
private readonly types: TypesService = new TypesService();
private getStartServicesOrDie?: StartServicesGetter<VisualizationsStartDeps, VisualizationsStart>;

constructor(initializerContext: PluginInitializerContext) {}

public setup(
core: CoreSetup,
core: CoreSetup<VisualizationsStartDeps, VisualizationsStart>,
{ expressions, embeddable, usageCollection, data }: VisualizationsSetupDeps
): VisualizationsSetup {
const start = (this.getStartServicesOrDie = createStartServicesGetter(core.getStartServices));
streamich marked this conversation as resolved.
Show resolved Hide resolved

setUISettings(core.uiSettings);
setUsageCollector(usageCollection);

Expand All @@ -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 {
Expand Down Expand Up @@ -171,7 +181,11 @@ export class VisualizationsPlugin
convertToSerializedVis,
convertFromSerializedVis,
savedVisualizationsLoader,
__LEGACY: { createVisEmbeddableFromObject },
__LEGACY: {
createVisEmbeddableFromObject: createVisEmbeddableFromObject({
start: this.getStartServicesOrDie!,
}),
},
};
}

Expand Down