From 6d6f304d68bbab0859b54d58dfa846a9e85967e8 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 7 May 2019 11:57:37 +0200 Subject: [PATCH 01/22] add nativerenderer component --- x-pack/plugins/lens/public/app_plugin/app.tsx | 9 +- .../utils/native_renderer/__mocks__/react.ts | 12 +++ .../native_renderer/native_renderer.test.tsx | 100 ++++++++++++++++++ .../utils/native_renderer/native_renderer.tsx | 65 ++++++++++++ 4 files changed, 179 insertions(+), 7 deletions(-) create mode 100644 x-pack/plugins/lens/public/utils/native_renderer/__mocks__/react.ts create mode 100644 x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx create mode 100644 x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 3e16a780830925..819109ea6d2767 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -8,20 +8,15 @@ import { I18nProvider } from '@kbn/i18n/react'; import React, { useCallback } from 'react'; import { EditorFrameSetup } from '../types'; +import { NativeRenderer } from '../utils/native_renderer/native_renderer'; export function App({ editorFrame }: { editorFrame: EditorFrameSetup }) { - const renderFrame = useCallback(node => { - if (node !== null) { - editorFrame.render(node); - } - }, []); - return (

Lens

-
+
); diff --git a/x-pack/plugins/lens/public/utils/native_renderer/__mocks__/react.ts b/x-pack/plugins/lens/public/utils/native_renderer/__mocks__/react.ts new file mode 100644 index 00000000000000..b09289b0516ce2 --- /dev/null +++ b/x-pack/plugins/lens/public/utils/native_renderer/__mocks__/react.ts @@ -0,0 +1,12 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; + +// This mock overwrites react to use the layout effect hook instead of the normal one. +// This is done to have effects executed synchronously to be able to test them without +// setTimeout hacks. +export default { ...React, useEffect: React.useLayoutEffect }; diff --git a/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx b/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx new file mode 100644 index 00000000000000..1340a477d570ce --- /dev/null +++ b/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx @@ -0,0 +1,100 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import { render } from 'react-dom'; +import { NativeRenderer } from './native_renderer'; + +describe('agnostic_renderer', () => { + let mountpoint: Element; + + beforeEach(() => { + mountpoint = document.createElement('div'); + }); + + afterEach(() => { + mountpoint.remove(); + }); + + it('should render element in container', () => { + const renderSpy = jest.fn(); + const testProps = { a: 'abc' }; + + render(, mountpoint); + const containerElement = mountpoint.firstElementChild; + expect(renderSpy).toHaveBeenCalledWith(containerElement, testProps); + }); + + it('should not render again if props do not change', () => { + const renderSpy = jest.fn(); + const testProps = { a: 'abc' }; + + render(, mountpoint); + render(, mountpoint); + expect(renderSpy).toHaveBeenCalledTimes(1); + }); + + it('should not render again if props do not change shallowly', () => { + const renderSpy = jest.fn(); + const testProps = { a: 'abc' }; + + render(, mountpoint); + render(, mountpoint); + expect(renderSpy).toHaveBeenCalledTimes(1); + }); + + it('should render again once if props change', () => { + const renderSpy = jest.fn(); + const testProps = { a: 'abc' }; + + render(, mountpoint); + render(, mountpoint); + render(, mountpoint); + expect(renderSpy).toHaveBeenCalledTimes(2); + const containerElement = mountpoint.firstElementChild; + expect(renderSpy).lastCalledWith(containerElement, { a: 'def' }); + }); + + it('should render again if props are extended', () => { + const renderSpy = jest.fn(); + const testProps = { a: 'abc' }; + + render(, mountpoint); + render(, mountpoint); + expect(renderSpy).toHaveBeenCalledTimes(2); + const containerElement = mountpoint.firstElementChild; + expect(renderSpy).lastCalledWith(containerElement, { a: 'abc', b: 'def' }); + }); + + it('should render again if props are limited', () => { + const renderSpy = jest.fn(); + const testProps = { a: 'abc', b: 'def' }; + + render(, mountpoint); + render(, mountpoint); + expect(renderSpy).toHaveBeenCalledTimes(2); + const containerElement = mountpoint.firstElementChild; + expect(renderSpy).lastCalledWith(containerElement, { a: 'abc' }); + }); + + it('should render a div as container', () => { + const renderSpy = jest.fn(); + const testProps = { a: 'abc' }; + + render(, mountpoint); + const containerElement: Element = mountpoint.firstElementChild!; + expect(containerElement.nodeName).toBe('DIV'); + }); + + it('should render a specified element as container', () => { + const renderSpy = jest.fn(); + const testProps = { a: 'abc' }; + + render(, mountpoint); + const containerElement: Element = mountpoint.firstElementChild!; + expect(containerElement.nodeName).toBe('SPAN'); + }); +}); diff --git a/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx b/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx new file mode 100644 index 00000000000000..46b0f78b0539a8 --- /dev/null +++ b/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx @@ -0,0 +1,65 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import PropTypes from 'prop-types'; +import React, { useEffect, useRef } from 'react'; + +export interface NativeRendererProps { + render: (domElement: Element, props: T) => void; + actualProps: T; + tag?: string; + children?: never; +} + +function isShallowDifferent(a: T, b: T): boolean { + for (const key in a) { + if (!(key in b) || a[key] !== b[key]) { + return true; + } + } + for (const key in b) { + if (!(key in a) || a[key] !== b[key]) { + return true; + } + } + + return false; +} + +export function NativeRenderer({ render, actualProps, tag }: NativeRendererProps) { + const elementRef = useRef(null); + const propsRef = useRef(null); + + function renderAndUpdate(element: Element) { + elementRef.current = element; + propsRef.current = actualProps; + render(element, actualProps); + } + + useEffect( + () => { + if (elementRef.current && isShallowDifferent(propsRef.current, actualProps)) { + renderAndUpdate(elementRef.current); + } + }, + [actualProps] + ); + + return React.createElement(tag || 'div', { + ref: element => { + if (element && element !== elementRef.current) { + renderAndUpdate(element); + } + }, + }); +} + +NativeRenderer.propTypes = { + render: PropTypes.func.isRequired, + actualProps: PropTypes.object, + + tag: PropTypes.string +}; \ No newline at end of file From 44894dfb69d79a99d4fef5c66865957ca096e891 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 7 May 2019 12:42:27 +0200 Subject: [PATCH 02/22] use native renderer in app and editor frame --- x-pack/plugins/lens/public/app_plugin/app.tsx | 2 +- .../public/editor_frame_plugin/editor_frame.tsx | 14 ++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 819109ea6d2767..d48badab710163 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -5,7 +5,7 @@ */ import { I18nProvider } from '@kbn/i18n/react'; -import React, { useCallback } from 'react'; +import React from 'react'; import { EditorFrameSetup } from '../types'; import { NativeRenderer } from '../utils/native_renderer/native_renderer'; diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx index 091862e635d99b..6dcc14542e21c0 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx @@ -6,6 +6,7 @@ import React from 'react'; import { Datasource, Visualization } from '../types'; +import { NativeRenderer } from '../utils/native_renderer/native_renderer'; interface EditorFrameProps { datasources: { [key: string]: Datasource }; @@ -20,15 +21,12 @@ export function EditorFrame(props: EditorFrameProps) {

Editor Frame

{keys.map(key => ( -
{ - if (domElement) { - props.datasources[key].renderDataPanel(domElement, { - state: {}, - setState: () => {}, - }); - } + render={props.datasources[key].renderDataPanel} + actualProps={{ + state: {}, + setState: () => {}, }} /> ))} From 943fca82e2cc9132199a1d83c7c13c1860c0a537 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 7 May 2019 13:44:40 +0200 Subject: [PATCH 03/22] remove prop types --- .../utils/native_renderer/native_renderer.tsx | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx b/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx index 46b0f78b0539a8..936433e4b2c784 100644 --- a/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx +++ b/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx @@ -4,7 +4,6 @@ * you may not use this file except in compliance with the Elastic License. */ -import PropTypes from 'prop-types'; import React, { useEffect, useRef } from 'react'; export interface NativeRendererProps { @@ -34,9 +33,9 @@ export function NativeRenderer({ render, actualProps, tag }: NativeRendererPr const propsRef = useRef(null); function renderAndUpdate(element: Element) { - elementRef.current = element; - propsRef.current = actualProps; - render(element, actualProps); + elementRef.current = element; + propsRef.current = actualProps; + render(element, actualProps); } useEffect( @@ -56,10 +55,3 @@ export function NativeRenderer({ render, actualProps, tag }: NativeRendererPr }, }); } - -NativeRenderer.propTypes = { - render: PropTypes.func.isRequired, - actualProps: PropTypes.object, - - tag: PropTypes.string -}; \ No newline at end of file From 731e2647996ba5059b2928711a4adedfe58527a6 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 8 May 2019 08:01:38 +0200 Subject: [PATCH 04/22] add unit test and fix linting issues --- x-pack/plugins/lens/public/index.ts | 2 +- .../utils/native_renderer/__mocks__/react.ts | 2 ++ .../native_renderer/native_renderer.test.tsx | 22 +++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/index.ts b/x-pack/plugins/lens/public/index.ts index 874249ff75d85c..f8f074cdb99bfd 100644 --- a/x-pack/plugins/lens/public/index.ts +++ b/x-pack/plugins/lens/public/index.ts @@ -6,8 +6,8 @@ export * from './types'; -import { IScope } from 'angular'; import { render, unmountComponentAtNode } from 'react-dom'; +import { IScope } from 'angular'; import chrome from 'ui/chrome'; import { appSetup, appStop } from './app_plugin'; diff --git a/x-pack/plugins/lens/public/utils/native_renderer/__mocks__/react.ts b/x-pack/plugins/lens/public/utils/native_renderer/__mocks__/react.ts index b09289b0516ce2..6039f0888f596c 100644 --- a/x-pack/plugins/lens/public/utils/native_renderer/__mocks__/react.ts +++ b/x-pack/plugins/lens/public/utils/native_renderer/__mocks__/react.ts @@ -9,4 +9,6 @@ import React from 'react'; // This mock overwrites react to use the layout effect hook instead of the normal one. // This is done to have effects executed synchronously to be able to test them without // setTimeout hacks. + +// eslint-disable-next-line import/no-default-export export default { ...React, useEffect: React.useLayoutEffect }; diff --git a/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx b/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx index 1340a477d570ce..9ff986a4707aa9 100644 --- a/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx +++ b/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx @@ -46,6 +46,28 @@ describe('agnostic_renderer', () => { expect(renderSpy).toHaveBeenCalledTimes(1); }); + it('should not render again for unchanged callback functions', () => { + const renderSpy = jest.fn(); + const testCallback = () => {}; + const testState = { a: 'abc' }; + + render( + , + mountpoint + ); + render( + , + mountpoint + ); + expect(renderSpy).toHaveBeenCalledTimes(1); + }); + it('should render again once if props change', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc' }; From cc1a0dbaeac72b93140e33aa66fc73943e72256a Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 8 May 2019 15:17:26 +0200 Subject: [PATCH 05/22] rename test suite --- .../lens/public/utils/native_renderer/native_renderer.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx b/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx index 9ff986a4707aa9..3af468e3916681 100644 --- a/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx +++ b/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx @@ -8,7 +8,7 @@ import React from 'react'; import { render } from 'react-dom'; import { NativeRenderer } from './native_renderer'; -describe('agnostic_renderer', () => { +describe('native_renderer', () => { let mountpoint: Element; beforeEach(() => { From 18f9d838e5a358acad1ca2baac2a5681b1582ba7 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Thu, 9 May 2019 17:01:04 +0200 Subject: [PATCH 06/22] rename prop and adjust shallow comparison function --- x-pack/plugins/lens/public/app_plugin/app.tsx | 2 +- .../editor_frame_plugin/editor_frame.tsx | 2 +- .../native_renderer/native_renderer.test.tsx | 32 +++++++++---------- .../utils/native_renderer/native_renderer.tsx | 25 ++++++++------- 4 files changed, 32 insertions(+), 29 deletions(-) diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index d48badab710163..ba426fa9685a9e 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -16,7 +16,7 @@ export function App({ editorFrame }: { editorFrame: EditorFrameSetup }) {

Lens

- +
); diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx index 6dcc14542e21c0..bbfd2e6933e3c4 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx @@ -24,7 +24,7 @@ export function EditorFrame(props: EditorFrameProps) { {}, }} diff --git a/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx b/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx index 3af468e3916681..22c0a8d3940439 100644 --- a/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx +++ b/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx @@ -23,7 +23,7 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc' }; - render(, mountpoint); + render(, mountpoint); const containerElement = mountpoint.firstElementChild; expect(renderSpy).toHaveBeenCalledWith(containerElement, testProps); }); @@ -32,8 +32,8 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc' }; - render(, mountpoint); - render(, mountpoint); + render(, mountpoint); + render(, mountpoint); expect(renderSpy).toHaveBeenCalledTimes(1); }); @@ -41,8 +41,8 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc' }; - render(, mountpoint); - render(, mountpoint); + render(, mountpoint); + render(, mountpoint); expect(renderSpy).toHaveBeenCalledTimes(1); }); @@ -54,14 +54,14 @@ describe('native_renderer', () => { render( , mountpoint ); render( , mountpoint ); @@ -72,9 +72,9 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc' }; - render(, mountpoint); - render(, mountpoint); - render(, mountpoint); + render(, mountpoint); + render(, mountpoint); + render(, mountpoint); expect(renderSpy).toHaveBeenCalledTimes(2); const containerElement = mountpoint.firstElementChild; expect(renderSpy).lastCalledWith(containerElement, { a: 'def' }); @@ -84,8 +84,8 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc' }; - render(, mountpoint); - render(, mountpoint); + render(, mountpoint); + render(, mountpoint); expect(renderSpy).toHaveBeenCalledTimes(2); const containerElement = mountpoint.firstElementChild; expect(renderSpy).lastCalledWith(containerElement, { a: 'abc', b: 'def' }); @@ -95,8 +95,8 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc', b: 'def' }; - render(, mountpoint); - render(, mountpoint); + render(, mountpoint); + render(, mountpoint); expect(renderSpy).toHaveBeenCalledTimes(2); const containerElement = mountpoint.firstElementChild; expect(renderSpy).lastCalledWith(containerElement, { a: 'abc' }); @@ -106,7 +106,7 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc' }; - render(, mountpoint); + render(, mountpoint); const containerElement: Element = mountpoint.firstElementChild!; expect(containerElement.nodeName).toBe('DIV'); }); @@ -115,7 +115,7 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc' }; - render(, mountpoint); + render(, mountpoint); const containerElement: Element = mountpoint.firstElementChild!; expect(containerElement.nodeName).toBe('SPAN'); }); diff --git a/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx b/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx index 936433e4b2c784..c1af413c8ed291 100644 --- a/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx +++ b/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx @@ -8,43 +8,46 @@ import React, { useEffect, useRef } from 'react'; export interface NativeRendererProps { render: (domElement: Element, props: T) => void; - actualProps: T; + nativeProps: T; tag?: string; children?: never; } function isShallowDifferent(a: T, b: T): boolean { + if (a === b) { + return false; + } + + if (Object.keys(a).length !== Object.keys(b).length) { + return true; + } + for (const key in a) { if (!(key in b) || a[key] !== b[key]) { return true; } } - for (const key in b) { - if (!(key in a) || a[key] !== b[key]) { - return true; - } - } return false; } -export function NativeRenderer({ render, actualProps, tag }: NativeRendererProps) { +export function NativeRenderer({ render, nativeProps, tag }: NativeRendererProps) { const elementRef = useRef(null); const propsRef = useRef(null); function renderAndUpdate(element: Element) { elementRef.current = element; - propsRef.current = actualProps; - render(element, actualProps); + propsRef.current = nativeProps; + render(element, nativeProps); } useEffect( () => { - if (elementRef.current && isShallowDifferent(propsRef.current, actualProps)) { + if (elementRef.current && isShallowDifferent(propsRef.current, nativeProps)) { renderAndUpdate(elementRef.current); } }, - [actualProps] + [nativeProps] ); return React.createElement(tag || 'div', { From 60cd8911a98b7be1a7b47b7a454b0a9201747c0c Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Thu, 9 May 2019 17:17:23 +0200 Subject: [PATCH 07/22] add documentation --- .../public/utils/native_renderer/native_renderer.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx b/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx index c1af413c8ed291..f2693ae23c6975 100644 --- a/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx +++ b/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx @@ -31,6 +31,15 @@ function isShallowDifferent(a: T, b: T): boolean { return false; } +/** + * A component which takes care of providing a mountpoint for a generic render + * function which takes an html element and an optional props object. + * It also takes care of calling render again if the props object changes. + * By default the mountpoint element will be a div, this can be changed with the + * `tag` prop. + * + * @param props + */ export function NativeRenderer({ render, nativeProps, tag }: NativeRendererProps) { const elementRef = useRef(null); const propsRef = useRef(null); From 0eb1f317c8247ef23d473a2fceda8668b0a2b2eb Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Thu, 9 May 2019 17:23:25 +0200 Subject: [PATCH 08/22] move native renderer to top level directory --- x-pack/plugins/lens/public/app_plugin/app.tsx | 2 +- .../lens/public/editor_frame_plugin/editor_frame.tsx | 2 +- .../public/{utils => }/native_renderer/__mocks__/react.ts | 0 x-pack/plugins/lens/public/native_renderer/index.ts | 7 +++++++ .../{utils => }/native_renderer/native_renderer.test.tsx | 0 .../public/{utils => }/native_renderer/native_renderer.tsx | 0 6 files changed, 9 insertions(+), 2 deletions(-) rename x-pack/plugins/lens/public/{utils => }/native_renderer/__mocks__/react.ts (100%) create mode 100644 x-pack/plugins/lens/public/native_renderer/index.ts rename x-pack/plugins/lens/public/{utils => }/native_renderer/native_renderer.test.tsx (100%) rename x-pack/plugins/lens/public/{utils => }/native_renderer/native_renderer.tsx (100%) diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index ba426fa9685a9e..29f1c2eec03f0d 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -8,7 +8,7 @@ import { I18nProvider } from '@kbn/i18n/react'; import React from 'react'; import { EditorFrameSetup } from '../types'; -import { NativeRenderer } from '../utils/native_renderer/native_renderer'; +import { NativeRenderer } from '../native_renderer'; export function App({ editorFrame }: { editorFrame: EditorFrameSetup }) { return ( diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx index bbfd2e6933e3c4..c74a107b96313f 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx @@ -6,7 +6,7 @@ import React from 'react'; import { Datasource, Visualization } from '../types'; -import { NativeRenderer } from '../utils/native_renderer/native_renderer'; +import { NativeRenderer } from '../native_renderer'; interface EditorFrameProps { datasources: { [key: string]: Datasource }; diff --git a/x-pack/plugins/lens/public/utils/native_renderer/__mocks__/react.ts b/x-pack/plugins/lens/public/native_renderer/__mocks__/react.ts similarity index 100% rename from x-pack/plugins/lens/public/utils/native_renderer/__mocks__/react.ts rename to x-pack/plugins/lens/public/native_renderer/__mocks__/react.ts diff --git a/x-pack/plugins/lens/public/native_renderer/index.ts b/x-pack/plugins/lens/public/native_renderer/index.ts new file mode 100644 index 00000000000000..076e91d9501184 --- /dev/null +++ b/x-pack/plugins/lens/public/native_renderer/index.ts @@ -0,0 +1,7 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + + export * from './native_renderer'; \ No newline at end of file diff --git a/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx b/x-pack/plugins/lens/public/native_renderer/native_renderer.test.tsx similarity index 100% rename from x-pack/plugins/lens/public/utils/native_renderer/native_renderer.test.tsx rename to x-pack/plugins/lens/public/native_renderer/native_renderer.test.tsx diff --git a/x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx b/x-pack/plugins/lens/public/native_renderer/native_renderer.tsx similarity index 100% rename from x-pack/plugins/lens/public/utils/native_renderer/native_renderer.tsx rename to x-pack/plugins/lens/public/native_renderer/native_renderer.tsx From a21b0eaadb129690375afadbd78528f9d4e3e68c Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Fri, 10 May 2019 10:02:11 +0200 Subject: [PATCH 09/22] add test, fix linting error and improve shallow equal comparison --- .../lens/public/native_renderer/index.ts | 2 +- .../native_renderer/native_renderer.test.tsx | 12 +++++++++++ .../native_renderer/native_renderer.tsx | 21 ++++++++++++++----- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/lens/public/native_renderer/index.ts b/x-pack/plugins/lens/public/native_renderer/index.ts index 076e91d9501184..0ef9bd8807bc5b 100644 --- a/x-pack/plugins/lens/public/native_renderer/index.ts +++ b/x-pack/plugins/lens/public/native_renderer/index.ts @@ -4,4 +4,4 @@ * you may not use this file except in compliance with the Elastic License. */ - export * from './native_renderer'; \ No newline at end of file +export * from './native_renderer'; diff --git a/x-pack/plugins/lens/public/native_renderer/native_renderer.test.tsx b/x-pack/plugins/lens/public/native_renderer/native_renderer.test.tsx index 22c0a8d3940439..089b1ed02d166d 100644 --- a/x-pack/plugins/lens/public/native_renderer/native_renderer.test.tsx +++ b/x-pack/plugins/lens/public/native_renderer/native_renderer.test.tsx @@ -80,6 +80,18 @@ describe('native_renderer', () => { expect(renderSpy).lastCalledWith(containerElement, { a: 'def' }); }); + it('should render again once if props is just a string', () => { + const renderSpy = jest.fn(); + const testProps = 'abc'; + + render(, mountpoint); + render(, mountpoint); + render(, mountpoint); + expect(renderSpy).toHaveBeenCalledTimes(2); + const containerElement = mountpoint.firstElementChild; + expect(renderSpy).lastCalledWith(containerElement, 'def'); + }); + it('should render again if props are extended', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc' }; diff --git a/x-pack/plugins/lens/public/native_renderer/native_renderer.tsx b/x-pack/plugins/lens/public/native_renderer/native_renderer.tsx index f2693ae23c6975..f0eb4b829c153c 100644 --- a/x-pack/plugins/lens/public/native_renderer/native_renderer.tsx +++ b/x-pack/plugins/lens/public/native_renderer/native_renderer.tsx @@ -13,17 +13,28 @@ export interface NativeRendererProps { children?: never; } -function isShallowDifferent(a: T, b: T): boolean { - if (a === b) { +function is(x: unknown, y: unknown) { + return (x === y && (x !== 0 || 1 / (x as number) === 1 / (y as number))) || (x !== x && y !== y); +} + +function isShallowDifferent(objA: T, objB: T): boolean { + if (is(objA, objB)) { return false; } - if (Object.keys(a).length !== Object.keys(b).length) { + if (typeof objA !== 'object' || objA === null || typeof objB !== 'object' || objB === null) { + return true; + } + + const keysA = Object.keys(objA) as Array; + const keysB = Object.keys(objB) as Array; + + if (keysA.length !== keysB.length) { return true; } - for (const key in a) { - if (!(key in b) || a[key] !== b[key]) { + for (let i = 0; i < keysA.length; i++) { + if (!window.hasOwnProperty.call(objB, keysA[i]) || !is(objA[keysA[i]], objB[keysA[i]])) { return true; } } From 36de9426111d06c4cbc021ca9f1e9c66fa63a34a Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Fri, 3 May 2019 14:52:44 -0400 Subject: [PATCH 10/22] Implement basic editor frame state handling --- x-pack/plugins/lens/public/app_plugin/app.tsx | 15 +- .../plugins/lens/public/app_plugin/plugin.tsx | 13 +- .../editor_frame_plugin/editor_frame.test.tsx | 416 ++++++++++++++++++ .../editor_frame_plugin/editor_frame.tsx | 227 +++++----- .../public/editor_frame_plugin/plugin.tsx | 48 +- .../state_management.test.ts | 143 ++++++ .../editor_frame_plugin/state_management.ts | 98 +++++ x-pack/plugins/lens/public/index.ts | 2 +- .../indexpattern_plugin/indexpattern.tsx | 18 +- .../public/native_renderer/__mocks__/react.ts | 14 + .../lens/public/native_renderer/index.ts | 7 + .../native_renderer/native_renderer.test.tsx | 134 ++++++ .../native_renderer/native_renderer.tsx | 80 ++++ x-pack/plugins/lens/public/types.ts | 14 +- 14 files changed, 1085 insertions(+), 144 deletions(-) create mode 100644 x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.test.tsx create mode 100644 x-pack/plugins/lens/public/editor_frame_plugin/state_management.test.ts create mode 100644 x-pack/plugins/lens/public/editor_frame_plugin/state_management.ts create mode 100644 x-pack/plugins/lens/public/native_renderer/__mocks__/react.ts create mode 100644 x-pack/plugins/lens/public/native_renderer/index.ts create mode 100644 x-pack/plugins/lens/public/native_renderer/native_renderer.test.tsx create mode 100644 x-pack/plugins/lens/public/native_renderer/native_renderer.tsx diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 3e16a780830925..16860142e28d0b 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -5,23 +5,18 @@ */ import { I18nProvider } from '@kbn/i18n/react'; -import React, { useCallback } from 'react'; +import React from 'react'; -import { EditorFrameSetup } from '../types'; - -export function App({ editorFrame }: { editorFrame: EditorFrameSetup }) { - const renderFrame = useCallback(node => { - if (node !== null) { - editorFrame.render(node); - } - }, []); +import { EditorFrameInstance } from '../types'; +import { NativeRenderer } from '../native_renderer'; +export function App({ editorFrame }: { editorFrame: EditorFrameInstance }) { return (

Lens

-
+
); diff --git a/x-pack/plugins/lens/public/app_plugin/plugin.tsx b/x-pack/plugins/lens/public/app_plugin/plugin.tsx index 1a096d7c1326cf..aeea52c23c0e7a 100644 --- a/x-pack/plugins/lens/public/app_plugin/plugin.tsx +++ b/x-pack/plugins/lens/public/app_plugin/plugin.tsx @@ -9,8 +9,12 @@ import { editorFrameSetup, editorFrameStop } from '../editor_frame_plugin'; import { indexPatternDatasourceSetup, indexPatternDatasourceStop } from '../indexpattern_plugin'; import { xyVisualizationSetup, xyVisualizationStop } from '../xy_visualization_plugin'; import { App } from './app'; +import { EditorFrameInstance } from '../types'; export class AppPlugin { + + private instance: EditorFrameInstance | null = null; + constructor() {} setup() { @@ -23,10 +27,17 @@ export class AppPlugin { editorFrame.registerDatasource('indexpattern', indexPattern); editorFrame.registerVisualization('xy', xyVisualization); - return ; + this.instance = editorFrame.createInstance({}); + + return ; } stop() { + if (this.instance) { + this.instance.unmount(); + } + + // TODO this will be handled by the plugin platform itself indexPatternDatasourceStop(); xyVisualizationStop(); editorFrameStop(); diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.test.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.test.tsx new file mode 100644 index 00000000000000..5d9353113a1c9e --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.test.tsx @@ -0,0 +1,416 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import { mount, ReactWrapper } from 'enzyme'; +import { EditorFrame } from './editor_frame'; +import { Visualization, Datasource, VisualizationProps, DatasourcePublicAPI } from '../types'; + +const nextTick = () => new Promise(resolve => setTimeout(resolve)); + +describe('editor_frame', () => { + const getMockVisualization = () => ({ + getMappingOfTableToRoles: jest.fn(), + getPersistableState: jest.fn(), + getSuggestions: jest.fn(), + initialize: jest.fn(), + renderConfigPanel: jest.fn(), + toExpression: jest.fn(), + }); + + const getMockDatasource = () => ({ + getDatasourceSuggestionsForField: jest.fn(), + getDatasourceSuggestionsFromCurrentState: jest.fn(), + getPersistableState: jest.fn(), + getPublicAPI: jest.fn(), + initialize: jest.fn(() => Promise.resolve()), + renderDataPanel: jest.fn(), + toExpression: jest.fn(), + }); + + let mockVisualization: Visualization; + let mockDatasource: Datasource; + + let mockVisualization2: Visualization; + let mockDatasource2: Datasource; + + beforeEach(() => { + mockVisualization = getMockVisualization(); + mockVisualization2 = getMockVisualization(); + + mockDatasource = getMockDatasource(); + mockDatasource2 = getMockDatasource(); + }); + + describe('initialization', () => { + it('should initialize initial datasource and visualization if present', () => { + mount( + + ); + + expect(mockVisualization.initialize).toHaveBeenCalled(); + expect(mockDatasource.initialize).toHaveBeenCalled(); + }); + + it('should not initialize datasource and visualization if no initial one is specificed', () => { + mount( + + ); + + expect(mockVisualization.initialize).not.toHaveBeenCalled(); + expect(mockDatasource.initialize).not.toHaveBeenCalled(); + }); + + it('should not render something before datasource is initialized', () => { + mount( + + ); + + expect(mockVisualization.renderConfigPanel).not.toHaveBeenCalled(); + expect(mockDatasource.renderDataPanel).not.toHaveBeenCalled(); + }); + + it('should render data panel after initialization is complete', async () => { + const initialState = {}; + let databaseInitialized: ({}) => void; + + mount( + + new Promise(resolve => { + databaseInitialized = resolve; + }), + }, + }} + initialDatasource="testDatasource" + initialVisualization="testVis" + /> + ); + + await nextTick(); + databaseInitialized!(initialState); + + await nextTick(); + expect(mockDatasource.renderDataPanel).toHaveBeenCalledWith( + expect.any(Element), + expect.objectContaining({ state: initialState }) + ); + }); + + it('should initialize visualization state and render config panel', async () => { + const initialState = {}; + + mount( + initialState }, + }} + datasources={{ + testDatasource: { + ...mockDatasource, + initialize: () => Promise.resolve(), + }, + }} + initialDatasource="testDatasource" + initialVisualization="testVis" + /> + ); + + await nextTick(); + + expect(mockVisualization.renderConfigPanel).toHaveBeenCalledWith( + expect.any(Element), + expect.objectContaining({ state: initialState }) + ); + }); + }); + + describe('state update', () => { + it('should re-render config panel after state update', async () => { + mount( + + ); + + await nextTick(); + + const updatedState = {}; + const setVisualizationState = (mockVisualization.renderConfigPanel as jest.Mock).mock + .calls[0][1].setState; + setVisualizationState(updatedState); + + expect(mockVisualization.renderConfigPanel).toHaveBeenCalledTimes(2); + expect(mockVisualization.renderConfigPanel).toHaveBeenLastCalledWith( + expect.any(Element), + expect.objectContaining({ + state: updatedState, + }) + ); + + // don't re-render datasource when visulization changes + expect(mockDatasource.renderDataPanel).toHaveBeenCalledTimes(1); + }); + + it('should re-render data panel after state update', async () => { + mount( + + ); + + await nextTick(); + + const updatedState = {}; + const setDatasourceState = (mockDatasource.renderDataPanel as jest.Mock).mock + .calls[0][1].setState; + setDatasourceState(updatedState); + + expect(mockDatasource.renderDataPanel).toHaveBeenCalledTimes(2); + expect(mockDatasource.renderDataPanel).toHaveBeenLastCalledWith( + expect.any(Element), + expect.objectContaining({ + state: updatedState, + }) + ); + }); + + it('should re-render config panel with updated datasource api after datasource state update', async () => { + mount( + + ); + + await nextTick(); + + const updatedPublicAPI = {}; + mockDatasource.getPublicAPI = jest.fn(() => updatedPublicAPI as unknown as DatasourcePublicAPI); + + const setDatasourceState = (mockDatasource.renderDataPanel as jest.Mock).mock + .calls[0][1].setState; + setDatasourceState({}); + + expect(mockVisualization.renderConfigPanel).toHaveBeenCalledTimes(2); + expect(mockVisualization.renderConfigPanel).toHaveBeenLastCalledWith( + expect.any(Element), + expect.objectContaining({ + datasource: updatedPublicAPI, + }) + ); + }); + }); + + describe('datasource public api communication', () => { + it('should pass the datasource api to the visualization', async () => { + const publicAPI = ({} as unknown) as DatasourcePublicAPI; + + mockDatasource.getPublicAPI = () => publicAPI; + + mount( + + ); + + await nextTick(); + + expect(mockVisualization.renderConfigPanel).toHaveBeenCalledWith( + expect.any(Element), + expect.objectContaining({ datasource: publicAPI }) + ); + }); + + it('should give access to the datasource state in the datasource factory function', async () => { + const datasourceState = {}; + mockDatasource.initialize = () => Promise.resolve(datasourceState); + + mount( + + ); + + await nextTick(); + + expect(mockDatasource.getPublicAPI).toHaveBeenCalledWith( + datasourceState, + expect.any(Function) + ); + }); + + it('should re-create the public api after state has been set', async () => { + mount( + + ); + + await nextTick(); + + const updatedState = {}; + const setDatasourceState = (mockDatasource.getPublicAPI as jest.Mock).mock.calls[0][1]; + setDatasourceState(updatedState); + + expect(mockDatasource.getPublicAPI).toHaveBeenCalledTimes(2); + expect(mockDatasource.getPublicAPI).toHaveBeenLastCalledWith( + updatedState, + expect.any(Function) + ); + }); + }); + + describe('switching', () => { + let instance: ReactWrapper; + beforeEach(async () => { + instance = mount( + + ); + await nextTick(); + + // necessary to flush elements to dom synchronously + instance.update(); + }); + + it('should have initialized only the initial datasource and visualization', () => { + expect(mockDatasource.initialize).toHaveBeenCalled(); + expect(mockDatasource2.initialize).not.toHaveBeenCalled(); + + expect(mockVisualization.initialize).toHaveBeenCalled(); + expect(mockVisualization2.initialize).not.toHaveBeenCalled(); + }); + + it('should initialize other datasource on switch', async () => { + instance + .find('select[data-test-subj="datasource-switch"]') + .simulate('change', { target: { value: 'testDatasource2' } }); + expect(mockDatasource2.initialize).toHaveBeenCalled(); + }); + + it('should call datasource render with new state on switch', async () => { + const initialState = {}; + mockDatasource2.initialize = () => Promise.resolve(initialState); + + instance + .find('select[data-test-subj="datasource-switch"]') + .simulate('change', { target: { value: 'testDatasource2' } }); + + await nextTick(); + + expect(mockDatasource2.renderDataPanel).toHaveBeenCalledWith( + expect.any(Element), + expect.objectContaining({ state: initialState }) + ); + }); + + it('should initialize other visualization on switch', async () => { + instance + .find('select[data-test-subj="visualization-switch"]') + .simulate('change', { target: { value: 'testVis2' } }); + expect(mockVisualization2.initialize).toHaveBeenCalled(); + }); + + it('should call visualization render with new state on switch', async () => { + const initialState = {}; + mockVisualization2.initialize = () => initialState; + + instance + .find('select[data-test-subj="visualization-switch"]') + .simulate('change', { target: { value: 'testVis2' } }); + + expect(mockVisualization2.renderConfigPanel).toHaveBeenCalledWith( + expect.any(Element), + expect.objectContaining({ state: initialState }) + ); + }); + }); +}); diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx index 40b9ba40cd430a..13e184ccb4d41f 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx @@ -4,120 +4,149 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { useReducer, useEffect } from 'react'; -import { Datasource, Visualization } from '../types'; +import React, { useEffect, useReducer, useMemo } from 'react'; +import { EuiSelect, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; +import { Datasource, Visualization, DatasourceDataPanelProps } from '../types'; +import { NativeRenderer } from '../native_renderer'; +import { reducer, getInitialState } from './state_management'; -interface EditorFrameProps { +export interface EditorFrameProps { datasources: { [key: string]: Datasource }; visualizations: { [key: string]: Visualization }; - initialDatasource?: string; -} - -interface DatasourceState { - datasourceName: string; - visualizationName: string; - - datasourceState: any; - visualizationState: any; -} - -interface UpdateDatasourceAction { - type: 'UPDATE_DATASOURCE'; - payload: any; -} - -interface UpdateVisualizationAction { - type: 'UPDATE_VISUALIZATION'; - payload: any; -} - -type Action = UpdateDatasourceAction | UpdateVisualizationAction; - -function stateReducer(state: DatasourceState, action: Action): DatasourceState { - switch (action.type) { - case 'UPDATE_DATASOURCE': - return { - ...state, - datasourceState: action.payload, - }; - case 'UPDATE_VISUALIZATION': - return { - ...state, - visualizationState: action.payload, - }; - } - return state; + initialDatasource: string | null; + initialVisualization: string | null; } export function EditorFrame(props: EditorFrameProps) { - const dsKeys = Object.keys(props.datasources); - const vKeys = Object.keys(props.visualizations); - - const [state, dispatch] = useReducer(stateReducer, { - datasourceName: props.initialDatasource || dsKeys[0], - visualizationName: vKeys[0], + const [state, dispatch] = useReducer(reducer, props, getInitialState); + + // Initialize current datasource + useEffect( + () => { + let datasourceGotSwitched = false; + if (state.datasourceIsLoading && state.activeDatasource) { + props.datasources[state.activeDatasource].initialize().then(datasourceState => { + if (!datasourceGotSwitched) { + dispatch({ + type: 'UPDATE_DATASOURCE_STATE', + newState: datasourceState, + }); + } + }); + + return () => { + datasourceGotSwitched = true; + }; + } + }, + [state.activeDatasource, state.datasourceIsLoading] + ); - datasourceState: null, - visualizationState: null, - }); + const datasourceStateUpdater = useMemo( + () => (newState: unknown) => { + dispatch({ + type: 'UPDATE_DATASOURCE_STATE', + newState, + }); + }, + [dispatch] + ); - useEffect(() => { - const vState = props.visualizations[state.visualizationName].initialize(); - props.datasources[state.datasourceName].initialize().then(dsState => { + const visualizationStateUpdater = useMemo( + () => (newState: unknown) => { dispatch({ - type: 'UPDATE_DATASOURCE', - payload: dsState, + type: 'UPDATE_VISUALIZATION_STATE', + newState, }); - }); + }, + [dispatch] + ); - dispatch({ - type: 'UPDATE_VISUALIZATION', - payload: vState, - }); - }, []); + const datasourcePublicAPI = useMemo( + () => + state.activeDatasource && !state.datasourceIsLoading + ? props.datasources[state.activeDatasource].getPublicAPI( + state.datasourceState, + datasourceStateUpdater + ) + : undefined, + [ + props.datasources, + state.datasourceIsLoading, + state.activeDatasource, + datasourceStateUpdater, + state.datasourceState, + ] + ); - return ( -
-

Editor Frame

+ function renderDatasource() { + const datasourceProps: DatasourceDataPanelProps = { + state: state.datasourceState, + setState: datasourceStateUpdater, + }; + + return ( + <> + ({ + value: datasourceId, + text: datasourceId, + }))} + value={state.activeDatasource || undefined} + onChange={e => { + dispatch({ type: 'SWITCH_DATASOURCE', newDatasourceId: e.target.value }); + }} + /> + {state.activeDatasource && !state.datasourceIsLoading && ( + + )} + + ); + } -
{ - if (domElement) { - props.datasources[state.datasourceName].renderDataPanel(domElement, { - state: state.datasourceState, - setState: newState => - dispatch({ - type: 'UPDATE_DATASOURCE', - payload: newState, - }), + function renderVisualization() { + return ( + <> + ({ + value: visualizationId, + text: visualizationId, + }))} + value={state.activeDatasource || undefined} + onChange={e => { + dispatch({ + type: 'SWITCH_VISUALIZATION', + newVisulizationId: e.target.value, + // TODO we probably want to have a separate API to "force" a visualization switch + // which isn't a result of a picked suggestion + initialState: props.visualizations[e.target.value].initialize(), }); - } - }} - /> + }} + /> + {state.activeVisualization && state.activeDatasource && !state.datasourceIsLoading && ( + + )} + + ); + } -
{ - if (domElement) { - props.visualizations[state.visualizationName].renderConfigPanel(domElement, { - datasource: props.datasources[state.datasourceName].getPublicAPI( - state.datasourceState, - newState => - dispatch({ - type: 'UPDATE_DATASOURCE', - payload: newState, - }) - ), - state: state.visualizationState, - setState: newState => - dispatch({ - type: 'UPDATE_VISUALIZATION', - payload: newState, - }), - }); - } - }} - /> -
+ return ( + + {renderDatasource()} + {renderVisualization()} + ); } diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx index 6a0a82877cefba..1b1957bf8d316d 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx @@ -6,7 +6,7 @@ import React from 'react'; import { render, unmountComponentAtNode } from 'react-dom'; -import { Datasource, Visualization, EditorFrameSetup } from '../types'; +import { Datasource, Visualization, EditorFrameSetup, DatasourcePublicAPI } from '../types'; import { EditorFrame } from './editor_frame'; @@ -20,34 +20,39 @@ class EditorFramePlugin { [key: string]: Visualization; } = {}; - private initialDatasource?: string; - - private element: Element | null = null; - public setup(): EditorFrameSetup { return { - render: domElement => { - this.element = domElement; - render( - , - domElement - ); + createInstance: () => { + let domElement: Element; + return { + mount: (element: Element) => { + domElement = element; + render( + , + domElement + ); + }, + unmount: () => { + if (domElement) { + unmountComponentAtNode(domElement); + } + }, + }; }, - registerDatasource: (name, datasource) => { + registerDatasource: async (name, datasource) => { // casting it to an unknown datasource. This doesn't introduce runtime errors // because each type T is always also an unknown, but typescript won't do it // on it's own because we are loosing type information here. // So it's basically explicitly saying "I'm dropping the information about type T here // because this information isn't useful to me." but without using any which can leak - this.datasources[name] = datasource as Datasource; + // const state = await datasource.initialize(); - if (!this.initialDatasource) { - this.initialDatasource = name; - } + this.datasources[name] = datasource as Datasource; }, registerVisualization: (name, visualization) => { this.visualizations[name] = visualization as Visualization; @@ -56,9 +61,6 @@ class EditorFramePlugin { } public stop() { - if (this.element) { - unmountComponentAtNode(this.element); - } return {}; } } diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/state_management.test.ts b/x-pack/plugins/lens/public/editor_frame_plugin/state_management.test.ts new file mode 100644 index 00000000000000..896818bddfc0f9 --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_plugin/state_management.test.ts @@ -0,0 +1,143 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { getInitialState, reducer } from './state_management'; +import { EditorFrameProps } from './editor_frame'; +import { Datasource, Visualization } from '../types'; + +describe('editor_frame state management', () => { + describe('initialization', () => { + let props: EditorFrameProps; + + beforeEach(() => { + props = { + datasources: { testDatasource: ({} as unknown) as Datasource }, + visualizations: { testVis: ({ initialize: jest.fn() } as unknown) as Visualization }, + initialDatasource: 'testDatasource', + initialVisualization: 'testVis', + }; + }); + + it('should store initial datasource and visualization', () => { + expect(getInitialState(props)).toEqual( + expect.objectContaining({ + activeDatasource: 'testDatasource', + activeVisualization: 'testVis', + }) + ); + }); + + it('should initialize visualization', () => { + const initialVisState = {}; + props.visualizations.testVis.initialize = jest.fn(() => initialVisState); + + const initialState = getInitialState(props); + + expect(initialState.visualizationState.testVis).toBe(initialVisState); + expect(props.visualizations.testVis.initialize).toHaveBeenCalled(); + }); + + it('should not initialize visualization if no initial visualization is passed in', () => { + const initialState = getInitialState({ ...props, initialVisualization: null }); + + expect(initialState.visualizationState).toEqual({}); + expect(props.visualizations.testVis.initialize).not.toHaveBeenCalled(); + }); + }); + + describe('state update', () => { + it('should update the corresponding visualization state on update', () => { + const newVisState = {}; + const newState = reducer( + { + activeDatasource: 'testDatasource', + activeVisualization: 'testVis', + datasourceIsLoading: false, + datasourceState: {}, + visualizationState: { + testVis: {}, + }, + }, + { + type: 'UPDATE_VISUALIZATION_STATE', + newState: newVisState, + } + ); + + expect(newState.visualizationState).toEqual({ + testVis: newVisState, + }); + }); + + it('should update the datasource state on update', () => { + const newDatasourceState = {}; + const newState = reducer( + { + activeDatasource: 'testDatasource', + activeVisualization: 'testVis', + datasourceIsLoading: false, + datasourceState: {}, + visualizationState: { + testVis: {}, + }, + }, + { + type: 'UPDATE_DATASOURCE_STATE', + newState: newDatasourceState, + } + ); + + expect(newState.datasourceState).toBe(newDatasourceState); + }); + + it('should should switch active visualization but dont loose old state', () => { + const testVisState = {}; + const newVisState = {}; + const newState = reducer( + { + activeDatasource: 'testDatasource', + activeVisualization: 'testVis', + datasourceIsLoading: false, + datasourceState: {}, + visualizationState: { + testVis: testVisState, + }, + }, + { + type: 'SWITCH_VISUALIZATION', + newVisulizationId: 'testVis2', + initialState: newVisState + } + ); + + expect(newState.visualizationState.testVis).toBe(testVisState); + expect(newState.visualizationState.testVis2).toBe(newVisState); + }); + + it('should should switch active datasource and purge visualization state', () => { + const newState = reducer( + { + activeDatasource: 'testDatasource', + activeVisualization: 'testVis', + datasourceIsLoading: false, + datasourceState: {}, + visualizationState: { + testVis: {}, + }, + }, + { + type: 'SWITCH_DATASOURCE', + newDatasourceId: 'testDatasource2' + } + ); + + expect(newState.visualizationState).toEqual({}); + expect(newState.activeVisualization).toBe(null); + expect(newState.activeDatasource).toBe('testDatasource2'); + expect(newState.datasourceState).toBe(null); + }); + }); +}); diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/state_management.ts b/x-pack/plugins/lens/public/editor_frame_plugin/state_management.ts new file mode 100644 index 00000000000000..e35e258d1153cd --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_plugin/state_management.ts @@ -0,0 +1,98 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { EditorFrameProps } from './editor_frame'; + +export interface EditorFrameState { + activeDatasource: string | null; + datasourceState: unknown; + datasourceIsLoading: boolean; + + activeVisualization: string | null; + visualizationState: { + [visualizationId: string]: unknown; + }; +} + +export type Action = + | { + type: 'UPDATE_DATASOURCE_STATE'; + newState: unknown; + } + | { + type: 'UPDATE_VISUALIZATION_STATE'; + newState: unknown; + } + | { + type: 'SWITCH_VISUALIZATION'; + newVisulizationId: string; + initialState: unknown; + } + | { + type: 'SWITCH_DATASOURCE'; + newDatasourceId: string; + }; + +export const getInitialState = (props: EditorFrameProps) => { + return { + datasourceState: null, + datasourceIsLoading: Boolean(props.initialDatasource), + activeDatasource: props.initialDatasource, + visualizationState: props.initialVisualization + ? { + [props.initialVisualization]: props.visualizations[ + props.initialVisualization + ].initialize(), + } + : {}, + activeVisualization: props.initialVisualization, + }; +}; + +export const reducer = (state: EditorFrameState, action: Action): EditorFrameState => { + switch (action.type) { + case 'SWITCH_DATASOURCE': + return { + ...state, + activeDatasource: action.newDatasourceId, + // purge all visualizations on datasource switch + visualizationState: {}, + activeVisualization: null, + datasourceState: null, + datasourceIsLoading: true, + }; + case 'SWITCH_VISUALIZATION': + return { + ...state, + activeVisualization: action.newVisulizationId, + visualizationState: { + ...state.visualizationState, + [action.newVisulizationId]: action.initialState, + }, + }; + case 'UPDATE_DATASOURCE_STATE': + return { + ...state, + // when the datasource state is updated, the initialization is complete + datasourceIsLoading: false, + datasourceState: action.newState, + }; + case 'UPDATE_VISUALIZATION_STATE': + if (state.activeVisualization) { + return { + ...state, + visualizationState: { + ...state.visualizationState, + [state.activeVisualization]: action.newState, + }, + }; + } else { + throw new Error('Invariant: visualization state got updated without active visualization'); + } + default: + return state; + } +}; diff --git a/x-pack/plugins/lens/public/index.ts b/x-pack/plugins/lens/public/index.ts index 874249ff75d85c..f8f074cdb99bfd 100644 --- a/x-pack/plugins/lens/public/index.ts +++ b/x-pack/plugins/lens/public/index.ts @@ -6,8 +6,8 @@ export * from './types'; -import { IScope } from 'angular'; import { render, unmountComponentAtNode } from 'react-dom'; +import { IScope } from 'angular'; import chrome from 'ui/chrome'; import { appSetup, appStop } from './app_plugin'; diff --git a/x-pack/plugins/lens/public/indexpattern_plugin/indexpattern.tsx b/x-pack/plugins/lens/public/indexpattern_plugin/indexpattern.tsx index 29b018320a6dbb..567bc6cfd10a87 100644 --- a/x-pack/plugins/lens/public/indexpattern_plugin/indexpattern.tsx +++ b/x-pack/plugins/lens/public/indexpattern_plugin/indexpattern.tsx @@ -36,6 +36,13 @@ export interface Field { searchable: boolean; } +export interface Field { + name: string; + type: string; + aggregatable: boolean; + searchable: boolean; +} + export interface IndexPatternPersistedState { currentIndexPattern: string; @@ -49,12 +56,13 @@ export type IndexPatternPrivateState = IndexPatternPersistedState & { indexPatterns: { [id: string]: IndexPattern }; }; +type PersistedKeys = 'currentIndexPattern' | 'columns' | 'columnOrder'; + +export type PersistableIndexPatternPrivateState = Pick; + // Not stateful. State is persisted to the frame -export const indexPatternDatasource: Datasource< - IndexPatternPrivateState, - IndexPatternPersistedState -> = { - async initialize(state?: IndexPatternPersistedState) { +export const indexPatternDatasource: Datasource = { + async initialize(state?: PersistableIndexPatternPrivateState) { // TODO: Make fetch request to load indexPatterns from saved objects if (state) { return { diff --git a/x-pack/plugins/lens/public/native_renderer/__mocks__/react.ts b/x-pack/plugins/lens/public/native_renderer/__mocks__/react.ts new file mode 100644 index 00000000000000..6039f0888f596c --- /dev/null +++ b/x-pack/plugins/lens/public/native_renderer/__mocks__/react.ts @@ -0,0 +1,14 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; + +// This mock overwrites react to use the layout effect hook instead of the normal one. +// This is done to have effects executed synchronously to be able to test them without +// setTimeout hacks. + +// eslint-disable-next-line import/no-default-export +export default { ...React, useEffect: React.useLayoutEffect }; diff --git a/x-pack/plugins/lens/public/native_renderer/index.ts b/x-pack/plugins/lens/public/native_renderer/index.ts new file mode 100644 index 00000000000000..0ef9bd8807bc5b --- /dev/null +++ b/x-pack/plugins/lens/public/native_renderer/index.ts @@ -0,0 +1,7 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +export * from './native_renderer'; diff --git a/x-pack/plugins/lens/public/native_renderer/native_renderer.test.tsx b/x-pack/plugins/lens/public/native_renderer/native_renderer.test.tsx new file mode 100644 index 00000000000000..089b1ed02d166d --- /dev/null +++ b/x-pack/plugins/lens/public/native_renderer/native_renderer.test.tsx @@ -0,0 +1,134 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import { render } from 'react-dom'; +import { NativeRenderer } from './native_renderer'; + +describe('native_renderer', () => { + let mountpoint: Element; + + beforeEach(() => { + mountpoint = document.createElement('div'); + }); + + afterEach(() => { + mountpoint.remove(); + }); + + it('should render element in container', () => { + const renderSpy = jest.fn(); + const testProps = { a: 'abc' }; + + render(, mountpoint); + const containerElement = mountpoint.firstElementChild; + expect(renderSpy).toHaveBeenCalledWith(containerElement, testProps); + }); + + it('should not render again if props do not change', () => { + const renderSpy = jest.fn(); + const testProps = { a: 'abc' }; + + render(, mountpoint); + render(, mountpoint); + expect(renderSpy).toHaveBeenCalledTimes(1); + }); + + it('should not render again if props do not change shallowly', () => { + const renderSpy = jest.fn(); + const testProps = { a: 'abc' }; + + render(, mountpoint); + render(, mountpoint); + expect(renderSpy).toHaveBeenCalledTimes(1); + }); + + it('should not render again for unchanged callback functions', () => { + const renderSpy = jest.fn(); + const testCallback = () => {}; + const testState = { a: 'abc' }; + + render( + , + mountpoint + ); + render( + , + mountpoint + ); + expect(renderSpy).toHaveBeenCalledTimes(1); + }); + + it('should render again once if props change', () => { + const renderSpy = jest.fn(); + const testProps = { a: 'abc' }; + + render(, mountpoint); + render(, mountpoint); + render(, mountpoint); + expect(renderSpy).toHaveBeenCalledTimes(2); + const containerElement = mountpoint.firstElementChild; + expect(renderSpy).lastCalledWith(containerElement, { a: 'def' }); + }); + + it('should render again once if props is just a string', () => { + const renderSpy = jest.fn(); + const testProps = 'abc'; + + render(, mountpoint); + render(, mountpoint); + render(, mountpoint); + expect(renderSpy).toHaveBeenCalledTimes(2); + const containerElement = mountpoint.firstElementChild; + expect(renderSpy).lastCalledWith(containerElement, 'def'); + }); + + it('should render again if props are extended', () => { + const renderSpy = jest.fn(); + const testProps = { a: 'abc' }; + + render(, mountpoint); + render(, mountpoint); + expect(renderSpy).toHaveBeenCalledTimes(2); + const containerElement = mountpoint.firstElementChild; + expect(renderSpy).lastCalledWith(containerElement, { a: 'abc', b: 'def' }); + }); + + it('should render again if props are limited', () => { + const renderSpy = jest.fn(); + const testProps = { a: 'abc', b: 'def' }; + + render(, mountpoint); + render(, mountpoint); + expect(renderSpy).toHaveBeenCalledTimes(2); + const containerElement = mountpoint.firstElementChild; + expect(renderSpy).lastCalledWith(containerElement, { a: 'abc' }); + }); + + it('should render a div as container', () => { + const renderSpy = jest.fn(); + const testProps = { a: 'abc' }; + + render(, mountpoint); + const containerElement: Element = mountpoint.firstElementChild!; + expect(containerElement.nodeName).toBe('DIV'); + }); + + it('should render a specified element as container', () => { + const renderSpy = jest.fn(); + const testProps = { a: 'abc' }; + + render(, mountpoint); + const containerElement: Element = mountpoint.firstElementChild!; + expect(containerElement.nodeName).toBe('SPAN'); + }); +}); diff --git a/x-pack/plugins/lens/public/native_renderer/native_renderer.tsx b/x-pack/plugins/lens/public/native_renderer/native_renderer.tsx new file mode 100644 index 00000000000000..f0eb4b829c153c --- /dev/null +++ b/x-pack/plugins/lens/public/native_renderer/native_renderer.tsx @@ -0,0 +1,80 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { useEffect, useRef } from 'react'; + +export interface NativeRendererProps { + render: (domElement: Element, props: T) => void; + nativeProps: T; + tag?: string; + children?: never; +} + +function is(x: unknown, y: unknown) { + return (x === y && (x !== 0 || 1 / (x as number) === 1 / (y as number))) || (x !== x && y !== y); +} + +function isShallowDifferent(objA: T, objB: T): boolean { + if (is(objA, objB)) { + return false; + } + + if (typeof objA !== 'object' || objA === null || typeof objB !== 'object' || objB === null) { + return true; + } + + const keysA = Object.keys(objA) as Array; + const keysB = Object.keys(objB) as Array; + + if (keysA.length !== keysB.length) { + return true; + } + + for (let i = 0; i < keysA.length; i++) { + if (!window.hasOwnProperty.call(objB, keysA[i]) || !is(objA[keysA[i]], objB[keysA[i]])) { + return true; + } + } + + return false; +} + +/** + * A component which takes care of providing a mountpoint for a generic render + * function which takes an html element and an optional props object. + * It also takes care of calling render again if the props object changes. + * By default the mountpoint element will be a div, this can be changed with the + * `tag` prop. + * + * @param props + */ +export function NativeRenderer({ render, nativeProps, tag }: NativeRendererProps) { + const elementRef = useRef(null); + const propsRef = useRef(null); + + function renderAndUpdate(element: Element) { + elementRef.current = element; + propsRef.current = nativeProps; + render(element, nativeProps); + } + + useEffect( + () => { + if (elementRef.current && isShallowDifferent(propsRef.current, nativeProps)) { + renderAndUpdate(elementRef.current); + } + }, + [nativeProps] + ); + + return React.createElement(tag || 'div', { + ref: element => { + if (element && element !== elementRef.current) { + renderAndUpdate(element); + } + }, + }); +} diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index ab70ff10e1cb54..67f4d2cdb09ddb 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -4,8 +4,15 @@ * you may not use this file except in compliance with the Elastic License. */ +export interface EditorFrameOptions { +} + +export interface EditorFrameInstance { + mount: (element: Element) => void; + unmount: () => void; +} export interface EditorFrameSetup { - render: (domElement: Element) => void; + createInstance: (options: EditorFrameOptions) => EditorFrameInstance; // generic type on the API functions to pull the "unknown vs. specific type" error into the implementation registerDatasource: (name: string, datasource: Datasource) => void; registerVisualization: (name: string, visualization: Visualization) => void; @@ -139,12 +146,9 @@ export interface VisualizationSuggestion { } export interface Visualization { - // For initializing, either from an empty state or from persisted state - // Because this will be called at runtime, state might have a type of `any` and - // visualizations should validate their arguments + // For initializing from saved object initialize: (state?: P) => T; - // Given the current state, which parts should be saved? getPersistableState: (state: T) => P; renderConfigPanel: (domElement: Element, props: VisualizationProps) => void; From 8147aaa957ce0b9505ec3c8cd6c6cb0f9d3b71ec Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Fri, 10 May 2019 15:49:02 +0200 Subject: [PATCH 11/22] fix linting errors --- x-pack/plugins/lens/public/app_plugin/plugin.tsx | 1 - .../editor_frame_plugin/editor_frame.test.tsx | 16 +++++++++------- .../editor_frame_plugin/state_management.test.ts | 4 ++-- .../public/indexpattern_plugin/indexpattern.tsx | 5 ++++- x-pack/plugins/lens/public/types.ts | 4 ++-- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/lens/public/app_plugin/plugin.tsx b/x-pack/plugins/lens/public/app_plugin/plugin.tsx index aeea52c23c0e7a..857cee9adbc64d 100644 --- a/x-pack/plugins/lens/public/app_plugin/plugin.tsx +++ b/x-pack/plugins/lens/public/app_plugin/plugin.tsx @@ -12,7 +12,6 @@ import { App } from './app'; import { EditorFrameInstance } from '../types'; export class AppPlugin { - private instance: EditorFrameInstance | null = null; constructor() {} diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.test.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.test.tsx index 5d9353113a1c9e..b1c46afd5e1dca 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.test.tsx @@ -194,7 +194,7 @@ describe('editor_frame', () => { // don't re-render datasource when visulization changes expect(mockDatasource.renderDataPanel).toHaveBeenCalledTimes(1); }); - + it('should re-render data panel after state update', async () => { mount( { await nextTick(); const updatedState = {}; - const setDatasourceState = (mockDatasource.renderDataPanel as jest.Mock).mock - .calls[0][1].setState; + const setDatasourceState = (mockDatasource.renderDataPanel as jest.Mock).mock.calls[0][1] + .setState; setDatasourceState(updatedState); expect(mockDatasource.renderDataPanel).toHaveBeenCalledTimes(2); @@ -224,7 +224,7 @@ describe('editor_frame', () => { }) ); }); - + it('should re-render config panel with updated datasource api after datasource state update', async () => { mount( { await nextTick(); const updatedPublicAPI = {}; - mockDatasource.getPublicAPI = jest.fn(() => updatedPublicAPI as unknown as DatasourcePublicAPI); + mockDatasource.getPublicAPI = jest.fn( + () => (updatedPublicAPI as unknown) as DatasourcePublicAPI + ); - const setDatasourceState = (mockDatasource.renderDataPanel as jest.Mock).mock - .calls[0][1].setState; + const setDatasourceState = (mockDatasource.renderDataPanel as jest.Mock).mock.calls[0][1] + .setState; setDatasourceState({}); expect(mockVisualization.renderConfigPanel).toHaveBeenCalledTimes(2); diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/state_management.test.ts b/x-pack/plugins/lens/public/editor_frame_plugin/state_management.test.ts index 896818bddfc0f9..fb96817685c45a 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/state_management.test.ts +++ b/x-pack/plugins/lens/public/editor_frame_plugin/state_management.test.ts @@ -109,7 +109,7 @@ describe('editor_frame state management', () => { { type: 'SWITCH_VISUALIZATION', newVisulizationId: 'testVis2', - initialState: newVisState + initialState: newVisState, } ); @@ -130,7 +130,7 @@ describe('editor_frame state management', () => { }, { type: 'SWITCH_DATASOURCE', - newDatasourceId: 'testDatasource2' + newDatasourceId: 'testDatasource2', } ); diff --git a/x-pack/plugins/lens/public/indexpattern_plugin/indexpattern.tsx b/x-pack/plugins/lens/public/indexpattern_plugin/indexpattern.tsx index 567bc6cfd10a87..53522ff39d6fb2 100644 --- a/x-pack/plugins/lens/public/indexpattern_plugin/indexpattern.tsx +++ b/x-pack/plugins/lens/public/indexpattern_plugin/indexpattern.tsx @@ -61,7 +61,10 @@ type PersistedKeys = 'currentIndexPattern' | 'columns' | 'columnOrder'; export type PersistableIndexPatternPrivateState = Pick; // Not stateful. State is persisted to the frame -export const indexPatternDatasource: Datasource = { +export const indexPatternDatasource: Datasource< + IndexPatternPrivateState, + PersistableIndexPatternPrivateState +> = { async initialize(state?: PersistableIndexPatternPrivateState) { // TODO: Make fetch request to load indexPatterns from saved objects if (state) { diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index 67f4d2cdb09ddb..a0328cb3bd9881 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -4,8 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ -export interface EditorFrameOptions { -} +// eslint-disable-next-line +export interface EditorFrameOptions {} export interface EditorFrameInstance { mount: (element: Element) => void; From 4baf6b196405d6f563e4078fa8b0fbe0df82d83d Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Fri, 10 May 2019 16:49:44 +0200 Subject: [PATCH 12/22] fix native renderer tests --- .../public/native_renderer/__mocks__/react.ts | 14 --- .../native_renderer/native_renderer.test.tsx | 87 +++++++++++++++---- 2 files changed, 70 insertions(+), 31 deletions(-) delete mode 100644 x-pack/plugins/lens/public/native_renderer/__mocks__/react.ts diff --git a/x-pack/plugins/lens/public/native_renderer/__mocks__/react.ts b/x-pack/plugins/lens/public/native_renderer/__mocks__/react.ts deleted file mode 100644 index 6039f0888f596c..00000000000000 --- a/x-pack/plugins/lens/public/native_renderer/__mocks__/react.ts +++ /dev/null @@ -1,14 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import React from 'react'; - -// This mock overwrites react to use the layout effect hook instead of the normal one. -// This is done to have effects executed synchronously to be able to test them without -// setTimeout hacks. - -// eslint-disable-next-line import/no-default-export -export default { ...React, useEffect: React.useLayoutEffect }; diff --git a/x-pack/plugins/lens/public/native_renderer/native_renderer.test.tsx b/x-pack/plugins/lens/public/native_renderer/native_renderer.test.tsx index 089b1ed02d166d..af5196165f9a55 100644 --- a/x-pack/plugins/lens/public/native_renderer/native_renderer.test.tsx +++ b/x-pack/plugins/lens/public/native_renderer/native_renderer.test.tsx @@ -7,6 +7,14 @@ import React from 'react'; import { render } from 'react-dom'; import { NativeRenderer } from './native_renderer'; +import { act } from 'react-dom/test-utils'; + +function renderAndTriggerHooks(element: JSX.Element, mountpoint: Element) { + // act takes care of triggering state hooks + act(() => { + render(element, mountpoint); + }); +} describe('native_renderer', () => { let mountpoint: Element; @@ -23,7 +31,10 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc' }; - render(, mountpoint); + renderAndTriggerHooks( + , + mountpoint + ); const containerElement = mountpoint.firstElementChild; expect(renderSpy).toHaveBeenCalledWith(containerElement, testProps); }); @@ -32,8 +43,14 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc' }; - render(, mountpoint); - render(, mountpoint); + renderAndTriggerHooks( + , + mountpoint + ); + renderAndTriggerHooks( + , + mountpoint + ); expect(renderSpy).toHaveBeenCalledTimes(1); }); @@ -41,8 +58,14 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc' }; - render(, mountpoint); - render(, mountpoint); + renderAndTriggerHooks( + , + mountpoint + ); + renderAndTriggerHooks( + , + mountpoint + ); expect(renderSpy).toHaveBeenCalledTimes(1); }); @@ -72,9 +95,18 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc' }; - render(, mountpoint); - render(, mountpoint); - render(, mountpoint); + renderAndTriggerHooks( + , + mountpoint + ); + renderAndTriggerHooks( + , + mountpoint + ); + renderAndTriggerHooks( + , + mountpoint + ); expect(renderSpy).toHaveBeenCalledTimes(2); const containerElement = mountpoint.firstElementChild; expect(renderSpy).lastCalledWith(containerElement, { a: 'def' }); @@ -84,9 +116,12 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = 'abc'; - render(, mountpoint); - render(, mountpoint); - render(, mountpoint); + renderAndTriggerHooks( + , + mountpoint + ); + renderAndTriggerHooks(, mountpoint); + renderAndTriggerHooks(, mountpoint); expect(renderSpy).toHaveBeenCalledTimes(2); const containerElement = mountpoint.firstElementChild; expect(renderSpy).lastCalledWith(containerElement, 'def'); @@ -96,8 +131,14 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc' }; - render(, mountpoint); - render(, mountpoint); + renderAndTriggerHooks( + , + mountpoint + ); + renderAndTriggerHooks( + , + mountpoint + ); expect(renderSpy).toHaveBeenCalledTimes(2); const containerElement = mountpoint.firstElementChild; expect(renderSpy).lastCalledWith(containerElement, { a: 'abc', b: 'def' }); @@ -107,8 +148,14 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc', b: 'def' }; - render(, mountpoint); - render(, mountpoint); + renderAndTriggerHooks( + , + mountpoint + ); + renderAndTriggerHooks( + , + mountpoint + ); expect(renderSpy).toHaveBeenCalledTimes(2); const containerElement = mountpoint.firstElementChild; expect(renderSpy).lastCalledWith(containerElement, { a: 'abc' }); @@ -118,7 +165,10 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc' }; - render(, mountpoint); + renderAndTriggerHooks( + , + mountpoint + ); const containerElement: Element = mountpoint.firstElementChild!; expect(containerElement.nodeName).toBe('DIV'); }); @@ -127,7 +177,10 @@ describe('native_renderer', () => { const renderSpy = jest.fn(); const testProps = { a: 'abc' }; - render(, mountpoint); + renderAndTriggerHooks( + , + mountpoint + ); const containerElement: Element = mountpoint.firstElementChild!; expect(containerElement.nodeName).toBe('SPAN'); }); From 033a632ba6b7c4f92e22370fe4460685d3dd647c Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Fri, 10 May 2019 17:31:15 +0200 Subject: [PATCH 13/22] re structure editor frame --- .../editor_frame_plugin/editor_frame.tsx | 152 ------------------ .../editor_frame/config_panel_wrapper.tsx | 63 ++++++++ .../editor_frame/data_panel_wrapper.tsx | 58 +++++++ .../{ => editor_frame}/editor_frame.test.tsx | 2 +- .../editor_frame/editor_frame.tsx | 88 ++++++++++ .../editor_frame_plugin/editor_frame/index.ts | 7 + .../public/editor_frame_plugin/plugin.tsx | 2 +- 7 files changed, 218 insertions(+), 154 deletions(-) delete mode 100644 x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx create mode 100644 x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx create mode 100644 x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/data_panel_wrapper.tsx rename x-pack/plugins/lens/public/editor_frame_plugin/{ => editor_frame}/editor_frame.test.tsx (99%) create mode 100644 x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx create mode 100644 x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/index.ts diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx deleted file mode 100644 index 13e184ccb4d41f..00000000000000 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.tsx +++ /dev/null @@ -1,152 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import React, { useEffect, useReducer, useMemo } from 'react'; -import { EuiSelect, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; -import { Datasource, Visualization, DatasourceDataPanelProps } from '../types'; -import { NativeRenderer } from '../native_renderer'; -import { reducer, getInitialState } from './state_management'; - -export interface EditorFrameProps { - datasources: { [key: string]: Datasource }; - visualizations: { [key: string]: Visualization }; - - initialDatasource: string | null; - initialVisualization: string | null; -} - -export function EditorFrame(props: EditorFrameProps) { - const [state, dispatch] = useReducer(reducer, props, getInitialState); - - // Initialize current datasource - useEffect( - () => { - let datasourceGotSwitched = false; - if (state.datasourceIsLoading && state.activeDatasource) { - props.datasources[state.activeDatasource].initialize().then(datasourceState => { - if (!datasourceGotSwitched) { - dispatch({ - type: 'UPDATE_DATASOURCE_STATE', - newState: datasourceState, - }); - } - }); - - return () => { - datasourceGotSwitched = true; - }; - } - }, - [state.activeDatasource, state.datasourceIsLoading] - ); - - const datasourceStateUpdater = useMemo( - () => (newState: unknown) => { - dispatch({ - type: 'UPDATE_DATASOURCE_STATE', - newState, - }); - }, - [dispatch] - ); - - const visualizationStateUpdater = useMemo( - () => (newState: unknown) => { - dispatch({ - type: 'UPDATE_VISUALIZATION_STATE', - newState, - }); - }, - [dispatch] - ); - - const datasourcePublicAPI = useMemo( - () => - state.activeDatasource && !state.datasourceIsLoading - ? props.datasources[state.activeDatasource].getPublicAPI( - state.datasourceState, - datasourceStateUpdater - ) - : undefined, - [ - props.datasources, - state.datasourceIsLoading, - state.activeDatasource, - datasourceStateUpdater, - state.datasourceState, - ] - ); - - function renderDatasource() { - const datasourceProps: DatasourceDataPanelProps = { - state: state.datasourceState, - setState: datasourceStateUpdater, - }; - - return ( - <> - ({ - value: datasourceId, - text: datasourceId, - }))} - value={state.activeDatasource || undefined} - onChange={e => { - dispatch({ type: 'SWITCH_DATASOURCE', newDatasourceId: e.target.value }); - }} - /> - {state.activeDatasource && !state.datasourceIsLoading && ( - - )} - - ); - } - - function renderVisualization() { - return ( - <> - ({ - value: visualizationId, - text: visualizationId, - }))} - value={state.activeDatasource || undefined} - onChange={e => { - dispatch({ - type: 'SWITCH_VISUALIZATION', - newVisulizationId: e.target.value, - // TODO we probably want to have a separate API to "force" a visualization switch - // which isn't a result of a picked suggestion - initialState: props.visualizations[e.target.value].initialize(), - }); - }} - /> - {state.activeVisualization && state.activeDatasource && !state.datasourceIsLoading && ( - - )} - - ); - } - - return ( - - {renderDatasource()} - {renderVisualization()} - - ); -} diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx new file mode 100644 index 00000000000000..e494b8f81d7866 --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx @@ -0,0 +1,63 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { useMemo } from 'react'; +import { EuiSelect } from '@elastic/eui'; +import { NativeRenderer } from '../../native_renderer'; +import { Action } from '../state_management'; +import { Visualization, DatasourcePublicAPI } from '../../types'; + +interface ConfigPanelWrapperProps { + visualizationState: Record; + visualizations: Record; + activeVisualization: string | null; + dispatch: (action: Action) => void; + datasourcePublicAPI: DatasourcePublicAPI; +} + +export function ConfigPanelWrapper(props: ConfigPanelWrapperProps) { + const setVisualizationState = useMemo( + () => (newState: unknown) => { + props.dispatch({ + type: 'UPDATE_VISUALIZATION_STATE', + newState, + }); + }, + [props.dispatch] + ); + + return ( + <> + ({ + value: visualizationId, + text: visualizationId, + }))} + value={props.activeVisualization || undefined} + onChange={e => { + props.dispatch({ + type: 'SWITCH_VISUALIZATION', + newVisulizationId: e.target.value, + // TODO we probably want to have a separate API to "force" a visualization switch + // which isn't a result of a picked suggestion + initialState: props.visualizations[e.target.value].initialize(), + }); + }} + /> + {props.activeVisualization && ( + + )} + + ); +} diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/data_panel_wrapper.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/data_panel_wrapper.tsx new file mode 100644 index 00000000000000..2fdf509c6716e1 --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/data_panel_wrapper.tsx @@ -0,0 +1,58 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { useMemo } from 'react'; +import { EuiSelect } from '@elastic/eui'; +import { DatasourceDataPanelProps, Datasource } from '../..'; +import { NativeRenderer } from '../../native_renderer'; +import { Action } from '../state_management'; + +interface DataPanelWrapperProps { + datasourceState: unknown; + datasources: Record; + activeDatasource: string | null; + datasourceIsLoading: boolean; + dispatch: (action: Action) => void; +} + +export function DataPanelWrapper(props: DataPanelWrapperProps) { + const setDatasourceState = useMemo( + () => (newState: unknown) => { + props.dispatch({ + type: 'UPDATE_DATASOURCE_STATE', + newState, + }); + }, + [props.dispatch] + ); + + const datasourceProps: DatasourceDataPanelProps = { + state: props.datasourceState, + setState: setDatasourceState, + }; + + return ( + <> + ({ + value: datasourceId, + text: datasourceId, + }))} + value={props.activeDatasource || undefined} + onChange={e => { + props.dispatch({ type: 'SWITCH_DATASOURCE', newDatasourceId: e.target.value }); + }} + /> + {props.activeDatasource && !props.datasourceIsLoading && ( + + )} + + ); +} diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.test.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx similarity index 99% rename from x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.test.tsx rename to x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx index b1c46afd5e1dca..1b2496b570d825 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx @@ -7,7 +7,7 @@ import React from 'react'; import { mount, ReactWrapper } from 'enzyme'; import { EditorFrame } from './editor_frame'; -import { Visualization, Datasource, VisualizationProps, DatasourcePublicAPI } from '../types'; +import { Visualization, Datasource, DatasourcePublicAPI } from '../../types'; const nextTick = () => new Promise(resolve => setTimeout(resolve)); diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx new file mode 100644 index 00000000000000..3f92021f8c2ad1 --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx @@ -0,0 +1,88 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { useEffect, useReducer, useMemo } from 'react'; +import { EuiSelect, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; +import { Datasource, Visualization, DatasourceDataPanelProps } from '../../types'; +import { NativeRenderer } from '../../native_renderer'; +import { reducer, getInitialState } from '../state_management'; +import { DataPanelWrapper } from './data_panel_wrapper'; +import { ConfigPanelWrapper } from './config_panel_wrapper'; + +export interface EditorFrameProps { + datasources: { [key: string]: Datasource }; + visualizations: { [key: string]: Visualization }; + + initialDatasource: string | null; + initialVisualization: string | null; +} + +export function EditorFrame(props: EditorFrameProps) { + const [state, dispatch] = useReducer(reducer, props, getInitialState); + + // Initialize current datasource + useEffect( + () => { + let datasourceGotSwitched = false; + if (state.datasourceIsLoading && state.activeDatasource) { + props.datasources[state.activeDatasource].initialize().then(datasourceState => { + if (!datasourceGotSwitched) { + dispatch({ + type: 'UPDATE_DATASOURCE_STATE', + newState: datasourceState, + }); + } + }); + + return () => { + datasourceGotSwitched = true; + }; + } + }, + [state.activeDatasource, state.datasourceIsLoading] + ); + + const datasourcePublicAPI = useMemo( + () => + state.activeDatasource && !state.datasourceIsLoading + ? props.datasources[state.activeDatasource].getPublicAPI( + state.datasourceState, + (newState: unknown) => { + dispatch({ + type: 'UPDATE_DATASOURCE_STATE', + newState, + }); + } + ) + : undefined, + [props.datasources, state.datasourceIsLoading, state.activeDatasource, state.datasourceState] + ); + + return ( + + + + + + {state.activeDatasource && !state.datasourceIsLoading && ( + + )} + + + ); +} diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/index.ts b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/index.ts new file mode 100644 index 00000000000000..41558caafc64cf --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/index.ts @@ -0,0 +1,7 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +export * from './editor_frame'; diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx index 1b1957bf8d316d..34923e98ad770f 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx @@ -6,7 +6,7 @@ import React from 'react'; import { render, unmountComponentAtNode } from 'react-dom'; -import { Datasource, Visualization, EditorFrameSetup, DatasourcePublicAPI } from '../types'; +import { Datasource, Visualization, EditorFrameSetup } from '../types'; import { EditorFrame } from './editor_frame'; From 345e20f534524a70a78d4002184d4218bfae8537 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Fri, 10 May 2019 18:15:49 +0200 Subject: [PATCH 14/22] fix tests --- .../editor_frame/editor_frame.test.tsx | 157 ++++++++++-------- 1 file changed, 90 insertions(+), 67 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx index 1b2496b570d825..4b334a541c984b 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx @@ -8,6 +8,7 @@ import React from 'react'; import { mount, ReactWrapper } from 'enzyme'; import { EditorFrame } from './editor_frame'; import { Visualization, Datasource, DatasourcePublicAPI } from '../../types'; +import { act } from 'react-dom/test-utils'; const nextTick = () => new Promise(resolve => setTimeout(resolve)); @@ -47,54 +48,60 @@ describe('editor_frame', () => { describe('initialization', () => { it('should initialize initial datasource and visualization if present', () => { - mount( - - ); + act(() => { + mount( + + ); + }); expect(mockVisualization.initialize).toHaveBeenCalled(); expect(mockDatasource.initialize).toHaveBeenCalled(); }); it('should not initialize datasource and visualization if no initial one is specificed', () => { - mount( - - ); + act(() => { + mount( + + ); + }); expect(mockVisualization.initialize).not.toHaveBeenCalled(); expect(mockDatasource.initialize).not.toHaveBeenCalled(); }); it('should not render something before datasource is initialized', () => { - mount( - - ); + act(() => { + mount( + + ); + }); expect(mockVisualization.renderConfigPanel).not.toHaveBeenCalled(); expect(mockDatasource.renderDataPanel).not.toHaveBeenCalled(); @@ -104,24 +111,26 @@ describe('editor_frame', () => { const initialState = {}; let databaseInitialized: ({}) => void; - mount( - - new Promise(resolve => { - databaseInitialized = resolve; - }), - }, - }} - initialDatasource="testDatasource" - initialVisualization="testVis" - /> - ); + act(() => { + mount( + + new Promise(resolve => { + databaseInitialized = resolve; + }), + }, + }} + initialDatasource="testDatasource" + initialVisualization="testVis" + /> + ); + }); await nextTick(); databaseInitialized!(initialState); @@ -181,7 +190,9 @@ describe('editor_frame', () => { const updatedState = {}; const setVisualizationState = (mockVisualization.renderConfigPanel as jest.Mock).mock .calls[0][1].setState; - setVisualizationState(updatedState); + act(() => { + setVisualizationState(updatedState); + }); expect(mockVisualization.renderConfigPanel).toHaveBeenCalledTimes(2); expect(mockVisualization.renderConfigPanel).toHaveBeenLastCalledWith( @@ -214,7 +225,9 @@ describe('editor_frame', () => { const updatedState = {}; const setDatasourceState = (mockDatasource.renderDataPanel as jest.Mock).mock.calls[0][1] .setState; - setDatasourceState(updatedState); + act(() => { + setDatasourceState(updatedState); + }); expect(mockDatasource.renderDataPanel).toHaveBeenCalledTimes(2); expect(mockDatasource.renderDataPanel).toHaveBeenLastCalledWith( @@ -248,7 +261,9 @@ describe('editor_frame', () => { const setDatasourceState = (mockDatasource.renderDataPanel as jest.Mock).mock.calls[0][1] .setState; - setDatasourceState({}); + act(() => { + setDatasourceState({}); + }); expect(mockVisualization.renderConfigPanel).toHaveBeenCalledTimes(2); expect(mockVisualization.renderConfigPanel).toHaveBeenLastCalledWith( @@ -330,7 +345,9 @@ describe('editor_frame', () => { const updatedState = {}; const setDatasourceState = (mockDatasource.getPublicAPI as jest.Mock).mock.calls[0][1]; - setDatasourceState(updatedState); + act(() => { + setDatasourceState(updatedState); + }); expect(mockDatasource.getPublicAPI).toHaveBeenCalledTimes(2); expect(mockDatasource.getPublicAPI).toHaveBeenLastCalledWith( @@ -372,9 +389,11 @@ describe('editor_frame', () => { }); it('should initialize other datasource on switch', async () => { - instance - .find('select[data-test-subj="datasource-switch"]') - .simulate('change', { target: { value: 'testDatasource2' } }); + act(() => { + instance + .find('select[data-test-subj="datasource-switch"]') + .simulate('change', { target: { value: 'testDatasource2' } }); + }); expect(mockDatasource2.initialize).toHaveBeenCalled(); }); @@ -395,9 +414,11 @@ describe('editor_frame', () => { }); it('should initialize other visualization on switch', async () => { - instance - .find('select[data-test-subj="visualization-switch"]') - .simulate('change', { target: { value: 'testVis2' } }); + act(() => { + instance + .find('select[data-test-subj="visualization-switch"]') + .simulate('change', { target: { value: 'testVis2' } }); + }); expect(mockVisualization2.initialize).toHaveBeenCalled(); }); @@ -405,9 +426,11 @@ describe('editor_frame', () => { const initialState = {}; mockVisualization2.initialize = () => initialState; - instance - .find('select[data-test-subj="visualization-switch"]') - .simulate('change', { target: { value: 'testVis2' } }); + act(() => { + instance + .find('select[data-test-subj="visualization-switch"]') + .simulate('change', { target: { value: 'testVis2' } }); + }); expect(mockVisualization2.renderConfigPanel).toHaveBeenCalledWith( expect.any(Element), From 48203ddc3cf51627b0c8d1431ed23f540c192b9b Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Fri, 10 May 2019 18:22:52 +0200 Subject: [PATCH 15/22] move layout to a separate component --- .../editor_frame/editor_frame.tsx | 22 +++++++++--------- .../editor_frame/frame_layout.tsx | 23 +++++++++++++++++++ 2 files changed, 34 insertions(+), 11 deletions(-) create mode 100644 x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/frame_layout.tsx diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx index 3f92021f8c2ad1..03acbc69a69c45 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx @@ -5,12 +5,11 @@ */ import React, { useEffect, useReducer, useMemo } from 'react'; -import { EuiSelect, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; -import { Datasource, Visualization, DatasourceDataPanelProps } from '../../types'; -import { NativeRenderer } from '../../native_renderer'; +import { Datasource, Visualization } from '../../types'; import { reducer, getInitialState } from '../state_management'; import { DataPanelWrapper } from './data_panel_wrapper'; import { ConfigPanelWrapper } from './config_panel_wrapper'; +import { FrameLayout } from './frame_layout'; export interface EditorFrameProps { datasources: { [key: string]: Datasource }; @@ -62,8 +61,8 @@ export function EditorFrame(props: EditorFrameProps) { ); return ( - - + - - - {state.activeDatasource && !state.datasourceIsLoading && ( + } + configPanel={ + state.activeDatasource && + !state.datasourceIsLoading && ( - )} - - + ) + } + /> ); } diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/frame_layout.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/frame_layout.tsx new file mode 100644 index 00000000000000..182d0c9f0b5929 --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/frame_layout.tsx @@ -0,0 +1,23 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; + +export interface FrameLayoutProps { + dataPanel: React.ReactNode; + configPanel: React.ReactNode; +} + +export function FrameLayout(props: FrameLayoutProps) { + return ( + + {/* TODO style this and add workspace prop and loading flags */} + {props.dataPanel} + {props.configPanel} + + ); +} From ee6fdbcee6b72a6dffbab294dfca4b46680f6596 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Sun, 12 May 2019 13:11:04 +0200 Subject: [PATCH 16/22] clean up and improve typings --- .../editor_frame/editor_frame.tsx | 4 ++-- .../public/editor_frame_plugin/plugin.tsx | 24 +++++++------------ 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx index 03acbc69a69c45..09eb5a7fcb0c76 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx @@ -12,8 +12,8 @@ import { ConfigPanelWrapper } from './config_panel_wrapper'; import { FrameLayout } from './frame_layout'; export interface EditorFrameProps { - datasources: { [key: string]: Datasource }; - visualizations: { [key: string]: Visualization }; + datasources: Record; + visualizations: Record; initialDatasource: string | null; initialVisualization: string | null; diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx index 34923e98ad770f..8787eadd7dcccd 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx @@ -10,15 +10,11 @@ import { Datasource, Visualization, EditorFrameSetup } from '../types'; import { EditorFrame } from './editor_frame'; -class EditorFramePlugin { +export class EditorFramePlugin { constructor() {} - private datasources: { - [key: string]: Datasource; - } = {}; - private visualizations: { - [key: string]: Visualization; - } = {}; + private datasources: Record = {}; + private visualizations: Record = {}; public setup(): EditorFrameSetup { return { @@ -26,6 +22,9 @@ class EditorFramePlugin { let domElement: Element; return { mount: (element: Element) => { + if (domElement) { + unmountComponentAtNode(domElement); + } domElement = element; render( { - // casting it to an unknown datasource. This doesn't introduce runtime errors - // because each type T is always also an unknown, but typescript won't do it - // on it's own because we are loosing type information here. - // So it's basically explicitly saying "I'm dropping the information about type T here - // because this information isn't useful to me." but without using any which can leak - // const state = await datasource.initialize(); - - this.datasources[name] = datasource as Datasource; + registerDatasource: (name, datasource) => { + this.datasources[name] = datasource as Datasource; }, registerVisualization: (name, visualization) => { this.visualizations[name] = visualization as Visualization; From 396cf1c7bd217f1bb53be5f8f44652a75535aeff Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Sun, 12 May 2019 13:11:45 +0200 Subject: [PATCH 17/22] add tests for plugin itself --- .../editor_frame_plugin/plugin.test.tsx | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 x-pack/plugins/lens/public/editor_frame_plugin/plugin.test.tsx diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/plugin.test.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/plugin.test.tsx new file mode 100644 index 00000000000000..1ca641b2e6e371 --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_plugin/plugin.test.tsx @@ -0,0 +1,112 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { EditorFramePlugin } from './plugin'; +import { Visualization, Datasource } from '../types'; + +const nextTick = () => new Promise(resolve => setTimeout(resolve)); + +describe('editor_frame plugin', () => { + let pluginInstance: EditorFramePlugin; + let mountpoint: Element; + + beforeEach(() => { + pluginInstance = new EditorFramePlugin(); + mountpoint = document.createElement('div'); + }); + + afterEach(() => { + mountpoint.remove(); + }); + + it('should create an editor frame instance which mounts and unmounts', () => { + expect(() => { + const publicAPI = pluginInstance.setup(); + const instance = publicAPI.createInstance({}); + instance.mount(mountpoint); + instance.unmount(); + }).not.toThrowError(); + }); + + it('should render something in the provided dom element', () => { + const publicAPI = pluginInstance.setup(); + const instance = publicAPI.createInstance({}); + instance.mount(mountpoint); + + expect(mountpoint.hasChildNodes()).toBe(true); + + instance.unmount(); + }); + + it('should not have child nodes after unmount', () => { + const publicAPI = pluginInstance.setup(); + const instance = publicAPI.createInstance({}); + instance.mount(mountpoint); + instance.unmount(); + + expect(mountpoint.hasChildNodes()).toBe(false); + }); + + it('should initialize and render provided datasource', async () => { + const publicAPI = pluginInstance.setup(); + const mockDatasource = { + getDatasourceSuggestionsForField: jest.fn(), + getDatasourceSuggestionsFromCurrentState: jest.fn(), + getPersistableState: jest.fn(), + getPublicAPI: jest.fn(), + initialize: jest.fn(() => Promise.resolve()), + renderDataPanel: jest.fn(), + toExpression: jest.fn(), + }; + + publicAPI.registerDatasource('test', mockDatasource); + + const instance = publicAPI.createInstance({}); + instance.mount(mountpoint); + + await nextTick(); + + expect(mockDatasource.initialize).toHaveBeenCalled(); + expect(mockDatasource.renderDataPanel).toHaveBeenCalled(); + + instance.unmount(); + }); + + it('should initialize visualization and render config panel', async () => { + const publicAPI = pluginInstance.setup(); + const mockDatasource: Datasource = { + getDatasourceSuggestionsForField: jest.fn(), + getDatasourceSuggestionsFromCurrentState: jest.fn(), + getPersistableState: jest.fn(), + getPublicAPI: jest.fn(), + initialize: jest.fn(() => Promise.resolve()), + renderDataPanel: jest.fn(), + toExpression: jest.fn(), + }; + + const mockVisualization: Visualization = { + getMappingOfTableToRoles: jest.fn(), + getPersistableState: jest.fn(), + getSuggestions: jest.fn(), + initialize: jest.fn(), + renderConfigPanel: jest.fn(), + toExpression: jest.fn(), + }; + + publicAPI.registerDatasource('test', mockDatasource); + publicAPI.registerVisualization('test', mockVisualization); + + const instance = publicAPI.createInstance({}); + instance.mount(mountpoint); + + await nextTick(); + + expect(mockVisualization.initialize).toHaveBeenCalled(); + expect(mockVisualization.renderConfigPanel).toHaveBeenCalled(); + + instance.unmount(); + }); +}); From f05024b64142ad0538137305698b1e20f1030f47 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Mon, 20 May 2019 06:27:42 -0400 Subject: [PATCH 18/22] resolve conflicts --- .../public/indexpattern_plugin/indexpattern.tsx | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_plugin/indexpattern.tsx b/x-pack/plugins/lens/public/indexpattern_plugin/indexpattern.tsx index 53522ff39d6fb2..29b018320a6dbb 100644 --- a/x-pack/plugins/lens/public/indexpattern_plugin/indexpattern.tsx +++ b/x-pack/plugins/lens/public/indexpattern_plugin/indexpattern.tsx @@ -36,13 +36,6 @@ export interface Field { searchable: boolean; } -export interface Field { - name: string; - type: string; - aggregatable: boolean; - searchable: boolean; -} - export interface IndexPatternPersistedState { currentIndexPattern: string; @@ -56,16 +49,12 @@ export type IndexPatternPrivateState = IndexPatternPersistedState & { indexPatterns: { [id: string]: IndexPattern }; }; -type PersistedKeys = 'currentIndexPattern' | 'columns' | 'columnOrder'; - -export type PersistableIndexPatternPrivateState = Pick; - // Not stateful. State is persisted to the frame export const indexPatternDatasource: Datasource< IndexPatternPrivateState, - PersistableIndexPatternPrivateState + IndexPatternPersistedState > = { - async initialize(state?: PersistableIndexPatternPrivateState) { + async initialize(state?: IndexPatternPersistedState) { // TODO: Make fetch request to load indexPatterns from saved objects if (state) { return { From 3d055ee79b20431b8d1b61d960cc5a6ae003af86 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 21 May 2019 12:29:19 +0200 Subject: [PATCH 19/22] adress review and restructure state --- .../editor_frame/config_panel_wrapper.tsx | 22 +-- .../editor_frame/data_panel_wrapper.tsx | 8 +- .../editor_frame/editor_frame.test.tsx | 73 ++++---- .../editor_frame/editor_frame.tsx | 43 ++--- .../editor_frame/state_management.test.ts | 156 ++++++++++++++++++ .../editor_frame/state_management.ts | 119 +++++++++++++ .../public/editor_frame_plugin/plugin.tsx | 55 +++--- .../state_management.test.ts | 143 ---------------- .../editor_frame_plugin/state_management.ts | 98 ----------- 9 files changed, 380 insertions(+), 337 deletions(-) create mode 100644 x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts create mode 100644 x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts delete mode 100644 x-pack/plugins/lens/public/editor_frame_plugin/state_management.test.ts delete mode 100644 x-pack/plugins/lens/public/editor_frame_plugin/state_management.ts diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx index e494b8f81d7866..1a8c9bfe3b2983 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx @@ -7,13 +7,13 @@ import React, { useMemo } from 'react'; import { EuiSelect } from '@elastic/eui'; import { NativeRenderer } from '../../native_renderer'; -import { Action } from '../state_management'; +import { Action } from './state_management'; import { Visualization, DatasourcePublicAPI } from '../../types'; interface ConfigPanelWrapperProps { - visualizationState: Record; - visualizations: Record; - activeVisualization: string | null; + visualizationStateMap: Record; + visualizationMap: Record; + activeVisualizationId: string | null; dispatch: (action: Action) => void; datasourcePublicAPI: DatasourcePublicAPI; } @@ -33,26 +33,26 @@ export function ConfigPanelWrapper(props: ConfigPanelWrapperProps) { <> ({ + options={Object.keys(props.visualizationMap).map(visualizationId => ({ value: visualizationId, text: visualizationId, }))} - value={props.activeVisualization || undefined} + value={props.activeVisualizationId || undefined} onChange={e => { props.dispatch({ type: 'SWITCH_VISUALIZATION', - newVisulizationId: e.target.value, + newVisualizationId: e.target.value, // TODO we probably want to have a separate API to "force" a visualization switch // which isn't a result of a picked suggestion - initialState: props.visualizations[e.target.value].initialize(), + initialState: props.visualizationMap[e.target.value].initialize(), }); }} /> - {props.activeVisualization && ( + {props.activeVisualizationId && ( ; + datasourceMap: Record; activeDatasource: string | null; datasourceIsLoading: boolean; dispatch: (action: Action) => void; @@ -38,7 +38,7 @@ export function DataPanelWrapper(props: DataPanelWrapperProps) { <> ({ + options={Object.keys(props.datasourceMap).map(datasourceId => ({ value: datasourceId, text: datasourceId, }))} @@ -49,7 +49,7 @@ export function DataPanelWrapper(props: DataPanelWrapperProps) { /> {props.activeDatasource && !props.datasourceIsLoading && ( )} diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx index 4b334a541c984b..860d5bb1d8008b 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx @@ -10,7 +10,9 @@ import { EditorFrame } from './editor_frame'; import { Visualization, Datasource, DatasourcePublicAPI } from '../../types'; import { act } from 'react-dom/test-utils'; -const nextTick = () => new Promise(resolve => setTimeout(resolve)); +// calling this function will wait for all pending Promises from mock +// datasources to be processed by its callers. +const waitForPromises = () => new Promise(resolve => setImmediate(resolve)); describe('editor_frame', () => { const getMockVisualization = () => ({ @@ -51,10 +53,10 @@ describe('editor_frame', () => { act(() => { mount( { act(() => { mount( { act(() => { mount( { act(() => { mount( @@ -132,10 +134,9 @@ describe('editor_frame', () => { ); }); - await nextTick(); databaseInitialized!(initialState); - await nextTick(); + await waitForPromises(); expect(mockDatasource.renderDataPanel).toHaveBeenCalledWith( expect.any(Element), expect.objectContaining({ state: initialState }) @@ -147,10 +148,10 @@ describe('editor_frame', () => { mount( initialState }, }} - datasources={{ + datasourceMap={{ testDatasource: { ...mockDatasource, initialize: () => Promise.resolve(), @@ -161,7 +162,7 @@ describe('editor_frame', () => { /> ); - await nextTick(); + await waitForPromises(); expect(mockVisualization.renderConfigPanel).toHaveBeenCalledWith( expect.any(Element), @@ -174,10 +175,10 @@ describe('editor_frame', () => { it('should re-render config panel after state update', async () => { mount( { /> ); - await nextTick(); + await waitForPromises(); const updatedState = {}; const setVisualizationState = (mockVisualization.renderConfigPanel as jest.Mock).mock @@ -209,10 +210,10 @@ describe('editor_frame', () => { it('should re-render data panel after state update', async () => { mount( { /> ); - await nextTick(); + await waitForPromises(); const updatedState = {}; const setDatasourceState = (mockDatasource.renderDataPanel as jest.Mock).mock.calls[0][1] @@ -241,10 +242,10 @@ describe('editor_frame', () => { it('should re-render config panel with updated datasource api after datasource state update', async () => { mount( { /> ); - await nextTick(); + await waitForPromises(); const updatedPublicAPI = {}; mockDatasource.getPublicAPI = jest.fn( @@ -283,10 +284,10 @@ describe('editor_frame', () => { mount( { /> ); - await nextTick(); + await waitForPromises(); expect(mockVisualization.renderConfigPanel).toHaveBeenCalledWith( expect.any(Element), @@ -308,10 +309,10 @@ describe('editor_frame', () => { mount( { /> ); - await nextTick(); + await waitForPromises(); expect(mockDatasource.getPublicAPI).toHaveBeenCalledWith( datasourceState, @@ -330,10 +331,10 @@ describe('editor_frame', () => { it('should re-create the public api after state has been set', async () => { mount( { /> ); - await nextTick(); + await waitForPromises(); const updatedState = {}; const setDatasourceState = (mockDatasource.getPublicAPI as jest.Mock).mock.calls[0][1]; @@ -362,11 +363,11 @@ describe('editor_frame', () => { beforeEach(async () => { instance = mount( { initialVisualization="testVis" /> ); - await nextTick(); + await waitForPromises(); // necessary to flush elements to dom synchronously instance.update(); @@ -405,7 +406,7 @@ describe('editor_frame', () => { .find('select[data-test-subj="datasource-switch"]') .simulate('change', { target: { value: 'testDatasource2' } }); - await nextTick(); + await waitForPromises(); expect(mockDatasource2.renderDataPanel).toHaveBeenCalledWith( expect.any(Element), diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx index 09eb5a7fcb0c76..c6e70ea22425fa 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx @@ -6,14 +6,14 @@ import React, { useEffect, useReducer, useMemo } from 'react'; import { Datasource, Visualization } from '../../types'; -import { reducer, getInitialState } from '../state_management'; +import { reducer, getInitialState } from './state_management'; import { DataPanelWrapper } from './data_panel_wrapper'; import { ConfigPanelWrapper } from './config_panel_wrapper'; import { FrameLayout } from './frame_layout'; export interface EditorFrameProps { - datasources: Record; - visualizations: Record; + datasourceMap: Record; + visualizationMap: Record; initialDatasource: string | null; initialVisualization: string | null; @@ -26,8 +26,8 @@ export function EditorFrame(props: EditorFrameProps) { useEffect( () => { let datasourceGotSwitched = false; - if (state.datasourceIsLoading && state.activeDatasource) { - props.datasources[state.activeDatasource].initialize().then(datasourceState => { + if (state.datasource.isLoading && state.datasource.activeId) { + props.datasourceMap[state.datasource.activeId].initialize().then(datasourceState => { if (!datasourceGotSwitched) { dispatch({ type: 'UPDATE_DATASOURCE_STATE', @@ -41,14 +41,14 @@ export function EditorFrame(props: EditorFrameProps) { }; } }, - [state.activeDatasource, state.datasourceIsLoading] + [state.datasource.activeId, state.datasource.isLoading] ); const datasourcePublicAPI = useMemo( () => - state.activeDatasource && !state.datasourceIsLoading - ? props.datasources[state.activeDatasource].getPublicAPI( - state.datasourceState, + state.datasource.activeId && !state.datasource.isLoading + ? props.datasourceMap[state.datasource.activeId].getPublicAPI( + state.datasource.state, (newState: unknown) => { dispatch({ type: 'UPDATE_DATASOURCE_STATE', @@ -57,29 +57,34 @@ export function EditorFrame(props: EditorFrameProps) { } ) : undefined, - [props.datasources, state.datasourceIsLoading, state.activeDatasource, state.datasourceState] + [ + props.datasourceMap, + state.datasource.isLoading, + state.datasource.activeId, + state.datasource.state, + ] ); return ( } configPanel={ - state.activeDatasource && - !state.datasourceIsLoading && ( + state.datasource.activeId && + !state.datasource.isLoading && ( ) } diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts new file mode 100644 index 00000000000000..c610a74368a013 --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts @@ -0,0 +1,156 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { getInitialState, reducer } from './state_management'; +import { EditorFrameProps } from '.'; +import { Datasource, Visualization } from '../../types'; + +describe('editor_frame state management', () => { + describe('initialization', () => { + let props: EditorFrameProps; + + beforeEach(() => { + props = { + datasourceMap: { testDatasource: ({} as unknown) as Datasource }, + visualizationMap: { testVis: ({ initialize: jest.fn() } as unknown) as Visualization }, + initialDatasource: 'testDatasource', + initialVisualization: 'testVis', + }; + }); + + it('should store initial datasource and visualization', () => { + const initialState = getInitialState(props); + expect(initialState.datasource.activeId).toEqual('testDatasource'); + expect(initialState.visualization.activeId).toEqual('testVis'); + }); + + it('should initialize visualization', () => { + const initialVisState = {}; + props.visualizationMap.testVis.initialize = jest.fn(() => initialVisState); + + const initialState = getInitialState(props); + + expect(initialState.visualization.stateMap.testVis).toBe(initialVisState); + expect(props.visualizationMap.testVis.initialize).toHaveBeenCalled(); + }); + + it('should not initialize visualization if no initial visualization is passed in', () => { + const initialState = getInitialState({ ...props, initialVisualization: null }); + + expect(initialState.visualization.stateMap).toEqual({}); + expect(props.visualizationMap.testVis.initialize).not.toHaveBeenCalled(); + }); + }); + + describe('state update', () => { + it('should update the corresponding visualization state on update', () => { + const newVisState = {}; + const newState = reducer( + { + datasource: { + activeId: 'testDatasource', + state: {}, + isLoading: false, + }, + visualization: { + activeId: 'testVis', + stateMap: { + testVis: {}, + }, + }, + }, + { + type: 'UPDATE_VISUALIZATION_STATE', + newState: newVisState, + } + ); + + expect(newState.visualization.stateMap).toEqual({ + testVis: newVisState, + }); + }); + + it('should update the datasource state on update', () => { + const newDatasourceState = {}; + const newState = reducer( + { + datasource: { + activeId: 'testDatasource', + state: {}, + isLoading: false, + }, + visualization: { + activeId: 'testVis', + stateMap: { + testVis: {}, + }, + }, + }, + { + type: 'UPDATE_DATASOURCE_STATE', + newState: newDatasourceState, + } + ); + + expect(newState.datasource.state).toBe(newDatasourceState); + }); + + it('should should switch active visualization but dont loose old state', () => { + const testVisState = {}; + const newVisState = {}; + const newState = reducer( + { + datasource: { + activeId: 'testDatasource', + state: {}, + isLoading: false, + }, + visualization: { + activeId: 'testVis', + stateMap: { + testVis: testVisState, + }, + }, + }, + { + type: 'SWITCH_VISUALIZATION', + newVisualizationId: 'testVis2', + initialState: newVisState, + } + ); + + expect(newState.visualization.stateMap.testVis).toBe(testVisState); + expect(newState.visualization.stateMap.testVis2).toBe(newVisState); + }); + + it('should should switch active datasource and purge visualization state', () => { + const newState = reducer( + { + datasource: { + activeId: 'testDatasource', + state: {}, + isLoading: false, + }, + visualization: { + activeId: 'testVis', + stateMap: { + testVis: {}, + }, + }, + }, + { + type: 'SWITCH_DATASOURCE', + newDatasourceId: 'testDatasource2', + } + ); + + expect(newState.visualization.stateMap).toEqual({}); + expect(newState.visualization.activeId).toBe(null); + expect(newState.datasource.activeId).toBe('testDatasource2'); + expect(newState.datasource.state).toBe(null); + }); + }); +}); diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts new file mode 100644 index 00000000000000..444551374389fc --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts @@ -0,0 +1,119 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { EditorFrameProps } from '.'; + +export interface EditorFrameState { + visualization: { + activeId: string | null; + stateMap: { + [visualizationId: string]: unknown; + }; + }; + datasource: { + activeId: string | null; + state: unknown; + isLoading: boolean; + }; +} + +export type Action = + | { + type: 'UPDATE_DATASOURCE_STATE'; + newState: unknown; + } + | { + type: 'UPDATE_VISUALIZATION_STATE'; + newState: unknown; + } + | { + type: 'SWITCH_VISUALIZATION'; + newVisualizationId: string; + initialState: unknown; + } + | { + type: 'SWITCH_DATASOURCE'; + newDatasourceId: string; + }; + +export const getInitialState = (props: EditorFrameProps): EditorFrameState => { + return { + datasource: { + state: null, + isLoading: Boolean(props.initialDatasource), + activeId: props.initialDatasource, + }, + visualization: { + stateMap: props.initialVisualization + ? { + [props.initialVisualization]: props.visualizationMap[ + props.initialVisualization + ].initialize(), + } + : {}, + activeId: props.initialVisualization, + }, + }; +}; + +export const reducer = (state: EditorFrameState, action: Action): EditorFrameState => { + switch (action.type) { + case 'SWITCH_DATASOURCE': + return { + ...state, + datasource: { + ...state.datasource, + isLoading: true, + state: null, + activeId: action.newDatasourceId, + }, + visualization: { + ...state.visualization, + // purge all visualizations on datasource switch + stateMap: {}, + activeId: null, + }, + }; + case 'SWITCH_VISUALIZATION': + return { + ...state, + visualization: { + ...state.visualization, + activeId: action.newVisualizationId, + stateMap: { + ...state.visualization.stateMap, + [action.newVisualizationId]: action.initialState, + }, + }, + }; + case 'UPDATE_DATASOURCE_STATE': + return { + ...state, + datasource: { + ...state.datasource, + // when the datasource state is updated, the initialization is complete + isLoading: false, + state: action.newState, + }, + }; + case 'UPDATE_VISUALIZATION_STATE': + if (!state.visualization.activeId) { + throw new Error('Invariant: visualization state got updated without active visualization'); + } + return { + ...state, + visualization: { + ...state.visualization, + stateMap: { + ...state.visualization.stateMap, + [state.visualization.activeId]: action.newState, + }, + }, + }; + default: + return state; + } +}; diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx index 8787eadd7dcccd..d358e46dbe0f9e 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx @@ -6,7 +6,7 @@ import React from 'react'; import { render, unmountComponentAtNode } from 'react-dom'; -import { Datasource, Visualization, EditorFrameSetup } from '../types'; +import { Datasource, Visualization, EditorFrameSetup, EditorFrameInstance } from '../types'; import { EditorFrame } from './editor_frame'; @@ -16,33 +16,36 @@ export class EditorFramePlugin { private datasources: Record = {}; private visualizations: Record = {}; - public setup(): EditorFrameSetup { + private createInstance(): EditorFrameInstance { + let domElement: Element; + + function unmount() { + if (domElement) { + unmountComponentAtNode(domElement); + } + } + return { - createInstance: () => { - let domElement: Element; - return { - mount: (element: Element) => { - if (domElement) { - unmountComponentAtNode(domElement); - } - domElement = element; - render( - , - domElement - ); - }, - unmount: () => { - if (domElement) { - unmountComponentAtNode(domElement); - } - }, - }; + mount: element => { + unmount(); + domElement = element; + render( + , + domElement + ); }, + unmount, + }; + } + + public setup(): EditorFrameSetup { + return { + createInstance: this.createInstance.bind(this), registerDatasource: (name, datasource) => { this.datasources[name] = datasource as Datasource; }, diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/state_management.test.ts b/x-pack/plugins/lens/public/editor_frame_plugin/state_management.test.ts deleted file mode 100644 index fb96817685c45a..00000000000000 --- a/x-pack/plugins/lens/public/editor_frame_plugin/state_management.test.ts +++ /dev/null @@ -1,143 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import { getInitialState, reducer } from './state_management'; -import { EditorFrameProps } from './editor_frame'; -import { Datasource, Visualization } from '../types'; - -describe('editor_frame state management', () => { - describe('initialization', () => { - let props: EditorFrameProps; - - beforeEach(() => { - props = { - datasources: { testDatasource: ({} as unknown) as Datasource }, - visualizations: { testVis: ({ initialize: jest.fn() } as unknown) as Visualization }, - initialDatasource: 'testDatasource', - initialVisualization: 'testVis', - }; - }); - - it('should store initial datasource and visualization', () => { - expect(getInitialState(props)).toEqual( - expect.objectContaining({ - activeDatasource: 'testDatasource', - activeVisualization: 'testVis', - }) - ); - }); - - it('should initialize visualization', () => { - const initialVisState = {}; - props.visualizations.testVis.initialize = jest.fn(() => initialVisState); - - const initialState = getInitialState(props); - - expect(initialState.visualizationState.testVis).toBe(initialVisState); - expect(props.visualizations.testVis.initialize).toHaveBeenCalled(); - }); - - it('should not initialize visualization if no initial visualization is passed in', () => { - const initialState = getInitialState({ ...props, initialVisualization: null }); - - expect(initialState.visualizationState).toEqual({}); - expect(props.visualizations.testVis.initialize).not.toHaveBeenCalled(); - }); - }); - - describe('state update', () => { - it('should update the corresponding visualization state on update', () => { - const newVisState = {}; - const newState = reducer( - { - activeDatasource: 'testDatasource', - activeVisualization: 'testVis', - datasourceIsLoading: false, - datasourceState: {}, - visualizationState: { - testVis: {}, - }, - }, - { - type: 'UPDATE_VISUALIZATION_STATE', - newState: newVisState, - } - ); - - expect(newState.visualizationState).toEqual({ - testVis: newVisState, - }); - }); - - it('should update the datasource state on update', () => { - const newDatasourceState = {}; - const newState = reducer( - { - activeDatasource: 'testDatasource', - activeVisualization: 'testVis', - datasourceIsLoading: false, - datasourceState: {}, - visualizationState: { - testVis: {}, - }, - }, - { - type: 'UPDATE_DATASOURCE_STATE', - newState: newDatasourceState, - } - ); - - expect(newState.datasourceState).toBe(newDatasourceState); - }); - - it('should should switch active visualization but dont loose old state', () => { - const testVisState = {}; - const newVisState = {}; - const newState = reducer( - { - activeDatasource: 'testDatasource', - activeVisualization: 'testVis', - datasourceIsLoading: false, - datasourceState: {}, - visualizationState: { - testVis: testVisState, - }, - }, - { - type: 'SWITCH_VISUALIZATION', - newVisulizationId: 'testVis2', - initialState: newVisState, - } - ); - - expect(newState.visualizationState.testVis).toBe(testVisState); - expect(newState.visualizationState.testVis2).toBe(newVisState); - }); - - it('should should switch active datasource and purge visualization state', () => { - const newState = reducer( - { - activeDatasource: 'testDatasource', - activeVisualization: 'testVis', - datasourceIsLoading: false, - datasourceState: {}, - visualizationState: { - testVis: {}, - }, - }, - { - type: 'SWITCH_DATASOURCE', - newDatasourceId: 'testDatasource2', - } - ); - - expect(newState.visualizationState).toEqual({}); - expect(newState.activeVisualization).toBe(null); - expect(newState.activeDatasource).toBe('testDatasource2'); - expect(newState.datasourceState).toBe(null); - }); - }); -}); diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/state_management.ts b/x-pack/plugins/lens/public/editor_frame_plugin/state_management.ts deleted file mode 100644 index e35e258d1153cd..00000000000000 --- a/x-pack/plugins/lens/public/editor_frame_plugin/state_management.ts +++ /dev/null @@ -1,98 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import { EditorFrameProps } from './editor_frame'; - -export interface EditorFrameState { - activeDatasource: string | null; - datasourceState: unknown; - datasourceIsLoading: boolean; - - activeVisualization: string | null; - visualizationState: { - [visualizationId: string]: unknown; - }; -} - -export type Action = - | { - type: 'UPDATE_DATASOURCE_STATE'; - newState: unknown; - } - | { - type: 'UPDATE_VISUALIZATION_STATE'; - newState: unknown; - } - | { - type: 'SWITCH_VISUALIZATION'; - newVisulizationId: string; - initialState: unknown; - } - | { - type: 'SWITCH_DATASOURCE'; - newDatasourceId: string; - }; - -export const getInitialState = (props: EditorFrameProps) => { - return { - datasourceState: null, - datasourceIsLoading: Boolean(props.initialDatasource), - activeDatasource: props.initialDatasource, - visualizationState: props.initialVisualization - ? { - [props.initialVisualization]: props.visualizations[ - props.initialVisualization - ].initialize(), - } - : {}, - activeVisualization: props.initialVisualization, - }; -}; - -export const reducer = (state: EditorFrameState, action: Action): EditorFrameState => { - switch (action.type) { - case 'SWITCH_DATASOURCE': - return { - ...state, - activeDatasource: action.newDatasourceId, - // purge all visualizations on datasource switch - visualizationState: {}, - activeVisualization: null, - datasourceState: null, - datasourceIsLoading: true, - }; - case 'SWITCH_VISUALIZATION': - return { - ...state, - activeVisualization: action.newVisulizationId, - visualizationState: { - ...state.visualizationState, - [action.newVisulizationId]: action.initialState, - }, - }; - case 'UPDATE_DATASOURCE_STATE': - return { - ...state, - // when the datasource state is updated, the initialization is complete - datasourceIsLoading: false, - datasourceState: action.newState, - }; - case 'UPDATE_VISUALIZATION_STATE': - if (state.activeVisualization) { - return { - ...state, - visualizationState: { - ...state.visualizationState, - [state.activeVisualization]: action.newState, - }, - }; - } else { - throw new Error('Invariant: visualization state got updated without active visualization'); - } - default: - return state; - } -}; From 7c3a2c849dfb11fc6a40858935d76e81a22893f2 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 21 May 2019 12:34:52 +0200 Subject: [PATCH 20/22] align naming --- .../editor_frame/editor_frame.test.tsx | 48 +++++++++---------- .../editor_frame/editor_frame.tsx | 4 +- .../editor_frame/state_management.test.ts | 6 +-- .../editor_frame/state_management.ts | 12 ++--- .../public/editor_frame_plugin/plugin.tsx | 4 +- 5 files changed, 37 insertions(+), 37 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx index 860d5bb1d8008b..32785d2d95753b 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx @@ -59,8 +59,8 @@ describe('editor_frame', () => { datasourceMap={{ testDatasource: mockDatasource, }} - initialDatasource="testDatasource" - initialVisualization="testVis" + initialDatasourceId="testDatasource" + initialVisualizationId="testVis" /> ); }); @@ -79,8 +79,8 @@ describe('editor_frame', () => { datasourceMap={{ testDatasource: mockDatasource, }} - initialDatasource={null} - initialVisualization={null} + initialDatasourceId={null} + initialVisualizationId={null} /> ); }); @@ -99,8 +99,8 @@ describe('editor_frame', () => { datasourceMap={{ testDatasource: mockDatasource, }} - initialDatasource="testDatasource" - initialVisualization="testVis" + initialDatasourceId="testDatasource" + initialVisualizationId="testVis" /> ); }); @@ -128,8 +128,8 @@ describe('editor_frame', () => { }), }, }} - initialDatasource="testDatasource" - initialVisualization="testVis" + initialDatasourceId="testDatasource" + initialVisualizationId="testVis" /> ); }); @@ -157,8 +157,8 @@ describe('editor_frame', () => { initialize: () => Promise.resolve(), }, }} - initialDatasource="testDatasource" - initialVisualization="testVis" + initialDatasourceId="testDatasource" + initialVisualizationId="testVis" /> ); @@ -181,8 +181,8 @@ describe('editor_frame', () => { datasourceMap={{ testDatasource: mockDatasource, }} - initialDatasource="testDatasource" - initialVisualization="testVis" + initialDatasourceId="testDatasource" + initialVisualizationId="testVis" /> ); @@ -216,8 +216,8 @@ describe('editor_frame', () => { datasourceMap={{ testDatasource: mockDatasource, }} - initialDatasource="testDatasource" - initialVisualization="testVis" + initialDatasourceId="testDatasource" + initialVisualizationId="testVis" /> ); @@ -248,8 +248,8 @@ describe('editor_frame', () => { datasourceMap={{ testDatasource: mockDatasource, }} - initialDatasource="testDatasource" - initialVisualization="testVis" + initialDatasourceId="testDatasource" + initialVisualizationId="testVis" /> ); @@ -290,8 +290,8 @@ describe('editor_frame', () => { datasourceMap={{ testDatasource: mockDatasource, }} - initialDatasource="testDatasource" - initialVisualization="testVis" + initialDatasourceId="testDatasource" + initialVisualizationId="testVis" /> ); @@ -315,8 +315,8 @@ describe('editor_frame', () => { datasourceMap={{ testDatasource: mockDatasource, }} - initialDatasource="testDatasource" - initialVisualization="testVis" + initialDatasourceId="testDatasource" + initialVisualizationId="testVis" /> ); @@ -337,8 +337,8 @@ describe('editor_frame', () => { datasourceMap={{ testDatasource: mockDatasource, }} - initialDatasource="testDatasource" - initialVisualization="testVis" + initialDatasourceId="testDatasource" + initialVisualizationId="testVis" /> ); @@ -371,8 +371,8 @@ describe('editor_frame', () => { testDatasource: mockDatasource, testDatasource2: mockDatasource2, }} - initialDatasource="testDatasource" - initialVisualization="testVis" + initialDatasourceId="testDatasource" + initialVisualizationId="testVis" /> ); await waitForPromises(); diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx index c6e70ea22425fa..069320ba55d841 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx @@ -15,8 +15,8 @@ export interface EditorFrameProps { datasourceMap: Record; visualizationMap: Record; - initialDatasource: string | null; - initialVisualization: string | null; + initialDatasourceId: string | null; + initialVisualizationId: string | null; } export function EditorFrame(props: EditorFrameProps) { diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts index c610a74368a013..ea40f4e203eb01 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts @@ -16,8 +16,8 @@ describe('editor_frame state management', () => { props = { datasourceMap: { testDatasource: ({} as unknown) as Datasource }, visualizationMap: { testVis: ({ initialize: jest.fn() } as unknown) as Visualization }, - initialDatasource: 'testDatasource', - initialVisualization: 'testVis', + initialDatasourceId: 'testDatasource', + initialVisualizationId: 'testVis', }; }); @@ -38,7 +38,7 @@ describe('editor_frame state management', () => { }); it('should not initialize visualization if no initial visualization is passed in', () => { - const initialState = getInitialState({ ...props, initialVisualization: null }); + const initialState = getInitialState({ ...props, initialVisualizationId: null }); expect(initialState.visualization.stateMap).toEqual({}); expect(props.visualizationMap.testVis.initialize).not.toHaveBeenCalled(); diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts index 444551374389fc..e8b8dac09d5d3b 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts @@ -43,18 +43,18 @@ export const getInitialState = (props: EditorFrameProps): EditorFrameState => { return { datasource: { state: null, - isLoading: Boolean(props.initialDatasource), - activeId: props.initialDatasource, + isLoading: Boolean(props.initialDatasourceId), + activeId: props.initialDatasourceId, }, visualization: { - stateMap: props.initialVisualization + stateMap: props.initialVisualizationId ? { - [props.initialVisualization]: props.visualizationMap[ - props.initialVisualization + [props.initialVisualizationId]: props.visualizationMap[ + props.initialVisualizationId ].initialize(), } : {}, - activeId: props.initialVisualization, + activeId: props.initialVisualizationId, }, }; }; diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx index d358e46dbe0f9e..07c18416011403 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/plugin.tsx @@ -33,8 +33,8 @@ export class EditorFramePlugin { , domElement ); From 849f3a11dc50acda1c44912160b3c65db8cf13be Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 21 May 2019 12:37:53 +0200 Subject: [PATCH 21/22] add comment --- .../public/editor_frame_plugin/editor_frame/editor_frame.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx index 069320ba55d841..159762939a58b5 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx @@ -44,6 +44,8 @@ export function EditorFrame(props: EditorFrameProps) { [state.datasource.activeId, state.datasource.isLoading] ); + // create public datasource api for current state + // as soon as datasource is available and memoize it const datasourcePublicAPI = useMemo( () => state.datasource.activeId && !state.datasource.isLoading From 21c8756f5ae20e251ea30fee19c3ff8eccb1d83e Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 21 May 2019 16:39:37 +0200 Subject: [PATCH 22/22] only save single visualization state --- .../editor_frame/config_panel_wrapper.tsx | 4 +-- .../editor_frame/editor_frame.tsx | 2 +- .../editor_frame/state_management.test.ts | 31 ++++++------------- .../editor_frame/state_management.ts | 28 +++++------------ 4 files changed, 21 insertions(+), 44 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx index 1a8c9bfe3b2983..b1329ee6fc2a85 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/config_panel_wrapper.tsx @@ -11,7 +11,7 @@ import { Action } from './state_management'; import { Visualization, DatasourcePublicAPI } from '../../types'; interface ConfigPanelWrapperProps { - visualizationStateMap: Record; + visualizationState: unknown; visualizationMap: Record; activeVisualizationId: string | null; dispatch: (action: Action) => void; @@ -52,7 +52,7 @@ export function ConfigPanelWrapper(props: ConfigPanelWrapperProps) { ) } diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts index ea40f4e203eb01..373b321309586a 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts @@ -33,14 +33,14 @@ describe('editor_frame state management', () => { const initialState = getInitialState(props); - expect(initialState.visualization.stateMap.testVis).toBe(initialVisState); + expect(initialState.visualization.state).toBe(initialVisState); expect(props.visualizationMap.testVis.initialize).toHaveBeenCalled(); }); it('should not initialize visualization if no initial visualization is passed in', () => { const initialState = getInitialState({ ...props, initialVisualizationId: null }); - expect(initialState.visualization.stateMap).toEqual({}); + expect(initialState.visualization.state).toEqual(null); expect(props.visualizationMap.testVis.initialize).not.toHaveBeenCalled(); }); }); @@ -57,9 +57,7 @@ describe('editor_frame state management', () => { }, visualization: { activeId: 'testVis', - stateMap: { - testVis: {}, - }, + state: {}, }, }, { @@ -68,9 +66,7 @@ describe('editor_frame state management', () => { } ); - expect(newState.visualization.stateMap).toEqual({ - testVis: newVisState, - }); + expect(newState.visualization.state).toBe(newVisState); }); it('should update the datasource state on update', () => { @@ -84,9 +80,7 @@ describe('editor_frame state management', () => { }, visualization: { activeId: 'testVis', - stateMap: { - testVis: {}, - }, + state: {}, }, }, { @@ -98,7 +92,7 @@ describe('editor_frame state management', () => { expect(newState.datasource.state).toBe(newDatasourceState); }); - it('should should switch active visualization but dont loose old state', () => { + it('should should switch active visualization', () => { const testVisState = {}; const newVisState = {}; const newState = reducer( @@ -110,9 +104,7 @@ describe('editor_frame state management', () => { }, visualization: { activeId: 'testVis', - stateMap: { - testVis: testVisState, - }, + state: testVisState, }, }, { @@ -122,8 +114,7 @@ describe('editor_frame state management', () => { } ); - expect(newState.visualization.stateMap.testVis).toBe(testVisState); - expect(newState.visualization.stateMap.testVis2).toBe(newVisState); + expect(newState.visualization.state).toBe(newVisState); }); it('should should switch active datasource and purge visualization state', () => { @@ -136,9 +127,7 @@ describe('editor_frame state management', () => { }, visualization: { activeId: 'testVis', - stateMap: { - testVis: {}, - }, + state: {}, }, }, { @@ -147,7 +136,7 @@ describe('editor_frame state management', () => { } ); - expect(newState.visualization.stateMap).toEqual({}); + expect(newState.visualization.state).toEqual(null); expect(newState.visualization.activeId).toBe(null); expect(newState.datasource.activeId).toBe('testDatasource2'); expect(newState.datasource.state).toBe(null); diff --git a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts index e8b8dac09d5d3b..2358da104378b5 100644 --- a/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts +++ b/x-pack/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts @@ -9,9 +9,7 @@ import { EditorFrameProps } from '.'; export interface EditorFrameState { visualization: { activeId: string | null; - stateMap: { - [visualizationId: string]: unknown; - }; + state: unknown; }; datasource: { activeId: string | null; @@ -47,13 +45,9 @@ export const getInitialState = (props: EditorFrameProps): EditorFrameState => { activeId: props.initialDatasourceId, }, visualization: { - stateMap: props.initialVisualizationId - ? { - [props.initialVisualizationId]: props.visualizationMap[ - props.initialVisualizationId - ].initialize(), - } - : {}, + state: props.initialVisualizationId + ? props.visualizationMap[props.initialVisualizationId].initialize() + : null, activeId: props.initialVisualizationId, }, }; @@ -72,8 +66,8 @@ export const reducer = (state: EditorFrameState, action: Action): EditorFrameSta }, visualization: { ...state.visualization, - // purge all visualizations on datasource switch - stateMap: {}, + // purge visualization on datasource switch + state: null, activeId: null, }, }; @@ -83,10 +77,7 @@ export const reducer = (state: EditorFrameState, action: Action): EditorFrameSta visualization: { ...state.visualization, activeId: action.newVisualizationId, - stateMap: { - ...state.visualization.stateMap, - [action.newVisualizationId]: action.initialState, - }, + state: action.initialState, }, }; case 'UPDATE_DATASOURCE_STATE': @@ -107,10 +98,7 @@ export const reducer = (state: EditorFrameState, action: Action): EditorFrameSta ...state, visualization: { ...state.visualization, - stateMap: { - ...state.visualization.stateMap, - [state.visualization.activeId]: action.newState, - }, + state: action.newState, }, }; default: