Lazy load metric & mardown visualizations (#78391)

* Lazy load metrics vis * Use common chart spinner * Simplify markdown renderer * Update tests * Update types for metric vis * Fix tests * Fix merge conflict Co-authored-by: Elastic Machine <[email protected]>
elastic · Sep 28, 2020 · 3f6c0d6 · 3f6c0d6
1 parent faf4b04
commit 3f6c0d6
Show file tree

Hide file tree

Showing 20 changed files with 132 additions and 199 deletions.
diff --git a/src/plugins/vis_type_markdown/kibana.json b/src/plugins/vis_type_markdown/kibana.json
@@ -4,5 +4,5 @@
     "ui": true,
     "server": true,
     "requiredPlugins": ["expressions", "visualizations"],
-    "requiredBundles": ["kibanaUtils", "kibanaReact", "charts", "visualizations", "expressions", "visDefaultEditor"]
+    "requiredBundles": ["kibanaReact", "charts", "visualizations", "expressions", "visDefaultEditor"]
 }
diff --git a/src/plugins/vis_type_markdown/public/__snapshots__/markdown_fn.test.ts.snap b/src/plugins/vis_type_markdown/public/__snapshots__/markdown_fn.test.ts.snap
diff --git a/src/plugins/vis_type_markdown/public/index.scss b/src/plugins/vis_type_markdown/public/index.scss
diff --git a/src/plugins/vis_type_markdown/public/markdown_fn.ts b/src/plugins/vis_type_markdown/public/markdown_fn.ts
@@ -21,16 +21,16 @@ import { i18n } from '@kbn/i18n';
 import { ExpressionFunctionDefinition, Render } from '../../expressions/public';
 import { Arguments, MarkdownVisParams } from './types';
 
