You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by kr...@apache.org on 2018/11/29 19:09:15 UTC
[incubator-superset] branch master updated: [READY] Update
SuperChart onRenderXXX listeners (#6376)
This is an automated email from the ASF dual-hosted git repository.
kristw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new e715cdb [READY] Update SuperChart onRenderXXX listeners (#6376)
e715cdb is described below
commit e715cdb9ac3b1c2ea6c0802a3ce57f9ebcb971c5
Author: Krist Wongsuphasawat <kr...@gmail.com>
AuthorDate: Thu Nov 29 11:09:08 2018 -0800
[READY] Update SuperChart onRenderXXX listeners (#6376)
* Revise LoadableRenderer (+3 squashed commits)
Squashed commits:
[f1614c7c] extract createLoadableRenderer into a separate function
[8689bc94] extend LoadableRenderer
[3d04ff2b] remove skipRendering
* add number of times function was called to the test
---
.../core/createLoadableRenderer_spec.jsx | 94 ++++++++++++++++++++++
superset/assets/src/chart/Chart.jsx | 1 -
.../visualizations/core/components/SuperChart.jsx | 38 +++++----
.../core/components/createLoadableRenderer.js | 46 +++++++++++
4 files changed, 162 insertions(+), 17 deletions(-)
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 0000000..f61d909
--- /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 (
+ <div className="test-component">test</div>
+ );
+ }
+ let loadChartSuccess;
+ let render;
+ let loading;
+ let LoadableRenderer;
+
+ beforeEach(() => {
+ loadChartSuccess = jest.fn(() => Promise.resolve(TestComponent));
+ render = jest.fn((loaded) => {
+ const { Chart } = loaded;
+ return (<Chart />);
+ });
+ loading = jest.fn(() => (<div>Loading</div>));
+ 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(
+ <LoadableRenderer
+ onRenderSuccess={onRenderSuccess}
+ onRenderFailure={onRenderFailure}
+ />,
+ );
+ 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(
+ <FailedRenderer
+ onRenderSuccess={onRenderSuccess}
+ onRenderFailure={onRenderFailure}
+ />,
+ );
+ 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(<LoadableRenderer />);
+ // 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 77f91b1..f5aa347 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}
/>
</div>
</ErrorBoundary>
diff --git a/superset/assets/src/visualizations/core/components/SuperChart.jsx b/superset/assets/src/visualizations/core/components/SuperChart.jsx
index 4361b1a..e4fad94 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 (
<Chart
{...this.processChartProps({
preTransformProps,
@@ -101,23 +105,19 @@ class SuperChart extends React.PureComponent {
})}
/>
);
- setTimeout(() => this.props.onRenderSuccess(), 0);
- return result;
}
- renderLoading(loadableProps, chartType) {
- const { error } = loadableProps;
+ renderLoading(loadingProps, chartType) {
+ const { error } = loadingProps;
if (error) {
- const result = (
+ return (
<div className="alert alert-warning" role="alert">
<strong>ERROR</strong>
<code>chartType="{chartType}"</code> —
{JSON.stringify(error)}
</div>
);
- 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}
/>
)}
</div>
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 0000000..5638473
--- /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;
+}