fix(ui-core): Invalid postTransform process (#34874)

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 = () => <span>test</span>;
+
+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(
+    <SuperChartCore
+      {...props}
+      preTransformProps={pre}
+      overrideTransformProps={transform}
+      postTransformProps={post}
+    />,
+  );
+
+  await waitFor(() => expect(pre).toHaveBeenCalledTimes(1));
+  expect(transform).toHaveBeenCalledTimes(1);
+  expect(post).toHaveBeenCalledTimes(1);
+
+  const updatedPost = jest.fn(x => x);
+
+  rerender(
+    <SuperChartCore
+      {...props}
+      preTransformProps={pre}
+      overrideTransformProps={transform}
+      postTransformProps={updatedPost}
+    />,
+  );
+  await waitFor(() => expect(updatedPost).toHaveBeenCalledTimes(1));
+  expect(transform).toHaveBeenCalledTimes(1);
+  expect(pre).toHaveBeenCalledTimes(1);
+});

superset-frontend/packages/superset-ui-core/src/chart/components/SuperChartCore.tsx

@@ -85,31 +85,74 @@ export default class SuperChartCore extends PureComponent<Props, {}> {
   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

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()
       );
     }

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 }) => (
-    <div data-test="mock-super-chart">{JSON.stringify(formData)}</div>
+  SuperChart: ({ postTransformProps = x => x, ...props }) => (
+    <div data-test="mock-super-chart">
+      {JSON.stringify(postTransformProps(props).formData)}
+    </div>
   ),
 }));
 
@@ -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(<ChartRenderer {...initialProps} />);
+  const updatedProps = {
+    ...initialProps,
+    postTransformProps,
+  };
+  expect(postTransformProps).toHaveBeenCalledTimes(0);
+  rerender(<ChartRenderer {...updatedProps} />);
+  expect(postTransformProps).toHaveBeenCalledTimes(1);
+});
+
 test('should identify matrixify property changes correctly', () => {
   // Test that formData with different matrixify properties triggers updates
   const initialProps = {