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

…63841)

* 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

@@ -4,7 +4,6 @@
   "server": true,
   "ui": true,
   "requiredPlugins": [
-    "bfetch",
-    "inspector"
+    "bfetch"
   ]
 }

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

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(),
       }),
   };
 };

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;

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

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<string, any>;
 
 export interface ExpressionRenderHandlerParams {

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<IInspector>('Inspector');
-
 export const [getInterpreter, setInterpreter] = createGetterSetter<ExpressionInterpreter>(
   'Interpreter'
 );

src/plugins/kibana_utils/public/core/create_start_service_getter.ts

@@ -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> => {

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"]
 }

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<VisualizeInput> & { id: string },
   parent?: IContainer
@@ -58,6 +59,7 @@ export const createVisEmbeddableFromObject = async (
         indexPatterns,
         editUrl,
         editable,
+        deps,
       },
       input,
       parent

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 = <T extends {}>(o: T): Array<keyof T> => Object.keys(o) as Array<keyof T>;
 
@@ -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<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
   ) {
@@ -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);
@@ -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() || '',
+    });
   };
 
   /**

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<Pick<VisualizationsStartDeps, 'inspector'>>;
+}
+
 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);

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(),
     });