From c17b13d0e180e216679945fc7d191aa78c878a55 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Mon, 17 Jun 2019 17:35:44 -0700 Subject: [PATCH] feat: add error boundary and responsiveness to SuperChart (#175) * feat: add fallback component * feat: add superchart shell * feat: add vx/responsive type declaration * fix: path and dependencies * test: add unit tests * test: add more tests * docs: add storybook * test: fix FallBackComponent test * feat: make fallback accepts width and height * test: reach 100% * fix: test * fix: add more checks * refactor: rename SuperChartKernel to SuperChartCore * refactor: separate backward-compatibility code into another wrapper --- package.json | 2 +- .../__mocks__/resize-observer-polyfill.js | 22 ++ packages/superset-ui-chart/package.json | 3 + .../src/components/FallbackComponent.tsx | 34 +++ .../src/components/SuperChart.tsx | 272 ++++++------------ .../src/components/SuperChartCore.tsx | 209 ++++++++++++++ .../src/components/SuperChartShell.tsx | 62 ++++ packages/superset-ui-chart/src/index.ts | 2 +- .../src/models/ChartProps.ts | 2 +- .../components/FallbackComponent.test.tsx | 38 +++ .../test/components/MockChartPlugins.tsx | 57 +++- .../test/components/SuperChart.test.tsx | 162 ----------- .../test/components/SuperChartCore.test.tsx | 179 ++++++++++++ .../test/components/SuperChartShell.test.tsx | 228 +++++++++++++++ .../test/components/promiseTimeout.tsx | 13 + .../types/@vx/responsive/index.d.ts | 70 +++++ .../superset-ui-demo/.storybook/config.js | 1 + .../.storybook/webpack.config.js | 5 + .../superset-ui-chart/SuperChartStories.tsx | 89 ++++++ .../stories/superset-ui-chart/index.ts | 3 +- 20 files changed, 1096 insertions(+), 357 deletions(-) create mode 100644 packages/superset-ui-chart/__mocks__/resize-observer-polyfill.js create mode 100644 packages/superset-ui-chart/src/components/FallbackComponent.tsx create mode 100644 packages/superset-ui-chart/src/components/SuperChartCore.tsx create mode 100644 packages/superset-ui-chart/src/components/SuperChartShell.tsx create mode 100644 packages/superset-ui-chart/test/components/FallbackComponent.test.tsx delete mode 100644 packages/superset-ui-chart/test/components/SuperChart.test.tsx create mode 100644 packages/superset-ui-chart/test/components/SuperChartCore.test.tsx create mode 100644 packages/superset-ui-chart/test/components/SuperChartShell.test.tsx create mode 100644 packages/superset-ui-chart/test/components/promiseTimeout.tsx create mode 100644 packages/superset-ui-chart/types/@vx/responsive/index.d.ts create mode 100644 packages/superset-ui-demo/storybook/stories/superset-ui-chart/SuperChartStories.tsx diff --git a/package.json b/package.json index 6e23b2f810..b6a7172c28 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "pretest": "yarn run lint", "prettier": "beemo prettier \"./packages/*/{src,test,storybook}/**/*.{js,jsx,ts,tsx,json,md}\"", "release": "yarn run prepare-release && lerna publish && yarn run postrelease", - "test": "yarn run type && yarn run jest", + "test": "yarn run jest", "test:watch": "yarn run lint:fix && beemo create-config jest --react && jest --watch" }, "repository": "https://github.com/apache-superset/superset-ui.git", diff --git a/packages/superset-ui-chart/__mocks__/resize-observer-polyfill.js b/packages/superset-ui-chart/__mocks__/resize-observer-polyfill.js new file mode 100644 index 0000000000..7d45aeb99d --- /dev/null +++ b/packages/superset-ui-chart/__mocks__/resize-observer-polyfill.js @@ -0,0 +1,22 @@ +const allCallbacks = []; + +export default function ResizeObserver(callback) { + if (callback) { + allCallbacks.push(callback); + } + + return { + disconnect: () => { + allCallbacks.splice(allCallbacks.findIndex(callback), 1); + }, + observe: () => {}, + }; +} + +const DEFAULT_OUTPUT = [{ contentRect: { height: 300, width: 300 } }]; + +export function triggerResizeObserver(output = DEFAULT_OUTPUT) { + allCallbacks.forEach(fn => { + fn(output); + }); +} diff --git a/packages/superset-ui-chart/package.json b/packages/superset-ui-chart/package.json index 4fd4ad3975..c4174c2c99 100644 --- a/packages/superset-ui-chart/package.json +++ b/packages/superset-ui-chart/package.json @@ -28,7 +28,9 @@ "dependencies": { "@types/react": "^16.7.17", "@types/react-loadable": "^5.4.2", + "@vx/responsive": "^0.0.189", "prop-types": "^15.6.2", + "react-error-boundary": "^1.2.5", "react-loadable": "^5.5.0", "reselect": "^4.0.0" }, @@ -40,6 +42,7 @@ "peerDependencies": { "@superset-ui/connection": "^0.11.0", "@superset-ui/core": "^0.11.0", + "@superset-ui/dimension": "^0.11.10", "react": "^15 || ^16" } } diff --git a/packages/superset-ui-chart/src/components/FallbackComponent.tsx b/packages/superset-ui-chart/src/components/FallbackComponent.tsx new file mode 100644 index 0000000000..e2038bd47d --- /dev/null +++ b/packages/superset-ui-chart/src/components/FallbackComponent.tsx @@ -0,0 +1,34 @@ +import React from 'react'; +import { FallbackPropsWithDimension } from './SuperChart'; + +export type Props = FallbackPropsWithDimension; + +const CONTAINER_STYLE = { + backgroundColor: '#000', + color: '#fff', + overflow: 'auto', + padding: 32, +}; + +export default function FallbackComponent({ componentStack, error, height, width }: Props) { + return ( +
+
+
+ Oops! An error occured! +
+ {error ? error.toString() : 'Unknown Error'} +
+ {componentStack && ( +
+ Stack Trace: + + {componentStack.split('\n').map((row: string) => ( +
{row}
+ ))} + +
+ )} +
+ ); +} diff --git a/packages/superset-ui-chart/src/components/SuperChart.tsx b/packages/superset-ui-chart/src/components/SuperChart.tsx index 4eba54591d..58238d96ed 100644 --- a/packages/superset-ui-chart/src/components/SuperChart.tsx +++ b/packages/superset-ui-chart/src/components/SuperChart.tsx @@ -1,209 +1,111 @@ -import * as React from 'react'; -import { createSelector } from 'reselect'; -import getChartComponentRegistry from '../registries/ChartComponentRegistrySingleton'; -import getChartTransformPropsRegistry from '../registries/ChartTransformPropsRegistrySingleton'; -import ChartProps from '../models/ChartProps'; -import createLoadableRenderer, { LoadableRenderer } from './createLoadableRenderer'; -import { ChartType } from '../models/ChartPlugin'; -import { PreTransformProps, TransformProps, PostTransformProps } from '../types/TransformFunction'; -import { HandlerFunction } from '../types/Base'; +import React from 'react'; +import ErrorBoundary, { ErrorBoundaryProps, FallbackProps } from 'react-error-boundary'; +import { parseLength } from '@superset-ui/dimension'; +import { ParentSize } from '@vx/responsive'; +import SuperChartCore, { Props as SuperChartCoreProps } from './SuperChartCore'; +import DefaultFallbackComponent from './FallbackComponent'; +import ChartProps, { ChartPropsConfig } from '../models/ChartProps'; -const IDENTITY = (x: any) => x; - -const EMPTY = () => null; - -/* eslint-disable sort-keys */ const defaultProps = { - id: '', - className: '', - preTransformProps: IDENTITY, - overrideTransformProps: undefined, - postTransformProps: IDENTITY, - onRenderSuccess() {}, - onRenderFailure() {}, + FallbackComponent: DefaultFallbackComponent, + // eslint-disable-next-line no-magic-numbers + height: 400 as string | number, + width: '100%' as string | number, }; -/* eslint-enable sort-keys */ - -interface LoadingProps { - error: any; -} - -interface LoadedModules { - Chart: ChartType; - transformProps: TransformProps; -} -interface RenderProps { - chartProps: ChartProps; - preTransformProps?: PreTransformProps; - postTransformProps?: PostTransformProps; -} +export type FallbackPropsWithDimension = FallbackProps & { width?: number; height?: number }; -const BLANK_CHART_PROPS = new ChartProps(); +export type Props = Omit & + Omit & { + disableErrorBoundary?: boolean; + debounceTime?: number; + FallbackComponent?: React.ComponentType; + onErrorBoundary?: ErrorBoundaryProps['onError']; + height?: number | string; + width?: number | string; + }; -export interface SuperChartProps { - id?: string; - className?: string; - chartProps?: ChartProps | null; - chartType: string; - preTransformProps?: PreTransformProps; - overrideTransformProps?: TransformProps; - postTransformProps?: PostTransformProps; - onRenderSuccess?: HandlerFunction; - onRenderFailure?: HandlerFunction; -} +type PropsWithDefault = Props & Readonly; -export default class SuperChart extends React.PureComponent { +export default class SuperChart extends React.PureComponent { static defaultProps = defaultProps; - processChartProps: (input: { - chartProps: ChartProps; - preTransformProps?: PreTransformProps; - transformProps?: TransformProps; - postTransformProps?: PostTransformProps; - }) => any; - - createLoadableRenderer: (input: { - chartType: string; - overrideTransformProps?: TransformProps; - }) => LoadableRenderer | (() => null); - - constructor(props: SuperChartProps) { - super(props); - - this.renderChart = this.renderChart.bind(this); - this.renderLoading = this.renderLoading.bind(this); - - // memoized function so it will not recompute - // and return previous value - // unless one of - // - preTransformProps - // - transformProps - // - postTransformProps - // - chartProps - // is changed. - this.processChartProps = createSelector( - input => input.preTransformProps, - input => input.transformProps, - input => input.postTransformProps, - input => input.chartProps, - (pre = IDENTITY, transform = IDENTITY, post = IDENTITY, chartProps) => - post(transform(pre(chartProps))), - ); - - const componentRegistry = getChartComponentRegistry(); - const transformPropsRegistry = getChartTransformPropsRegistry(); - - // memoized function so it will not recompute - // and return previous value - // unless one of - // - chartType - // - overrideTransformProps - // is changed. - this.createLoadableRenderer = createSelector( - input => input.chartType, - input => input.overrideTransformProps, - (chartType, overrideTransformProps) => { - if (chartType) { - const Renderer = createLoadableRenderer({ - loader: { - Chart: () => componentRegistry.getAsPromise(chartType), - transformProps: overrideTransformProps - ? () => Promise.resolve(overrideTransformProps) - : () => transformPropsRegistry.getAsPromise(chartType), - }, - loading: (loadingProps: LoadingProps) => this.renderLoading(loadingProps, chartType), - render: this.renderChart, - }); - - // Trigger preloading. - Renderer.preload(); - - return Renderer; - } - - return EMPTY; - }, - ); - } - - renderChart(loaded: LoadedModules, props: RenderProps) { - const { Chart, transformProps } = loaded; - const { chartProps, preTransformProps, postTransformProps } = props; + private createChartProps = ChartProps.createSelector(); - return ( - - ); - } - - renderLoading(loadingProps: LoadingProps, chartType: string) { - const { error } = loadingProps; - - if (error) { - return ( -
- ERROR  - chartType="{chartType}" — - {error.toString()} -
- ); - } - - return null; - } - - render() { + renderChart(width: number, height: number) { const { id, className, + chartType, preTransformProps, + overrideTransformProps, postTransformProps, - chartProps = BLANK_CHART_PROPS, onRenderSuccess, onRenderFailure, - } = this.props; + disableErrorBoundary, + FallbackComponent, + onErrorBoundary, + ...rest + } = this.props as PropsWithDefault; + + const chart = ( + + ); - // Create LoadableRenderer and start preloading - // the lazy-loaded Chart components - const Renderer = this.createLoadableRenderer(this.props); + // Include the error boundary by default unless it is specifically disabled. + return disableErrorBoundary === true ? ( + chart + ) : ( + ( + + )} + onError={onErrorBoundary} + > + {chart} + + ); + } - // Do not render if chartProps is set to null. - // but the pre-loading has been started in this.createLoadableRenderer - // to prepare for rendering once chartProps becomes available. - if (chartProps === null) { - return null; - } + render() { + const { width: inputWidth, height: inputHeight } = this.props as PropsWithDefault; - const containerProps: { - id?: string; - className?: string; - } = {}; - if (id) { - containerProps.id = id; - } - if (className) { - containerProps.className = className; + // Parse them in case they are % or 'auto' + const widthInfo = parseLength(inputWidth); + const heightInfo = parseLength(inputHeight); + + // If any of the dimension is dynamic, get parent's dimension + if (widthInfo.isDynamic || heightInfo.isDynamic) { + const { debounceTime } = this.props; + + return ( + + {({ width, height }) => + width > 0 && + height > 0 && + this.renderChart( + widthInfo.isDynamic ? Math.floor(width * widthInfo.multiplier) : widthInfo.value, + heightInfo.isDynamic ? Math.floor(height * heightInfo.multiplier) : heightInfo.value, + ) + } + + ); } - return ( -
- -
- ); + return this.renderChart(widthInfo.value, heightInfo.value); } } diff --git a/packages/superset-ui-chart/src/components/SuperChartCore.tsx b/packages/superset-ui-chart/src/components/SuperChartCore.tsx new file mode 100644 index 0000000000..a1a80c7d3f --- /dev/null +++ b/packages/superset-ui-chart/src/components/SuperChartCore.tsx @@ -0,0 +1,209 @@ +import * as React from 'react'; +import { createSelector } from 'reselect'; +import getChartComponentRegistry from '../registries/ChartComponentRegistrySingleton'; +import getChartTransformPropsRegistry from '../registries/ChartTransformPropsRegistrySingleton'; +import ChartProps from '../models/ChartProps'; +import createLoadableRenderer, { LoadableRenderer } from './createLoadableRenderer'; +import { ChartType } from '../models/ChartPlugin'; +import { PreTransformProps, TransformProps, PostTransformProps } from '../types/TransformFunction'; +import { HandlerFunction } from '../types/Base'; + +const IDENTITY = (x: any) => x; + +const EMPTY = () => null; + +/* eslint-disable sort-keys */ +const defaultProps = { + id: '', + className: '', + preTransformProps: IDENTITY, + overrideTransformProps: undefined, + postTransformProps: IDENTITY, + onRenderSuccess() {}, + onRenderFailure() {}, +}; +/* eslint-enable sort-keys */ + +interface LoadingProps { + error: any; +} + +interface LoadedModules { + Chart: ChartType; + transformProps: TransformProps; +} + +interface RenderProps { + chartProps: ChartProps; + preTransformProps?: PreTransformProps; + postTransformProps?: PostTransformProps; +} + +const BLANK_CHART_PROPS = new ChartProps(); + +export type Props = { + id?: string; + className?: string; + chartProps?: ChartProps | null; + chartType: string; + preTransformProps?: PreTransformProps; + overrideTransformProps?: TransformProps; + postTransformProps?: PostTransformProps; + onRenderSuccess?: HandlerFunction; + onRenderFailure?: HandlerFunction; +}; + +export default class SuperChartCore extends React.PureComponent { + static defaultProps = defaultProps; + + processChartProps: (input: { + chartProps: ChartProps; + preTransformProps?: PreTransformProps; + transformProps?: TransformProps; + postTransformProps?: PostTransformProps; + }) => any; + + createLoadableRenderer: (input: { + chartType: string; + overrideTransformProps?: TransformProps; + }) => LoadableRenderer | (() => null); + + constructor(props: Props) { + super(props); + + this.renderChart = this.renderChart.bind(this); + this.renderLoading = this.renderLoading.bind(this); + + // memoized function so it will not recompute + // and return previous value + // unless one of + // - preTransformProps + // - transformProps + // - postTransformProps + // - chartProps + // is changed. + this.processChartProps = createSelector( + input => input.preTransformProps, + input => input.transformProps, + input => input.postTransformProps, + input => input.chartProps, + (pre = IDENTITY, transform = IDENTITY, post = IDENTITY, chartProps) => + post(transform(pre(chartProps))), + ); + + const componentRegistry = getChartComponentRegistry(); + const transformPropsRegistry = getChartTransformPropsRegistry(); + + // memoized function so it will not recompute + // and return previous value + // unless one of + // - chartType + // - overrideTransformProps + // is changed. + this.createLoadableRenderer = createSelector( + input => input.chartType, + input => input.overrideTransformProps, + (chartType, overrideTransformProps) => { + if (chartType) { + const Renderer = createLoadableRenderer({ + loader: { + Chart: () => componentRegistry.getAsPromise(chartType), + transformProps: overrideTransformProps + ? () => Promise.resolve(overrideTransformProps) + : () => transformPropsRegistry.getAsPromise(chartType), + }, + loading: (loadingProps: LoadingProps) => this.renderLoading(loadingProps, chartType), + render: this.renderChart, + }); + + // Trigger preloading. + Renderer.preload(); + + return Renderer; + } + + return EMPTY; + }, + ); + } + + renderChart(loaded: LoadedModules, props: RenderProps) { + const { Chart, transformProps } = loaded; + const { chartProps, preTransformProps, postTransformProps } = props; + + return ( + + ); + } + + renderLoading(loadingProps: LoadingProps, chartType: string) { + const { error } = loadingProps; + + if (error) { + return ( +
+ ERROR  + chartType="{chartType}" — + {error.toString()} +
+ ); + } + + return null; + } + + render() { + const { + id, + className, + preTransformProps, + postTransformProps, + chartProps = BLANK_CHART_PROPS, + onRenderSuccess, + onRenderFailure, + } = this.props; + + // Create LoadableRenderer and start preloading + // the lazy-loaded Chart components + const Renderer = this.createLoadableRenderer(this.props); + + // Do not render if chartProps is set to null. + // but the pre-loading has been started in this.createLoadableRenderer + // to prepare for rendering once chartProps becomes available. + if (chartProps === null) { + return null; + } + + const containerProps: { + id?: string; + className?: string; + } = {}; + if (id) { + containerProps.id = id; + } + if (className) { + containerProps.className = className; + } + + return ( +
+ +
+ ); + } +} diff --git a/packages/superset-ui-chart/src/components/SuperChartShell.tsx b/packages/superset-ui-chart/src/components/SuperChartShell.tsx new file mode 100644 index 0000000000..06bed84b22 --- /dev/null +++ b/packages/superset-ui-chart/src/components/SuperChartShell.tsx @@ -0,0 +1,62 @@ +import React from 'react'; +import SuperChart, { Props as SuperChartProps } from './SuperChart'; +import ChartProps, { ChartPropsConfig } from '../models/ChartProps'; + +/** SuperChart Props for version 0.11 and below has chartProps */ +type ClassicProps = Omit< + SuperChartProps, + | 'annotationData' + | 'datasource' + | 'filters' + | 'formData' + | 'payload' + | 'onAddFilter' + | 'onError' + | 'setControlValue' + | 'setTooltip' + | 'width' + | 'height' +> & { + chartProps: ChartProps | ChartPropsConfig; +}; + +export type Props = ClassicProps | SuperChartProps; + +export default function SuperChartShell(props: Props) { + if ('chartProps' in props) { + const { chartProps, ...rest } = props; + + const { + annotationData, + datasource, + filters, + formData, + payload, + onAddFilter, + onError, + setControlValue, + setTooltip, + width, + height, + } = chartProps; + + return ( + + ); + } + + return ; +} diff --git a/packages/superset-ui-chart/src/index.ts b/packages/superset-ui-chart/src/index.ts index f9352c83fc..0a992f95ec 100644 --- a/packages/superset-ui-chart/src/index.ts +++ b/packages/superset-ui-chart/src/index.ts @@ -5,7 +5,7 @@ export { default as ChartProps } from './models/ChartProps'; export { default as createLoadableRenderer } from './components/createLoadableRenderer'; export { default as reactify } from './components/reactify'; -export { default as SuperChart } from './components/SuperChart'; +export { default as SuperChart } from './components/SuperChartShell'; export { default as getChartBuildQueryRegistry, diff --git a/packages/superset-ui-chart/src/models/ChartProps.ts b/packages/superset-ui-chart/src/models/ChartProps.ts index 62803e2518..e5317156ba 100644 --- a/packages/superset-ui-chart/src/models/ChartProps.ts +++ b/packages/superset-ui-chart/src/models/ChartProps.ts @@ -12,7 +12,7 @@ export type QueryData = PlainObject; type Filters = any[]; type ChartPropsSelector = (c: ChartPropsConfig) => ChartProps; -interface ChartPropsConfig { +export interface ChartPropsConfig { annotationData?: AnnotationData; datasource?: SnakeCaseDatasource; filters?: Filters; diff --git a/packages/superset-ui-chart/test/components/FallbackComponent.test.tsx b/packages/superset-ui-chart/test/components/FallbackComponent.test.tsx new file mode 100644 index 0000000000..4df12df503 --- /dev/null +++ b/packages/superset-ui-chart/test/components/FallbackComponent.test.tsx @@ -0,0 +1,38 @@ +import React from 'react'; +import { shallow } from 'enzyme'; +import FallbackComponent from '../../src/components/FallbackComponent'; + +describe('FallbackComponent', () => { + const ERROR = new Error('CaffeineOverLoadException'); + const STACK_TRACE = 'Error at line 1: x.drink(coffee)'; + + it('renders error and stack trace', () => { + const wrapper = shallow(); + const messages = wrapper.find('code'); + expect(messages).toHaveLength(2); + expect(messages.at(0).text()).toEqual('Error: CaffeineOverLoadException'); + expect(messages.at(1).text()).toEqual('Error at line 1: x.drink(coffee)'); + }); + + it('renders error only', () => { + const wrapper = shallow(); + const messages = wrapper.find('code'); + expect(messages).toHaveLength(1); + expect(messages.at(0).text()).toEqual('Error: CaffeineOverLoadException'); + }); + + it('renders stacktrace only', () => { + const wrapper = shallow(); + const messages = wrapper.find('code'); + expect(messages).toHaveLength(2); + expect(messages.at(0).text()).toEqual('Unknown Error'); + expect(messages.at(1).text()).toEqual('Error at line 1: x.drink(coffee)'); + }); + + it('renders when nothing is given', () => { + const wrapper = shallow(); + const messages = wrapper.find('code'); + expect(messages).toHaveLength(1); + expect(messages.at(0).text()).toEqual('Unknown Error'); + }); +}); diff --git a/packages/superset-ui-chart/test/components/MockChartPlugins.tsx b/packages/superset-ui-chart/test/components/MockChartPlugins.tsx index 6e6f3e6d5f..8540cdd2a9 100644 --- a/packages/superset-ui-chart/test/components/MockChartPlugins.tsx +++ b/packages/superset-ui-chart/test/components/MockChartPlugins.tsx @@ -1,18 +1,47 @@ import React from 'react'; import { ChartMetadata, ChartPlugin, ChartFormData } from '../../src'; +const DIMENSION_STYLE = { + fontSize: 36, + fontWeight: 700, + flex: '1 1 auto', + display: 'flex', + alignItems: 'center', +}; + export const TestComponent = ({ + formData, message, width, height, }: { - message: string; - width: number; - height: number; + formData?: any; + message?: string; + width?: number; + height?: number; }) => ( -
- {message || 'test-message'} - {[width, height].join('x')} +
+
+ {message || 'custom component'} +
+
+ {[width, height].join('x')} +
+
+ {JSON.stringify(formData)} +
); @@ -20,6 +49,7 @@ export const ChartKeys = { DILIGENT: 'diligent-chart', LAZY: 'lazy-chart', SLOW: 'slow-chart', + BUGGY: 'buggy-chart', }; export class DiligentChartPlugin extends ChartPlugin { @@ -67,3 +97,18 @@ export class SlowChartPlugin extends ChartPlugin { }); } } + +export class BuggyChartPlugin extends ChartPlugin { + constructor() { + super({ + metadata: new ChartMetadata({ + name: ChartKeys.BUGGY, + thumbnail: '', + }), + Chart: () => { + throw new Error('The component is too buggy to render.'); + }, + transformProps: x => x, + }); + } +} diff --git a/packages/superset-ui-chart/test/components/SuperChart.test.tsx b/packages/superset-ui-chart/test/components/SuperChart.test.tsx deleted file mode 100644 index d204120c48..0000000000 --- a/packages/superset-ui-chart/test/components/SuperChart.test.tsx +++ /dev/null @@ -1,162 +0,0 @@ -import React from 'react'; -import { mount, shallow } from 'enzyme'; -import { ChartProps } from '../../src'; -import { - ChartKeys, - DiligentChartPlugin, - LazyChartPlugin, - SlowChartPlugin, -} from './MockChartPlugins'; -import SuperChart from '../../src/components/SuperChart'; - -describe('SuperChart', () => { - const chartProps = new ChartProps(); - - new DiligentChartPlugin().configure({ key: ChartKeys.DILIGENT }).register(); - new LazyChartPlugin().configure({ key: ChartKeys.LAZY }).register(); - new SlowChartPlugin().configure({ key: ChartKeys.SLOW }).register(); - - describe('registered charts', () => { - it('renders registered chart', done => { - const wrapper = shallow( - , - ); - setTimeout(() => { - expect(wrapper.render().find('div.test-component')).toHaveLength(1); - done(); - }, 0); - }); - it('renders registered chart with default export', done => { - const wrapper = shallow(); - setTimeout(() => { - expect(wrapper.render().find('div.test-component')).toHaveLength(1); - done(); - }, 0); - }); - it('does not render if chartType is not set', done => { - // @ts-ignore chartType is required - const wrapper = shallow(); - setTimeout(() => { - expect(wrapper.render().children()).toHaveLength(0); - done(); - }, 5); - }); - it('adds id to container if specified', done => { - const wrapper = shallow(); - setTimeout(() => { - expect(wrapper.render().attr('id')).toEqual('the-chart'); - done(); - }, 0); - }); - it('adds class to container if specified', done => { - const wrapper = shallow(); - setTimeout(() => { - expect(wrapper.hasClass('the-chart')).toBeTruthy(); - done(); - }, 0); - }); - it('uses overrideTransformProps when specified', done => { - const wrapper = shallow( - ({ message: 'hulk' })} - />, - ); - setTimeout(() => { - expect( - wrapper - .render() - .find('span.message') - .text(), - ).toEqual('hulk'); - done(); - }, 0); - }); - it('uses preTransformProps when specified', done => { - const chartPropsWithPayload = new ChartProps({ - payload: { message: 'hulk' }, - }); - const wrapper = shallow( - chartPropsWithPayload} - overrideTransformProps={props => props.payload} - />, - ); - setTimeout(() => { - expect( - wrapper - .render() - .find('span.message') - .text(), - ).toEqual('hulk'); - done(); - }, 0); - }); - it('uses postTransformProps when specified', done => { - const wrapper = shallow( - ({ message: 'hulk' })} - />, - ); - setTimeout(() => { - expect( - wrapper - .render() - .find('span.message') - .text(), - ).toEqual('hulk'); - done(); - }, 0); - }); - it('renders if chartProps is not specified', done => { - const wrapper = shallow(); - setTimeout(() => { - expect(wrapper.render().find('div.test-component')).toHaveLength(1); - done(); - }, 0); - }); - it('does not render anything while waiting for Chart code to load', done => { - const wrapper = shallow(); - setTimeout(() => { - expect(wrapper.render().children()).toHaveLength(0); - done(); - }, 0); - }); - it('eventually renders after Chart is loaded', done => { - const wrapper = shallow(); - setTimeout(() => { - expect(wrapper.render().find('div.test-component')).toHaveLength(1); - done(); - }, 1500); - }); - it('does not render if chartProps is null', done => { - const wrapper = shallow(); - setTimeout(() => { - expect(wrapper.render().find('div.test-component')).toHaveLength(0); - done(); - }, 0); - }); - }); - - describe('unregistered charts', () => { - it('renders error message', done => { - const wrapper = mount(); - setTimeout(() => { - expect(wrapper.render().find('.alert')).toHaveLength(1); - done(); - }, 0); - }); - }); - - describe('.processChartProps()', () => { - it('use identity functions for unspecified transforms', () => { - const chart = new SuperChart({ - chartType: ChartKeys.DILIGENT, - }); - const chartProps2 = new ChartProps(); - expect(chart.processChartProps({ chartProps: chartProps2 })).toBe(chartProps2); - }); - }); -}); diff --git a/packages/superset-ui-chart/test/components/SuperChartCore.test.tsx b/packages/superset-ui-chart/test/components/SuperChartCore.test.tsx new file mode 100644 index 0000000000..506f71518d --- /dev/null +++ b/packages/superset-ui-chart/test/components/SuperChartCore.test.tsx @@ -0,0 +1,179 @@ +import React from 'react'; +import { mount, shallow } from 'enzyme'; +import { ChartProps } from '../../src'; +import { + ChartKeys, + DiligentChartPlugin, + LazyChartPlugin, + SlowChartPlugin, +} from './MockChartPlugins'; +import SuperChartCore from '../../src/components/SuperChartCore'; +import promiseTimeout from './promiseTimeout'; + +describe('SuperChartCore', () => { + const chartProps = new ChartProps(); + + const plugins = [ + new DiligentChartPlugin().configure({ key: ChartKeys.DILIGENT }), + new LazyChartPlugin().configure({ key: ChartKeys.LAZY }), + new SlowChartPlugin().configure({ key: ChartKeys.SLOW }), + ]; + + beforeAll(() => { + plugins.forEach(p => { + p.unregister().register(); + }); + }); + + afterAll(() => { + plugins.forEach(p => { + p.unregister(); + }); + }); + + describe('registered charts', () => { + it('renders registered chart', () => { + const wrapper = shallow( + , + ); + + return promiseTimeout(() => { + expect(wrapper.render().find('div.test-component')).toHaveLength(1); + }); + }); + it('renders registered chart with lazy loading', () => { + const wrapper = shallow(); + + return promiseTimeout(() => { + expect(wrapper.render().find('div.test-component')).toHaveLength(1); + }); + }); + it('does not render if chartType is not set', () => { + // @ts-ignore chartType is required + const wrapper = shallow(); + + return promiseTimeout(() => { + expect(wrapper.render().children()).toHaveLength(0); + }, 5); + }); + it('adds id to container if specified', () => { + const wrapper = shallow(); + + return promiseTimeout(() => { + expect(wrapper.render().attr('id')).toEqual('the-chart'); + }); + }); + it('adds class to container if specified', () => { + const wrapper = shallow( + , + ); + + return promiseTimeout(() => { + expect(wrapper.hasClass('the-chart')).toBeTruthy(); + }, 0); + }); + it('uses overrideTransformProps when specified', () => { + const wrapper = shallow( + ({ message: 'hulk' })} + />, + ); + + return promiseTimeout(() => { + expect( + wrapper + .render() + .find('.message') + .text(), + ).toEqual('hulk'); + }); + }); + it('uses preTransformProps when specified', () => { + const chartPropsWithPayload = new ChartProps({ + payload: { message: 'hulk' }, + }); + const wrapper = shallow( + chartPropsWithPayload} + overrideTransformProps={props => props.payload} + />, + ); + + return promiseTimeout(() => { + expect( + wrapper + .render() + .find('.message') + .text(), + ).toEqual('hulk'); + }); + }); + it('uses postTransformProps when specified', () => { + const wrapper = shallow( + ({ message: 'hulk' })} + />, + ); + + return promiseTimeout(() => { + expect( + wrapper + .render() + .find('.message') + .text(), + ).toEqual('hulk'); + }); + }); + it('renders if chartProps is not specified', () => { + const wrapper = shallow(); + + return promiseTimeout(() => { + expect(wrapper.render().find('div.test-component')).toHaveLength(1); + }); + }); + it('does not render anything while waiting for Chart code to load', () => { + const wrapper = shallow(); + + return promiseTimeout(() => { + expect(wrapper.render().children()).toHaveLength(0); + }); + }); + it('eventually renders after Chart is loaded', () => { + const wrapper = shallow(); + + return promiseTimeout(() => { + expect(wrapper.render().find('div.test-component')).toHaveLength(1); + }, 1500); + }); + it('does not render if chartProps is null', () => { + const wrapper = shallow(); + + return promiseTimeout(() => { + expect(wrapper.render().find('div.test-component')).toHaveLength(0); + }); + }); + }); + + describe('unregistered charts', () => { + it('renders error message', () => { + const wrapper = mount(); + + return promiseTimeout(() => { + expect(wrapper.render().find('.alert')).toHaveLength(1); + }); + }); + }); + + describe('.processChartProps()', () => { + it('use identity functions for unspecified transforms', () => { + const chart = new SuperChartCore({ + chartType: ChartKeys.DILIGENT, + }); + const chartProps2 = new ChartProps(); + expect(chart.processChartProps({ chartProps: chartProps2 })).toBe(chartProps2); + }); + }); +}); diff --git a/packages/superset-ui-chart/test/components/SuperChartShell.test.tsx b/packages/superset-ui-chart/test/components/SuperChartShell.test.tsx new file mode 100644 index 0000000000..1afc2dcdee --- /dev/null +++ b/packages/superset-ui-chart/test/components/SuperChartShell.test.tsx @@ -0,0 +1,228 @@ +/* eslint-disable import/first */ +import React from 'react'; +import { mount } from 'enzyme'; + +jest.mock('resize-observer-polyfill'); +// @ts-ignore +import { triggerResizeObserver } from 'resize-observer-polyfill'; +import ErrorBoundary from 'react-error-boundary'; +import { ChartProps, SuperChart } from '../../src'; +import RealSuperChart from '../../src/components/SuperChart'; +import { ChartKeys, DiligentChartPlugin, BuggyChartPlugin } from './MockChartPlugins'; +import promiseTimeout from './promiseTimeout'; + +function expectDimension(renderedWrapper: Cheerio, width: number, height: number) { + expect(renderedWrapper.find('.dimension').text()).toEqual([width, height].join('x')); +} + +describe('SuperChart', () => { + const plugins = [ + new DiligentChartPlugin().configure({ key: ChartKeys.DILIGENT }), + new BuggyChartPlugin().configure({ key: ChartKeys.BUGGY }), + ]; + + beforeAll(() => { + plugins.forEach(p => { + p.unregister().register(); + }); + }); + + afterAll(() => { + plugins.forEach(p => { + p.unregister(); + }); + }); + + describe('includes ErrorBoundary', () => { + it('renders default FallbackComponent', () => { + jest.spyOn(RealSuperChart.defaultProps, 'FallbackComponent'); + const wrapper = mount(); + const renderedWrapper = wrapper.render(); + + return promiseTimeout(() => { + expect(renderedWrapper.find('div.test-component')).toHaveLength(0); + expect(RealSuperChart.defaultProps.FallbackComponent).toHaveBeenCalledTimes(1); + }, 100); + }); + it('renders custom FallbackComponent', () => { + const CustomFallbackComponent = jest.fn(() =>
Custom Fallback!
); + const wrapper = mount( + , + ); + + return promiseTimeout(() => { + expect(wrapper.render().find('div.test-component')).toHaveLength(0); + expect(CustomFallbackComponent).toBeCalledTimes(1); + }); + }); + it('call onErrorBoundary', () => { + const handleError = jest.fn(); + mount( + , + ); + + return promiseTimeout(() => { + expect(handleError).toHaveBeenCalledTimes(1); + }); + }); + it('does not include ErrorBoundary if told so', () => { + const inactiveErrorHandler = jest.fn(); + const activeErrorHandler = jest.fn(); + mount( + + + , + ); + + return promiseTimeout(() => { + expect(activeErrorHandler).toHaveBeenCalledTimes(1); + expect(inactiveErrorHandler).toHaveBeenCalledTimes(0); + }); + }); + }); + + describe('supports multiple way of specifying chartProps', () => { + it('chartProps is instanceof ChartProps', () => { + const wrapper = mount( + , + ); + + return promiseTimeout(() => { + const renderedWrapper = wrapper.render(); + expect(renderedWrapper.find('div.test-component')).toHaveLength(1); + expectDimension(renderedWrapper, 20, 20); + }); + }); + it('chartProps is ChartPropsConfig', () => { + const wrapper = mount( + , + ); + + return promiseTimeout(() => { + const renderedWrapper = wrapper.render(); + expect(renderedWrapper.find('div.test-component')).toHaveLength(1); + expectDimension(renderedWrapper, 201, 202); + }); + }); + it('fields of chartProps are listed as props of SuperChart', () => { + const wrapper = mount( + , + ); + + return promiseTimeout(() => { + const renderedWrapper = wrapper.render(); + expect(renderedWrapper.find('div.test-component')).toHaveLength(1); + expectDimension(renderedWrapper, 101, 118); + }); + }); + }); + + describe('supports dynamic width and/or height', () => { + it('works with width and height that are numbers', () => { + const wrapper = mount(); + + return promiseTimeout(() => { + const renderedWrapper = wrapper.render(); + expect(renderedWrapper.find('div.test-component')).toHaveLength(1); + expectDimension(renderedWrapper, 100, 100); + }); + }); + it('works when width and height are percent', () => { + const wrapper = mount( + , + ); + triggerResizeObserver(); + + return promiseTimeout(() => { + const renderedWrapper = wrapper.render(); + expect(renderedWrapper.find('div.test-component')).toHaveLength(1); + expectDimension(renderedWrapper, 300, 300); + }, 100); + }); + it('works when only width is percent', () => { + const wrapper = mount( + , + ); + triggerResizeObserver(); + + return promiseTimeout(() => { + const renderedWrapper = wrapper.render(); + expect(renderedWrapper.find('div.test-component')).toHaveLength(1); + expectDimension(renderedWrapper, 150, 125); + }, 100); + }); + it('works when only height is percent', () => { + const wrapper = mount( + , + ); + triggerResizeObserver(); + + return promiseTimeout(() => { + const renderedWrapper = wrapper.render(); + expect(renderedWrapper.find('div.test-component')).toHaveLength(1); + expectDimension(renderedWrapper, 50, 75); + }, 100); + }); + it('works when width and height are not specified', () => { + const wrapper = mount(); + triggerResizeObserver(); + + return promiseTimeout(() => { + const renderedWrapper = wrapper.render(); + expect(renderedWrapper.find('div.test-component')).toHaveLength(1); + expectDimension(renderedWrapper, 300, 400); + }, 100); + }); + it('works when width and height are inside chartProps', () => { + const wrapper = mount( + , + ); + + return promiseTimeout(() => { + const renderedWrapper = wrapper.render(); + expect(renderedWrapper.find('div.test-component')).toHaveLength(1); + expectDimension(renderedWrapper, 123, 456); + }, 100); + }); + it('works when there is chartProps but still no width and height', () => { + const wrapper = mount( + , + ); + triggerResizeObserver(); + + return promiseTimeout(() => { + const renderedWrapper = wrapper.render(); + expect(renderedWrapper.find('div.test-component')).toHaveLength(1); + expectDimension(renderedWrapper, 300, 400); + }, 100); + }); + }); +}); diff --git a/packages/superset-ui-chart/test/components/promiseTimeout.tsx b/packages/superset-ui-chart/test/components/promiseTimeout.tsx new file mode 100644 index 0000000000..c5d9af9963 --- /dev/null +++ b/packages/superset-ui-chart/test/components/promiseTimeout.tsx @@ -0,0 +1,13 @@ +/** setTimeout that returns a promise */ +export default function promiseTimeout( + /** A function to be executed after the timer expires. */ + func: Function, + /** The time, in milliseconds (thousandths of a second), the timer should wait before the specified function or code is executed. If this parameter is omitted, a value of 0 is used, meaning execute "immediately", or more accurately, as soon as possible. */ + delay?: number, +) { + return new Promise(resolve => { + setTimeout(() => { + resolve(func()); + }, delay); + }); +} diff --git a/packages/superset-ui-chart/types/@vx/responsive/index.d.ts b/packages/superset-ui-chart/types/@vx/responsive/index.d.ts new file mode 100644 index 0000000000..4ee0d9b6d0 --- /dev/null +++ b/packages/superset-ui-chart/types/@vx/responsive/index.d.ts @@ -0,0 +1,70 @@ +declare module '@vx/responsive' { + import React from 'react'; + + export const ScaleSVG: React.ComponentType<{ + children: React.ReactNode; + width: number | string; + height: number | string; + xOrigin?: number | string; + yOrigin?: number | string; + preserveAspectRatio?: string; + innerRef?: () => void | string; + }>; + + export interface ParentSizeState { + width: number; + height: number; + top: number; + left: number; + } + + export const ParentSize: React.ComponentClass< + { + className?: string; + children: (renderProps: { + width: number; + height: number; + top: number; + left: number; + ref: HTMLElement; + resize: (state: ParentSizeState) => void; + }) => React.ReactNode; + debounceTime?: number; + }, + ParentSizeState + >; + + export interface WithParentSizeProps { + parentWidth: number; + parentHeight: number; + } + + export function withParentSize( + BaseComponent: React.ComponentType, + ): React.ComponentClass< + { + debounceTime?: number; + } & T, + { + parentWidth: number | null; + parentHeight: number | null; + } + >; + + export interface WithScreenSizeProps { + screeWidth: number; + screenHeight: number; + } + + export function withScreenSize( + BaseComponent: React.ComponentType, + ): React.ComponentClass< + { + windowResizeDebounceTime?: number; + } & T, + { + screenWidth: number | null; + screenHeight: number | null; + } + >; +} diff --git a/packages/superset-ui-demo/.storybook/config.js b/packages/superset-ui-demo/.storybook/config.js index b5baeb07b5..33f3917ad1 100644 --- a/packages/superset-ui-demo/.storybook/config.js +++ b/packages/superset-ui-demo/.storybook/config.js @@ -6,6 +6,7 @@ addParameters({ addonPanelInRight: false, enableShortcuts: false, goFullScreen: false, + hierarchyRootSeparator: null, hierarchySeparator: /\|/, selectedAddonPanel: undefined, // The order of addons in the "Addon panel" is the same as you import them in 'addons.js'. The first panel will be opened by default as you run Storybook showAddonPanel: true, diff --git a/packages/superset-ui-demo/.storybook/webpack.config.js b/packages/superset-ui-demo/.storybook/webpack.config.js index 42533ec3b3..f09b052645 100644 --- a/packages/superset-ui-demo/.storybook/webpack.config.js +++ b/packages/superset-ui-demo/.storybook/webpack.config.js @@ -7,6 +7,11 @@ module.exports = async ({ config }) => { '@babel/preset-react', '@babel/preset-typescript', ], + plugins: [ + '@babel/plugin-proposal-object-rest-spread', + '@babel/plugin-proposal-class-properties', + '@babel/plugin-syntax-dynamic-import', + ], }, test: /\.tsx?$/, exclude: /node_modules/, diff --git a/packages/superset-ui-demo/storybook/stories/superset-ui-chart/SuperChartStories.tsx b/packages/superset-ui-demo/storybook/stories/superset-ui-chart/SuperChartStories.tsx new file mode 100644 index 0000000000..a94e3d0eab --- /dev/null +++ b/packages/superset-ui-demo/storybook/stories/superset-ui-chart/SuperChartStories.tsx @@ -0,0 +1,89 @@ +import React from 'react'; +import { text } from '@storybook/addon-knobs'; +import { SuperChart, ChartProps } from '../../../../superset-ui-chart/src'; +import { + DiligentChartPlugin, + BuggyChartPlugin, + ChartKeys, +} from '../../../../superset-ui-chart/test/components/MockChartPlugins'; + +new DiligentChartPlugin().configure({ key: ChartKeys.DILIGENT }).register(); +new BuggyChartPlugin().configure({ key: ChartKeys.BUGGY }).register(); + +export default [ + { + renderStory: () => { + const width = text('Vis width', '50%'); + const height = text('Vis height', '75%'); + + return ( + + ); + }, + storyName: 'Basic', + storyPath: '@superset-ui/chart|SuperChart', + }, + { + renderStory: () => { + const width = text('Vis width', '500'); + const height = text('Vis height', '300'); + + return ( + + ); + }, + storyName: 'passing ChartPropsConfig', + storyPath: '@superset-ui/chart|SuperChart', + }, + { + renderStory: () => { + const width = text('Vis width', '500'); + const height = text('Vis height', '300'); + + return ( + + ); + }, + storyName: 'passing ChartProps', + storyPath: '@superset-ui/chart|SuperChart', + }, + { + renderStory: () => { + const width = text('Vis width', '500'); + const height = text('Vis height', '300'); + + return ( + + ); + }, + storyName: 'With error boundary', + storyPath: '@superset-ui/chart|SuperChart', + }, +]; diff --git a/packages/superset-ui-demo/storybook/stories/superset-ui-chart/index.ts b/packages/superset-ui-demo/storybook/stories/superset-ui-chart/index.ts index aa39e3d724..917c2b123c 100644 --- a/packages/superset-ui-demo/storybook/stories/superset-ui-chart/index.ts +++ b/packages/superset-ui-demo/storybook/stories/superset-ui-chart/index.ts @@ -1,5 +1,6 @@ import ChartDataProviderStories from './ChartDataProviderStories'; +import SuperChartStories from './SuperChartStories'; export default { - examples: [...ChartDataProviderStories], + examples: [...ChartDataProviderStories, ...SuperChartStories], };