diff --git a/superset/assets/spec/javascripts/visualizations/core/createLoadableRenderer_spec.jsx b/superset/assets/spec/javascripts/visualizations/core/createLoadableRenderer_spec.jsx new file mode 100644 index 00000000000..f61d9096cea --- /dev/null +++ b/superset/assets/spec/javascripts/visualizations/core/createLoadableRenderer_spec.jsx @@ -0,0 +1,94 @@ +import React from 'react'; +import { shallow } from 'enzyme'; +import createLoadableRenderer from 'src/visualizations/core/components/createLoadableRenderer'; + +describe('createLoadableRenderer', () => { + function TestComponent() { + return ( +
test
+ ); + } + let loadChartSuccess; + let render; + let loading; + let LoadableRenderer; + + beforeEach(() => { + loadChartSuccess = jest.fn(() => Promise.resolve(TestComponent)); + render = jest.fn((loaded) => { + const { Chart } = loaded; + return (); + }); + loading = jest.fn(() => (
Loading
)); + LoadableRenderer = createLoadableRenderer({ + loader: { + Chart: loadChartSuccess, + }, + loading, + render, + }); + }); + + describe('returns a LoadableRenderer class', () => { + it('LoadableRenderer.preload() preloads the lazy-load components', () => { + expect(LoadableRenderer.preload).toBeInstanceOf(Function); + LoadableRenderer.preload(); + expect(loadChartSuccess).toHaveBeenCalledTimes(1); + }); + + it('calls onRenderSuccess when succeeds', (done) => { + const onRenderSuccess = jest.fn(); + const onRenderFailure = jest.fn(); + shallow( + , + ); + expect(loadChartSuccess).toHaveBeenCalled(); + setTimeout(() => { + expect(render).toHaveBeenCalledTimes(1); + expect(onRenderSuccess).toHaveBeenCalledTimes(1); + expect(onRenderFailure).not.toHaveBeenCalled(); + done(); + }, 10); + }); + + it('calls onRenderFailure when fails', (done) => { + const loadChartFailure = jest.fn(() => Promise.reject('Invalid chart')); + const FailedRenderer = createLoadableRenderer({ + loader: { + Chart: loadChartFailure, + }, + loading, + render, + }); + const onRenderSuccess = jest.fn(); + const onRenderFailure = jest.fn(); + shallow( + , + ); + expect(loadChartFailure).toHaveBeenCalledTimes(1); + setTimeout(() => { + expect(render).not.toHaveBeenCalled(); + expect(onRenderSuccess).not.toHaveBeenCalled(); + expect(onRenderFailure).toHaveBeenCalledTimes(1); + done(); + }, 10); + }); + + it('renders the lazy-load components', (done) => { + const wrapper = shallow(); + // lazy-loaded component not rendered immediately + expect(wrapper.find(TestComponent)).toHaveLength(0); + setTimeout(() => { + // but rendered after the component is loaded. + expect(wrapper.find(TestComponent)).toHaveLength(1); + done(); + }, 10); + }); + }); +}); diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx index 77f91b19848..f5aa34767c7 100644 --- a/superset/assets/src/chart/Chart.jsx +++ b/superset/assets/src/chart/Chart.jsx @@ -211,7 +211,6 @@ class Chart extends React.PureComponent { chartProps={skipChartRendering ? null : this.prepareChartProps()} onRenderSuccess={this.handleRenderSuccess} onRenderFailure={this.handleRenderFailure} - skipRendering={skipChartRendering} /> diff --git a/superset/assets/src/visualizations/core/components/SuperChart.jsx b/superset/assets/src/visualizations/core/components/SuperChart.jsx index 4361b1a1259..e4fad94fa1e 100644 --- a/superset/assets/src/visualizations/core/components/SuperChart.jsx +++ b/superset/assets/src/visualizations/core/components/SuperChart.jsx @@ -1,8 +1,8 @@ import React from 'react'; -import Loadable from 'react-loadable'; import PropTypes from 'prop-types'; import { createSelector } from 'reselect'; import { getChartComponentRegistry, getChartTransformPropsRegistry, ChartProps } from '@superset-ui/chart'; +import createLoadableRenderer from './createLoadableRenderer'; const IDENTITY = x => x; @@ -16,7 +16,6 @@ const propTypes = { postTransformProps: PropTypes.func, onRenderSuccess: PropTypes.func, onRenderFailure: PropTypes.func, - skipRendering: PropTypes.bool, }; const defaultProps = { id: '', @@ -26,7 +25,6 @@ const defaultProps = { postTransformProps: IDENTITY, onRenderSuccess() {}, onRenderFailure() {}, - skipRendering: false, }; class SuperChart extends React.PureComponent { @@ -66,7 +64,7 @@ class SuperChart extends React.PureComponent { input => input.overrideTransformProps, (chartType, overrideTransformProps) => { if (chartType) { - return Loadable.Map({ + const LoadableRenderer = createLoadableRenderer({ loader: { Chart: () => componentRegistry.getAsPromise(chartType), transformProps: overrideTransformProps @@ -76,12 +74,18 @@ class SuperChart extends React.PureComponent { loading: loadingProps => this.renderLoading(loadingProps, chartType), render: this.renderChart, }); + + // Trigger preloading. + LoadableRenderer.preload(); + + return LoadableRenderer; } return null; }, ); } + renderChart(loaded, props) { const Chart = loaded.Chart.default || loaded.Chart; const transformProps = loaded.transformProps.default || loaded.transformProps; @@ -91,7 +95,7 @@ class SuperChart extends React.PureComponent { postTransformProps, } = props; - const result = ( + return ( ); - setTimeout(() => this.props.onRenderSuccess(), 0); - return result; } - renderLoading(loadableProps, chartType) { - const { error } = loadableProps; + renderLoading(loadingProps, chartType) { + const { error } = loadingProps; if (error) { - const result = ( + return (
ERROR  chartType="{chartType}" — {JSON.stringify(error)}
); - setTimeout(() => this.props.onRenderFailure(error), 0); - return result; } return null; @@ -130,14 +130,18 @@ class SuperChart extends React.PureComponent { preTransformProps, postTransformProps, chartProps, - skipRendering, + onRenderSuccess, + onRenderFailure, } = this.props; + // Create LoadableRenderer and start preloading + // the lazy-loaded Chart components const LoadableRenderer = this.createLoadableRenderer(this.props); - // Use this to allow loading the vis components - // without rendering (while waiting for data) - if (skipRendering || !chartProps) { + // Do not render if chartProps is not available. + // but the pre-loading has been started in this.createLoadableRenderer + // to prepare for rendering once chartProps becomes available. + if (!chartProps) { return null; } @@ -148,6 +152,8 @@ class SuperChart extends React.PureComponent { preTransformProps={preTransformProps} postTransformProps={postTransformProps} chartProps={chartProps} + onRenderSuccess={onRenderSuccess} + onRenderFailure={onRenderFailure} /> )} diff --git a/superset/assets/src/visualizations/core/components/createLoadableRenderer.js b/superset/assets/src/visualizations/core/components/createLoadableRenderer.js new file mode 100644 index 00000000000..5638473367f --- /dev/null +++ b/superset/assets/src/visualizations/core/components/createLoadableRenderer.js @@ -0,0 +1,46 @@ +import Loadable from 'react-loadable'; +import PropTypes from 'prop-types'; + +const propTypes = { + onRenderSuccess: PropTypes.func, + onRenderFailure: PropTypes.func, +}; + +const defaultProps = { + onRenderSuccess() {}, + onRenderFailure() {}, +}; + +export default function createLoadableRenderer(options) { + const LoadableRenderer = Loadable.Map(options); + + // Extends the behavior of LoadableComponent + // generated by react-loadable + // to provide post-render listeners + class CustomLoadableRenderer extends LoadableRenderer { + componentDidMount() { + this.afterRender(); + } + + componentDidUpdate() { + this.afterRender(); + } + + afterRender() { + const { loaded, loading, error } = this.state; + if (!loading) { + if (error) { + this.props.onRenderFailure(error); + } else if (loaded && Object.keys(loaded).length > 0) { + this.props.onRenderSuccess(); + } + } + } + } + + CustomLoadableRenderer.defaultProps = defaultProps; + CustomLoadableRenderer.propTypes = propTypes; + CustomLoadableRenderer.preload = LoadableRenderer.preload; + + return CustomLoadableRenderer; +}