You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/05/11 16:55:34 UTC

(superset) branch master updated: chore(lint): convert ChartDataProvider and StatefulChart to function components (#39456)

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 2fe6269c22b chore(lint): convert ChartDataProvider and StatefulChart to function components (#39456)
2fe6269c22b is described below

commit 2fe6269c22b5646727c56876a241e1c8f95ee5e4
Author: Evan Rusackas <ev...@preset.io>
AuthorDate: Mon May 11 09:55:27 2026 -0700

    chore(lint): convert ChartDataProvider and StatefulChart to function components (#39456)
    
    Co-authored-by: Claude Sonnet 4.6 <no...@anthropic.com>
---
 .../src/chart/components/ChartDataProvider.tsx     | 204 +++++++++++----------
 1 file changed, 106 insertions(+), 98 deletions(-)

diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/ChartDataProvider.tsx b/superset-frontend/packages/superset-ui-core/src/chart/components/ChartDataProvider.tsx
index 457bdd9fefb..541bf19b9e4 100644
--- a/superset-frontend/packages/superset-ui-core/src/chart/components/ChartDataProvider.tsx
+++ b/superset-frontend/packages/superset-ui-core/src/chart/components/ChartDataProvider.tsx
@@ -17,8 +17,7 @@
  * under the License.
  */
 
-/* eslint react/sort-comp: 'off' */
-import { PureComponent, ReactNode } from 'react';
+import { ReactNode, useCallback, useEffect, useMemo, useState } from 'react';
 import {
   SupersetClientInterface,
   RequestConfig,
@@ -67,103 +66,112 @@ export type ChartDataProviderState = {
   error?: ProvidedProps['error'];
 };
 
-class ChartDataProvider extends PureComponent<
-  ChartDataProviderProps,
-  ChartDataProviderState
-> {
-  readonly chartClient: ChartClient;
-
-  constructor(props: ChartDataProviderProps) {
-    super(props);
-    this.state = { status: 'uninitialized' };
-    this.chartClient = new ChartClient({ client: props.client });
-  }
-
-  componentDidMount() {
-    this.handleFetchData();
-  }
-
-  componentDidUpdate(prevProps: ChartDataProviderProps) {
-    const { formData, sliceId } = this.props;
-    if (formData !== prevProps.formData || sliceId !== prevProps.sliceId) {
-      this.handleFetchData();
-    }
-  }
-
-  private extractSliceIdAndFormData() {
-    const { formData, sliceId } = this.props;
-    return formData ? { formData } : { sliceId: sliceId as number };
-  }
-
-  private handleFetchData = () => {
-    const {
-      loadDatasource,
-      formDataRequestOptions,
-      datasourceRequestOptions,
-      queryRequestOptions,
-    } = this.props;
-
-    this.setState({ status: 'loading' }, () => {
-      try {
-        this.chartClient
-          .loadFormData(
-            this.extractSliceIdAndFormData(),
-            formDataRequestOptions,
-          )
-          .then(formData =>
-            Promise.all([
-              loadDatasource
-                ? this.chartClient.loadDatasource(
-                    formData.datasource,
-                    datasourceRequestOptions,
-                  )
-                : Promise.resolve(undefined),
-              this.chartClient.loadQueryData(formData, queryRequestOptions),
-            ]).then(
-              ([datasource, queriesData]) =>
-                // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
-                ({
-                  datasource,
-                  formData,
-                  queriesData,
-                }) as Payload,
-            ),
-          )
-          .then(this.handleReceiveData)
-          .catch(this.handleError);
-      } catch (error) {
-        this.handleError(error as Error);
-      }
-    });
-  };
-
-  private handleReceiveData = (payload?: Payload) => {
-    const { onLoaded } = this.props;
-    if (onLoaded) onLoaded(payload);
-    this.setState({ payload, status: 'loaded' });
-  };
-
-  private handleError = (error: ProvidedProps['error']) => {
-    const { onError } = this.props;
-    if (onError) onError(error);
-    this.setState({ error, status: 'error' });
-  };
-
-  render() {
-    const { children } = this.props;
-    const { status, payload, error } = this.state;
-
-    switch (status) {
-      case 'loading':
-        return children({ loading: true });
-      case 'loaded':
-        return children({ payload });
-      case 'error':
-        return children({ error });
-      case 'uninitialized':
-      default:
-        return null;
+function ChartDataProvider({
+  children,
+  client,
+  formData,
+  sliceId,
+  loadDatasource,
+  onError,
+  onLoaded,
+  formDataRequestOptions,
+  datasourceRequestOptions,
+  queryRequestOptions,
+}: ChartDataProviderProps): JSX.Element | null {
+  const [state, setState] = useState<ChartDataProviderState>({
+    status: 'uninitialized',
+  });
+
+  const chartClient = useMemo(() => new ChartClient({ client }), [client]);
+
+  const extractSliceIdAndFormData = useCallback(
+    (): SliceIdAndOrFormData =>
+      formData ? { formData } : { sliceId: sliceId as number },
+    [formData, sliceId],
+  );
+
+  const handleReceiveData = useCallback(
+    (payload?: Payload) => {
+      if (onLoaded) onLoaded(payload);
+      setState({ payload, status: 'loaded' });
+    },
+    [onLoaded],
+  );
+
+  const handleError = useCallback(
+    (error: ProvidedProps['error']) => {
+      if (onError) onError(error);
+      setState({ error, status: 'error' });
+    },
+    [onError],
+  );
+
+  const handleFetchData = useCallback(() => {
+    setState({ status: 'loading' });
+    try {
+      chartClient
+        .loadFormData(extractSliceIdAndFormData(), formDataRequestOptions)
+        .then(loadedFormData =>
+          Promise.all([
+            loadDatasource
+              ? chartClient.loadDatasource(
+                  loadedFormData.datasource,
+                  datasourceRequestOptions,
+                )
+              : Promise.resolve(undefined),
+            chartClient.loadQueryData(loadedFormData, queryRequestOptions),
+          ]).then(
+            ([datasource, queriesData]) =>
+              ({
+                datasource,
+                formData: loadedFormData,
+                queriesData,
+              }) as Payload,
+          ),
+        )
+        .then(handleReceiveData)
+        .catch(handleError);
+    } catch (error) {
+      handleError(error as Error);
     }
+  }, [
+    chartClient,
+    extractSliceIdAndFormData,
+    formDataRequestOptions,
+    loadDatasource,
+    datasourceRequestOptions,
+    queryRequestOptions,
+    handleReceiveData,
+    handleError,
+  ]);
+
+  // Fetch on mount and only refetch when formData or sliceId changes.
+  // This preserves the original class component's componentDidUpdate
+  // semantics (which compared only formData and sliceId). Other
+  // fetch-related inputs referenced by handleFetchData (callbacks and
+  // request option props) are intentionally excluded from the dependency
+  // array, so the exhaustive-deps rule is suppressed here.
+  useEffect(() => {
+    handleFetchData();
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [formData, sliceId]);
+
+  const { status, payload, error } = state;
+
+  // Wrap the children result in a Fragment so the component's return type
+  // stays `JSX.Element | null` (which TypeScript requires for JSX components)
+  // while still letting consumers return any ReactNode (strings, fragments,
+  // arrays, null, etc.) from the render prop.
+  switch (status) {
+    case 'loading':
+      return <>{children({ loading: true })}</>;
+    case 'loaded':
+      return <>{children({ payload })}</>;
+    case 'error':
+      return <>{children({ error })}</>;
+    case 'uninitialized':
+    default:
+      return null;
   }
 }