-interface RenderValue {
+export interface MarkdownVisRenderValue {
   visType: 'markdown';
-  visConfig: MarkdownVisParams;
+  visParams: MarkdownVisParams;
 }
 
 export type MarkdownVisExpressionFunctionDefinition = ExpressionFunctionDefinition<
   'markdownVis',
   unknown,
   Arguments,
-  Render<RenderValue>
+  Render<MarkdownVisRenderValue>
 >;
 
 export const createMarkdownVisFn = (): MarkdownVisExpressionFunctionDefinition => ({
@@ -70,7 +70,7 @@ export const createMarkdownVisFn = (): MarkdownVisExpressionFunctionDefinition =
       as: 'markdown_vis',
       value: {
         visType: 'markdown',
-        visConfig: {
+        visParams: {
           markdown: args.markdown,
           openLinksInNewTab: args.openLinksInNewTab,
           fontSize: parseInt(args.font.spec.fontSize || '12', 10),

diff --git a/src/plugins/vis_type_markdown/public/markdown_renderer.tsx b/src/plugins/vis_type_markdown/public/markdown_renderer.tsx
@@ -17,41 +17,29 @@
  * under the License.
  */
 
-import React from 'react';
+import React, { lazy } from 'react';
 import { render, unmountComponentAtNode } from 'react-dom';
 import { VisualizationContainer } from '../../visualizations/public';
 import { ExpressionRenderDefinition } from '../../expressions/common/expression_renderers';
-import { MarkdownVisWrapper } from './markdown_vis_controller';
-import { StartServicesGetter } from '../../kibana_utils/public';
+import { MarkdownVisRenderValue } from './markdown_fn';
 
-export const getMarkdownRenderer = (start: StartServicesGetter) => {
-  const markdownVisRenderer: () => ExpressionRenderDefinition = () => ({
-    name: 'markdown_vis',
-    displayName: 'markdown visualization',
-    reuseDomNode: true,
-    render: async (domNode: HTMLElement, config: any, handlers: any) => {
-      const { visConfig } = config;
+// @ts-ignore
+const MarkdownVisComponent = lazy(() => import('./markdown_vis_controller'));
 
-      const I18nContext = await start().core.i18n.Context;
+export const markdownVisRenderer: ExpressionRenderDefinition<MarkdownVisRenderValue> = {
+  name: 'markdown_vis',
+  displayName: 'markdown visualization',
+  reuseDomNode: true,
+  render: async (domNode, { visParams }, handlers) => {
+    handlers.onDestroy(() => {
+      unmountComponentAtNode(domNode);
+    });
 
-      handlers.onDestroy(() => {
-        unmountComponentAtNode(domNode);
-      });
-
-      render(
-        <I18nContext>
-          <VisualizationContainer className="markdownVis">
-            <MarkdownVisWrapper
-              visParams={visConfig}
-              renderComplete={handlers.done}
-              fireEvent={handlers.event}
-            />
-          </VisualizationContainer>
-        </I18nContext>,
-        domNode
-      );
-    },
-  });
-
-  return markdownVisRenderer;
+    render(
+      <VisualizationContainer className="markdownVis">
+        <MarkdownVisComponent {...visParams} renderComplete={handlers.done} />
+      </VisualizationContainer>,
+      domNode
+    );
+  },
 };
diff --git a/...s_type_markdown/public/_markdown_vis.scss → ...is_type_markdown/public/markdown_vis.scss b/...s_type_markdown/public/_markdown_vis.scss → ...is_type_markdown/public/markdown_vis.scss
@@ -1,3 +1,10 @@
+// Prefix all styles with "mkd" to avoid conflicts.
+// Examples
+//   mkdChart
+//   mkdChart__legend
+//   mkdChart__legend--small
+//   mkdChart__legend-isLoading
+
 .mkdVis {
   padding: $euiSizeS;
   width: 100%;

diff --git a/src/plugins/vis_type_markdown/public/markdown_vis_controller.test.tsx b/src/plugins/vis_type_markdown/public/markdown_vis_controller.test.tsx
@@ -20,7 +20,7 @@
 import React from 'react';
 import { wait } from '@testing-library/dom';
 import { render, cleanup } from '@testing-library/react/pure';
-import { MarkdownVisWrapper } from './markdown_vis_controller';
+import MarkdownVisComponent from './markdown_vis_controller';
 
 afterEach(cleanup);
 
@@ -36,7 +36,7 @@ describe('markdown vis controller', () => {
     };
 
     const { getByTestId, getByText } = render(
-      <MarkdownVisWrapper visParams={vis.params} renderComplete={jest.fn()} fireEvent={jest.fn()} />
+      <MarkdownVisComponent {...vis.params} renderComplete={jest.fn()} />
     );
 
     await wait(() => getByTestId('markdownBody'));
@@ -60,7 +60,7 @@ describe('markdown vis controller', () => {
     };
 
     const { getByTestId, getByText } = render(
-      <MarkdownVisWrapper visParams={vis.params} renderComplete={jest.fn()} fireEvent={jest.fn()} />
+      <MarkdownVisComponent {...vis.params} renderComplete={jest.fn()} />
     );
 
     await wait(() => getByTestId('markdownBody'));
@@ -82,17 +82,15 @@ describe('markdown vis controller', () => {
     };
 
     const { getByTestId, getByText, rerender } = render(
-      <MarkdownVisWrapper visParams={vis.params} renderComplete={jest.fn()} fireEvent={jest.fn()} />
+      <MarkdownVisComponent {...vis.params} renderComplete={jest.fn()} />
     );
 
     await wait(() => getByTestId('markdownBody'));
 
     expect(getByText(/initial/i)).toBeInTheDocument();
 
     vis.params.markdown = 'Updated';
-    rerender(
-      <MarkdownVisWrapper visParams={vis.params} renderComplete={jest.fn()} fireEvent={jest.fn()} />
-    );
+    rerender(<MarkdownVisComponent {...vis.params} renderComplete={jest.fn()} />);
 
     expect(getByText(/Updated/i)).toBeInTheDocument();
   });
@@ -114,11 +112,7 @@ describe('markdown vis controller', () => {
 
     it('should be called on initial rendering', async () => {
       const { getByTestId } = render(
-        <MarkdownVisWrapper
-          visParams={vis.params}
-          renderComplete={renderComplete}
-          fireEvent={jest.fn()}
-        />
+        <MarkdownVisComponent {...vis.params} renderComplete={renderComplete} />
       );
 
       await wait(() => getByTestId('markdownBody'));
@@ -128,11 +122,7 @@ describe('markdown vis controller', () => {
 
     it('should be called on successive render when params change', async () => {
       const { getByTestId, rerender } = render(
-        <MarkdownVisWrapper
-          visParams={vis.params}
-          renderComplete={renderComplete}
-          fireEvent={jest.fn()}
-        />
+        <MarkdownVisComponent {...vis.params} renderComplete={renderComplete} />
       );
 
       await wait(() => getByTestId('markdownBody'));
@@ -142,24 +132,14 @@ describe('markdown vis controller', () => {
       renderComplete.mockClear();
       vis.params.markdown = 'changed';
 
-      rerender(
-        <MarkdownVisWrapper
-          visParams={vis.params}
-          renderComplete={renderComplete}
-          fireEvent={jest.fn()}
-        />
-      );
+      rerender(<MarkdownVisComponent {...vis.params} renderComplete={renderComplete} />);
 
       expect(renderComplete).toHaveBeenCalledTimes(1);
     });
 
     it('should be called on successive render even without data change', async () => {
       const { getByTestId, rerender } = render(
-        <MarkdownVisWrapper
-          visParams={vis.params}
-          renderComplete={renderComplete}
-          fireEvent={jest.fn()}
-        />
+        <MarkdownVisComponent {...vis.params} renderComplete={renderComplete} />
       );
 
       await wait(() => getByTestId('markdownBody'));
@@ -168,13 +148,7 @@ describe('markdown vis controller', () => {
 
       renderComplete.mockClear();
 
-      rerender(
-        <MarkdownVisWrapper
-          visParams={vis.params}
-          renderComplete={renderComplete}
-          fireEvent={jest.fn()}
-        />
-      );
+      rerender(<MarkdownVisComponent {...vis.params} renderComplete={renderComplete} />);
 
       expect(renderComplete).toHaveBeenCalledTimes(1);
     });

diff --git a/src/plugins/vis_type_markdown/public/markdown_vis_controller.tsx b/src/plugins/vis_type_markdown/public/markdown_vis_controller.tsx
@@ -17,83 +17,35 @@
  * under the License.
  */
 
-import React from 'react';
+import React, { useEffect } from 'react';
 import { Markdown } from '../../kibana_react/public';
 import { MarkdownVisParams } from './types';
 
+import './markdown_vis.scss';
+
 interface MarkdownVisComponentProps extends MarkdownVisParams {
   renderComplete: () => void;
 }
 
-/**
- * The MarkdownVisComponent renders markdown to HTML and presents it.
- */
-class MarkdownVisComponent extends React.Component<MarkdownVisComponentProps> {
-  /**
-   * Will be called after the first render when the component is present in the DOM.
-   *
-   * We call renderComplete here, to signal, that we are done with rendering.
-   */
-  componentDidMount() {
-    this.props.renderComplete();
-  }
-
-  /**
-   * Will be called after the component has been updated and the changes has been
-   * flushed into the DOM.
-   *
-   * We will use this to signal that we are done rendering by calling the
-   * renderComplete property.
-   */
-  componentDidUpdate() {
-    this.props.renderComplete();
-  }
+const MarkdownVisComponent = ({
+  fontSize,
+  markdown,
+  openLinksInNewTab,
+  renderComplete,
+}: MarkdownVisComponentProps) => {
+  useEffect(renderComplete); // renderComplete will be called after each render to signal, that we are done with rendering.
 
-  /**
-   * Render the actual HTML.
-   * Note: if only fontSize parameter has changed, this method will be called
-   * and return the appropriate JSX, but React will detect, that only the
-   * style argument has been updated, and thus only set this attribute to the DOM.
-   */
-  render() {
-    return (
-      <div className="mkdVis" style={{ fontSize: `${this.props.fontSize}pt` }}>
-        <Markdown
-          data-test-subj="markdownBody"
-          markdown={this.props.markdown}
-          openLinksInNewTab={this.props.openLinksInNewTab}
-        />
-      </div>
-    );
-  }
-}
-
-/**
- * This is a wrapper component, that is actually used as the visualization.
- * The sole purpose of this component is to extract all required parameters from
- * the properties and pass them down as separate properties to the actual component.
- * That way the actual (MarkdownVisComponent) will properly trigger it's prop update
- * callback (componentWillReceiveProps) if one of these params change. It wouldn't
- * trigger otherwise (e.g. it doesn't for this wrapper), since it only triggers
- * if the reference to the prop changes (in this case the reference to vis).
- *
- * The way React works, this wrapper nearly brings no overhead, but allows us
- * to use proper lifecycle methods in the actual component.
- */
-
-export interface MarkdownVisWrapperProps {
-  visParams: MarkdownVisParams;
-  fireEvent: (event: any) => void;
-  renderComplete: () => void;
-}
-
-export function MarkdownVisWrapper(props: MarkdownVisWrapperProps) {
   return (
-    <MarkdownVisComponent
-      fontSize={props.visParams.fontSize}
-      markdown={props.visParams.markdown}
-      openLinksInNewTab={props.visParams.openLinksInNewTab}
-      renderComplete={props.renderComplete}
-    />
+    <div className="mkdVis" style={{ fontSize: `${fontSize}pt` }}>
+      <Markdown
+        data-test-subj="markdownBody"
+        markdown={markdown}
+        openLinksInNewTab={openLinksInNewTab}
+      />
+    </div>
   );
-}
+};
+
+// default export required for React.Lazy
+// eslint-disable-next-line import/no-default-export
+export { MarkdownVisComponent as default };
diff --git a/src/plugins/vis_type_markdown/public/plugin.ts b/src/plugins/vis_type_markdown/public/plugin.ts
@@ -24,10 +24,7 @@ import { VisualizationsSetup } from '../../visualizations/public';
 import { markdownVisDefinition } from './markdown_vis';
 import { createMarkdownVisFn } from './markdown_fn';
 import { ConfigSchema } from '../config';
-
-import './index.scss';
-import { getMarkdownRenderer } from './markdown_renderer';
-import { createStartServicesGetter } from '../../kibana_utils/public';
+import { markdownVisRenderer } from './markdown_renderer';
 
 /** @internal */
 export interface MarkdownPluginSetupDependencies {
@@ -44,9 +41,8 @@ export class MarkdownPlugin implements Plugin<void, void> {
   }
 
   public setup(core: CoreSetup, { expressions, visualizations }: MarkdownPluginSetupDependencies) {
-    const start = createStartServicesGetter(core.getStartServices);
     visualizations.createBaseVisualization(markdownVisDefinition);
-    expressions.registerRenderer(getMarkdownRenderer(start));
+    expressions.registerRenderer(markdownVisRenderer);
     expressions.registerFunction(createMarkdownVisFn);
   }
 

diff --git a/...s/vis_type_metric/public/_metric_vis.scss → ..._metric/public/components/metric_vis.scss b/...s/vis_type_metric/public/_metric_vis.scss → ..._metric/public/components/metric_vis.scss
@@ -1,3 +1,10 @@
+// Prefix all styles with "mtr" to avoid conflicts.
+// Examples
+//   mtrChart
+//   mtrChart__legend
+//   mtrChart__legend--small
+//   mtrChart__legend-isLoading
+
 .mtrVis {
   width: 100%;
   display: flex;

diff --git a/src/plugins/vis_type_metric/public/components/metric_vis_component.test.tsx b/src/plugins/vis_type_metric/public/components/metric_vis_component.test.tsx
@@ -20,7 +20,7 @@
 import React from 'react';
 import { shallow } from 'enzyme';
 
-import { MetricVisComponent, MetricVisComponentProps } from './metric_vis_component';
+import MetricVisComponent, { MetricVisComponentProps } from './metric_vis_component';
 
 jest.mock('../services', () => ({
   getFormatService: () => ({