diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChartCore.test.tsx b/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChartCore.test.tsx new file mode 100644 index 000000000000..4a77e184d5f4 --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChartCore.test.tsx @@ -0,0 +1,90 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { render, waitFor } from '@testing-library/react'; + +import { + ChartPlugin, + ChartMetadata, + DatasourceType, + getChartComponentRegistry, +} from '@superset-ui/core'; + +import SuperChartCore from './SuperChartCore'; + +const props = { + chartType: 'line', +}; +const FakeChart = () => test; + +beforeEach(() => { + const metadata = new ChartMetadata({ + name: 'test-chart', + thumbnail: '', + }); + const buildQuery = () => ({ + datasource: { id: 1, type: DatasourceType.Table }, + queries: [{ granularity: 'day' }], + force: false, + result_format: 'json', + result_type: 'full', + }); + const controlPanel = { abc: 1 }; + const plugin = new ChartPlugin({ + metadata, + Chart: FakeChart, + buildQuery, + controlPanel, + }); + plugin.configure({ key: props.chartType }).register(); +}); + +test('should return the result from cache unless transformProps has changed', async () => { + const pre = jest.fn(x => x); + const transform = jest.fn(x => x); + const post = jest.fn(x => x); + expect(getChartComponentRegistry().get(props.chartType)).toBe(FakeChart); + + expect(pre).toHaveBeenCalledTimes(0); + const { rerender } = render( + , + ); + + await waitFor(() => expect(pre).toHaveBeenCalledTimes(1)); + expect(transform).toHaveBeenCalledTimes(1); + expect(post).toHaveBeenCalledTimes(1); + + const updatedPost = jest.fn(x => x); + + rerender( + , + ); + await waitFor(() => expect(updatedPost).toHaveBeenCalledTimes(1)); + expect(transform).toHaveBeenCalledTimes(1); + expect(pre).toHaveBeenCalledTimes(1); +}); diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChartCore.tsx b/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChartCore.tsx index f980ed13cf05..3f847ea4b572 100644 --- a/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChartCore.tsx +++ b/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChartCore.tsx @@ -85,31 +85,74 @@ export default class SuperChartCore extends PureComponent { container?: HTMLElement | null; /** - * memoized function so it will not recompute - * and return previous value + * memoized function so it will not recompute and return previous value * unless one of * - preTransformProps - * - transformProps - * - postTransformProps * - chartProps * is changed. */ - processChartProps = createSelector( + preSelector = createSelector( [ (input: { chartProps: ChartProps; preTransformProps?: PreTransformProps; - transformProps?: TransformProps; - postTransformProps?: PostTransformProps; }) => input.chartProps, input => input.preTransformProps, + ], + (chartProps, pre = IDENTITY) => pre(chartProps), + ); + + /** + * memoized function so it will not recompute and return previous value + * unless one of the input arguments have changed. + */ + transformSelector = createSelector( + [ + (input: { chartProps: ChartProps; transformProps?: TransformProps }) => + input.chartProps, input => input.transformProps, + ], + (preprocessedChartProps, transform = IDENTITY) => + transform(preprocessedChartProps), + ); + + /** + * memoized function so it will not recompute and return previous value + * unless one of the input arguments have changed. + */ + postSelector = createSelector( + [ + (input: { + chartProps: ChartProps; + postTransformProps?: PostTransformProps; + }) => input.chartProps, input => input.postTransformProps, ], - (chartProps, pre = IDENTITY, transform = IDENTITY, post = IDENTITY) => - post(transform(pre(chartProps))), + (transformedChartProps, post = IDENTITY) => post(transformedChartProps), ); + /** + * Using each memoized function to retrieve the computed chartProps + */ + processChartProps = ({ + chartProps, + preTransformProps, + transformProps, + postTransformProps, + }: { + chartProps: ChartProps; + preTransformProps?: PreTransformProps; + transformProps?: TransformProps; + postTransformProps?: PostTransformProps; + }) => + this.postSelector({ + chartProps: this.transformSelector({ + chartProps: this.preSelector({ chartProps, preTransformProps }), + transformProps, + }), + postTransformProps, + }); + /** * memoized function so it will not recompute * and return previous value diff --git a/superset-frontend/src/components/Chart/ChartRenderer.jsx b/superset-frontend/src/components/Chart/ChartRenderer.jsx index c3e2f3a8e0ee..d10d7b0439a9 100644 --- a/superset-frontend/src/components/Chart/ChartRenderer.jsx +++ b/superset-frontend/src/components/Chart/ChartRenderer.jsx @@ -182,6 +182,7 @@ class ChartRenderer extends Component { this.props.formData.subcategories || nextProps.cacheBusterProp !== this.props.cacheBusterProp || nextProps.emitCrossFilters !== this.props.emitCrossFilters || + nextProps.postTransformProps !== this.props.postTransformProps || hasMatrixifyChanges() ); } diff --git a/superset-frontend/src/components/Chart/ChartRenderer.test.jsx b/superset-frontend/src/components/Chart/ChartRenderer.test.jsx index 90442564b596..c5734fdcce05 100644 --- a/superset-frontend/src/components/Chart/ChartRenderer.test.jsx +++ b/superset-frontend/src/components/Chart/ChartRenderer.test.jsx @@ -27,8 +27,10 @@ import { ChartSource } from 'src/types/ChartSource'; jest.mock('@superset-ui/core', () => ({ ...jest.requireActual('@superset-ui/core'), - SuperChart: ({ formData }) => ( -
{JSON.stringify(formData)}
+ SuperChart: ({ postTransformProps = x => x, ...props }) => ( +
+ {JSON.stringify(postTransformProps(props).formData)} +
), })); @@ -119,6 +121,23 @@ test('should detect changes in matrixify properties', () => { }); }); +test('should detect changes in postTransformProps', () => { + const postTransformProps = jest.fn(x => x); + const initialProps = { + ...requiredProps, + queriesResponse: [{ data: 'initial' }], + chartStatus: 'success', + }; + const { rerender } = render(); + const updatedProps = { + ...initialProps, + postTransformProps, + }; + expect(postTransformProps).toHaveBeenCalledTimes(0); + rerender(); + expect(postTransformProps).toHaveBeenCalledTimes(1); +}); + test('should identify matrixify property changes correctly', () => { // Test that formData with different matrixify properties triggers updates const initialProps = {