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/04/17 17:35:08 UTC
(superset) branch chore/fc-06-super-chart created (now f337950eb2f)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a change to branch chore/fc-06-super-chart
in repository https://gitbox.apache.org/repos/asf/superset.git
at f337950eb2f chore(lint): convert SuperChart and SuperChartCore to function components
This branch includes the following new commits:
new f337950eb2f chore(lint): convert SuperChart and SuperChartCore to function components
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: chore(lint): convert SuperChart and SuperChartCore to function components
Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch chore/fc-06-super-chart
in repository https://gitbox.apache.org/repos/asf/superset.git
commit f337950eb2f2d3f4547c4e6f2f838d51c3bbdcca
Author: Evan Rusackas <ev...@rusackas.com>
AuthorDate: Fri Apr 17 10:34:55 2026 -0700
chore(lint): convert SuperChart and SuperChartCore to function components
Converts SuperChart and SuperChartCore in superset-ui-core from class
components to function components. Updates associated unit tests.
Co-Authored-By: Claude Sonnet 4.6 <no...@anthropic.com>
---
.../src/chart/components/SuperChart.tsx | 429 ++++++++++++---------
.../src/chart/components/SuperChartCore.tsx | 378 ++++++++++--------
.../test/chart/components/SuperChart.test.tsx | 22 +-
.../test/chart/components/SuperChartCore.test.tsx | 28 +-
4 files changed, 495 insertions(+), 362 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChart.tsx b/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChart.tsx
index 553a98f568a..2114cf3c4d5 100644
--- a/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChart.tsx
+++ b/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChart.tsx
@@ -21,8 +21,10 @@ import {
ReactNode,
RefObject,
ComponentType,
- PureComponent,
Fragment,
+ useCallback,
+ useMemo,
+ useRef,
} from 'react';
import {
@@ -32,22 +34,19 @@ import {
} from 'react-error-boundary';
import { ParentSize } from '@visx/responsive';
import { createSelector } from 'reselect';
-import { withTheme } from '@emotion/react';
+import { useTheme } from '@emotion/react';
import { parseLength, Dimension } from '../../dimension';
import getChartMetadataRegistry from '../registries/ChartMetadataRegistrySingleton';
-import SuperChartCore, { Props as SuperChartCoreProps } from './SuperChartCore';
+import SuperChartCore, {
+ Props as SuperChartCoreProps,
+ SuperChartCoreRef,
+} from './SuperChartCore';
import DefaultFallbackComponent from './FallbackComponent';
import ChartProps, { ChartPropsConfig } from '../models/ChartProps';
import NoResultsComponent from './NoResultsComponent';
import { isMatrixifyEnabled } from '../types/matrixify';
import MatrixifyGridRenderer from './Matrixify/MatrixifyGridRenderer';
-
-const defaultProps = {
- FallbackComponent: DefaultFallbackComponent,
- height: 400 as string | number,
- width: '100%' as string | number,
- enableNoResults: true,
-};
+import { SupersetTheme } from '@apache-superset/core/ui';
export type FallbackPropsWithDimension = FallbackProps & Partial<Dimension>;
@@ -56,7 +55,9 @@ export type WrapperProps = Dimension & {
};
export type Props = Omit<SuperChartCoreProps, 'chartProps'> &
- Omit<ChartPropsConfig, 'width' | 'height'> & {
+ Omit<ChartPropsConfig, 'width' | 'height' | 'theme'> & {
+ /** Theme object (optional, falls back to ThemeProvider context) */
+ theme?: SupersetTheme;
/**
* Set this to true to disable error boundary built-in in SuperChart
* and let the error propagate to upper level
@@ -102,215 +103,261 @@ export type Props = Omit<SuperChartCoreProps, 'chartProps'> &
inContextMenu?: boolean;
};
-type PropsWithDefault = Props & Readonly<typeof defaultProps>;
-
-class SuperChart extends PureComponent<Props, {}> {
+function SuperChart({
+ id,
+ className,
+ chartType,
+ preTransformProps,
+ overrideTransformProps,
+ postTransformProps,
+ onRenderSuccess,
+ onRenderFailure,
+ disableErrorBoundary,
+ FallbackComponent = DefaultFallbackComponent,
+ onErrorBoundary,
+ Wrapper,
+ queriesData,
+ enableNoResults = true,
+ noResults,
+ theme: themeProp,
+ debounceTime,
+ height = 400,
+ width = '100%',
+ ...rest
+}: Props): JSX.Element {
/**
- * SuperChart's core
+ * SuperChart's core ref
*/
- core?: SuperChartCore | null;
+ const coreRef = useRef<SuperChartCoreRef | null>(null);
- private createChartProps = ChartProps.createSelector();
+ // Use theme from hook, falling back to prop if provided
+ const themeFromContext = useTheme() as SupersetTheme;
+ const theme = themeProp ?? themeFromContext;
- private parseDimension = createSelector(
- [
- ({ width }: { width: string | number; height: string | number }) => width,
- ({ height }) => height,
- ],
- (width, height) => {
- // Parse them in case they are % or 'auto'
- const widthInfo = parseLength(width);
- const heightInfo = parseLength(height);
- const boxHeight = heightInfo.isDynamic
- ? `${heightInfo.multiplier * 100}%`
- : heightInfo.value;
- const boxWidth = widthInfo.isDynamic
- ? `${widthInfo.multiplier * 100}%`
- : widthInfo.value;
- const style = {
- height: boxHeight,
- width: boxWidth,
- };
+ const createChartProps = useMemo(() => ChartProps.createSelector(), []);
- // bounding box will ensure that when one dimension is not dynamic
- // e.g. height = 300
- // the auto size will be bound to that value instead of being 100% by default
- // e.g. height: 300 instead of height: '100%'
- const BoundingBox =
- widthInfo.isDynamic &&
- heightInfo.isDynamic &&
- widthInfo.multiplier === 1 &&
- heightInfo.multiplier === 1
- ? Fragment
- : ({ children }: { children: ReactNode }) => (
- <div style={style}>{children}</div>
- );
+ const parseDimension = useMemo(
+ () =>
+ createSelector(
+ [
+ ({ width: w }: { width: string | number; height: string | number }) =>
+ w,
+ ({
+ height: h,
+ }: {
+ width: string | number;
+ height: string | number;
+ }) => h,
+ ],
+ (w, h) => {
+ // Parse them in case they are % or 'auto'
+ const widthInfo = parseLength(w);
+ const heightInfo = parseLength(h);
+ const boxHeight = heightInfo.isDynamic
+ ? `${heightInfo.multiplier * 100}%`
+ : heightInfo.value;
+ const boxWidth = widthInfo.isDynamic
+ ? `${widthInfo.multiplier * 100}%`
+ : widthInfo.value;
+ const style = {
+ height: boxHeight,
+ width: boxWidth,
+ };
- return { BoundingBox, heightInfo, widthInfo };
- },
+ // bounding box will ensure that when one dimension is not dynamic
+ // e.g. height = 300
+ // the auto size will be bound to that value instead of being 100% by default
+ // e.g. height: 300 instead of height: '100%'
+ const BoundingBox =
+ widthInfo.isDynamic &&
+ heightInfo.isDynamic &&
+ widthInfo.multiplier === 1 &&
+ heightInfo.multiplier === 1
+ ? Fragment
+ : ({ children }: { children: ReactNode }) => (
+ <div style={style}>{children}</div>
+ );
+
+ return { BoundingBox, heightInfo, widthInfo };
+ },
+ ),
+ [],
);
- static defaultProps = defaultProps;
+ const setRef = useCallback((core: SuperChartCoreRef | null) => {
+ coreRef.current = core;
+ }, []);
- private setRef = (core: SuperChartCore | null) => {
- this.core = core;
- };
+ const getQueryCount = useCallback(
+ () => getChartMetadataRegistry().get(chartType)?.queryObjectCount ?? 1,
+ [chartType],
+ );
- private getQueryCount = () =>
- getChartMetadataRegistry().get(this.props.chartType)?.queryObjectCount ?? 1;
+ const renderChart = useCallback(
+ (chartWidth: number, chartHeight: number) => {
+ const chartProps = createChartProps({
+ ...rest,
+ queriesData,
+ height: chartHeight,
+ width: chartWidth,
+ theme,
+ });
- renderChart(width: number, height: number) {
- const {
- id,
- className,
- chartType,
- preTransformProps,
- overrideTransformProps,
- postTransformProps,
- onRenderSuccess,
- onRenderFailure,
- disableErrorBoundary,
- FallbackComponent,
- onErrorBoundary,
- Wrapper,
- queriesData,
- enableNoResults,
- noResults,
- theme,
- ...rest
- } = this.props as PropsWithDefault;
+ // Check if Matrixify is enabled - use rawFormData (snake_case)
+ const matrixifyEnabled = isMatrixifyEnabled(chartProps.rawFormData);
- const chartProps = this.createChartProps({
- ...rest,
- queriesData,
- height,
- width,
- theme,
- });
+ if (matrixifyEnabled) {
+ // When matrixify is enabled, queriesData is expected to be empty
+ // since each cell fetches its own data via StatefulChart
+ const matrixifyChart = (
+ <MatrixifyGridRenderer
+ formData={chartProps.rawFormData}
+ datasource={chartProps.datasource}
+ width={chartWidth}
+ height={chartHeight}
+ hooks={chartProps.hooks}
+ />
+ );
- // Check if Matrixify is enabled - use rawFormData (snake_case)
- const matrixifyEnabled = isMatrixifyEnabled(chartProps.rawFormData);
+ // Apply wrapper if provided
+ const wrappedChart = Wrapper ? (
+ <Wrapper width={chartWidth} height={chartHeight}>
+ {matrixifyChart}
+ </Wrapper>
+ ) : (
+ matrixifyChart
+ );
- if (matrixifyEnabled) {
- // When matrixify is enabled, queriesData is expected to be empty
- // since each cell fetches its own data via StatefulChart
- const matrixifyChart = (
- <MatrixifyGridRenderer
- formData={chartProps.rawFormData}
- datasource={chartProps.datasource}
- width={width}
- height={height}
- hooks={chartProps.hooks}
- />
- );
+ // Include error boundary unless disabled
+ return disableErrorBoundary === true ? (
+ wrappedChart
+ ) : (
+ <ErrorBoundary
+ FallbackComponent={props => (
+ <FallbackComponent
+ width={chartWidth}
+ height={chartHeight}
+ {...props}
+ />
+ )}
+ onError={onErrorBoundary}
+ >
+ {wrappedChart}
+ </ErrorBoundary>
+ );
+ }
- // Apply wrapper if provided
- const wrappedChart = Wrapper ? (
- <Wrapper width={width} height={height}>
- {matrixifyChart}
- </Wrapper>
- ) : (
- matrixifyChart
- );
+ // Check for no results only for non-matrixified charts
+ const noResultQueries =
+ enableNoResults &&
+ (!queriesData ||
+ queriesData
+ .slice(0, getQueryCount())
+ .every(
+ ({ data }) => !data || (Array.isArray(data) && data.length === 0),
+ ));
- // Include error boundary unless disabled
+ let chart: JSX.Element;
+ if (noResultQueries) {
+ chart = noResults ? (
+ <>{noResults}</>
+ ) : (
+ <NoResultsComponent
+ id={id}
+ className={className}
+ height={chartHeight}
+ width={chartWidth}
+ />
+ );
+ } else {
+ const chartWithoutWrapper = (
+ <SuperChartCore
+ ref={setRef}
+ id={id}
+ className={className}
+ chartType={chartType}
+ chartProps={chartProps}
+ preTransformProps={preTransformProps}
+ overrideTransformProps={overrideTransformProps}
+ postTransformProps={postTransformProps}
+ onRenderSuccess={onRenderSuccess}
+ onRenderFailure={onRenderFailure}
+ />
+ );
+ chart = Wrapper ? (
+ <Wrapper width={chartWidth} height={chartHeight}>
+ {chartWithoutWrapper}
+ </Wrapper>
+ ) : (
+ chartWithoutWrapper
+ );
+ }
+ // Include the error boundary by default unless it is specifically disabled.
return disableErrorBoundary === true ? (
- wrappedChart
+ chart
) : (
<ErrorBoundary
FallbackComponent={props => (
- <FallbackComponent width={width} height={height} {...props} />
+ <FallbackComponent
+ width={chartWidth}
+ height={chartHeight}
+ {...props}
+ />
)}
onError={onErrorBoundary}
>
- {wrappedChart}
+ {chart}
</ErrorBoundary>
);
- }
+ },
+ [
+ createChartProps,
+ rest,
+ queriesData,
+ theme,
+ Wrapper,
+ disableErrorBoundary,
+ FallbackComponent,
+ onErrorBoundary,
+ enableNoResults,
+ getQueryCount,
+ noResults,
+ id,
+ className,
+ setRef,
+ chartType,
+ preTransformProps,
+ overrideTransformProps,
+ postTransformProps,
+ onRenderSuccess,
+ onRenderFailure,
+ ],
+ );
- // Check for no results only for non-matrixified charts
- const noResultQueries =
- enableNoResults &&
- (!queriesData ||
- queriesData
- .slice(0, this.getQueryCount())
- .every(
- ({ data }) => !data || (Array.isArray(data) && data.length === 0),
- ));
+ const { heightInfo, widthInfo, BoundingBox } = parseDimension({
+ width,
+ height,
+ });
- let chart;
- if (noResultQueries) {
- chart = noResults || (
- <NoResultsComponent
- id={id}
- className={className}
- height={height}
- width={width}
- />
- );
- } else {
- const chartWithoutWrapper = (
- <SuperChartCore
- ref={this.setRef}
- id={id}
- className={className}
- chartType={chartType}
- chartProps={chartProps}
- preTransformProps={preTransformProps}
- overrideTransformProps={overrideTransformProps}
- postTransformProps={postTransformProps}
- onRenderSuccess={onRenderSuccess}
- onRenderFailure={onRenderFailure}
- />
- );
- chart = Wrapper ? (
- <Wrapper width={width} height={height}>
- {chartWithoutWrapper}
- </Wrapper>
- ) : (
- chartWithoutWrapper
- );
- }
- // Include the error boundary by default unless it is specifically disabled.
- return disableErrorBoundary === true ? (
- chart
- ) : (
- <ErrorBoundary
- FallbackComponent={props => (
- <FallbackComponent width={width} height={height} {...props} />
- )}
- onError={onErrorBoundary}
- >
- {chart}
- </ErrorBoundary>
+ // If any of the dimension is dynamic, get parent's dimension
+ if (widthInfo.isDynamic || heightInfo.isDynamic) {
+ return (
+ <BoundingBox>
+ <ParentSize debounceTime={debounceTime}>
+ {({ width: parentWidth, height: parentHeight }) =>
+ renderChart(
+ widthInfo.isDynamic ? Math.floor(parentWidth) : widthInfo.value,
+ heightInfo.isDynamic
+ ? Math.floor(parentHeight)
+ : heightInfo.value,
+ )
+ }
+ </ParentSize>
+ </BoundingBox>
);
}
- render() {
- const { heightInfo, widthInfo, BoundingBox } = this.parseDimension(
- this.props as PropsWithDefault,
- );
-
- // If any of the dimension is dynamic, get parent's dimension
- if (widthInfo.isDynamic || heightInfo.isDynamic) {
- const { debounceTime } = this.props;
-
- return (
- <BoundingBox>
- <ParentSize debounceTime={debounceTime}>
- {({ width, height }) =>
- this.renderChart(
- widthInfo.isDynamic ? Math.floor(width) : widthInfo.value,
- heightInfo.isDynamic ? Math.floor(height) : heightInfo.value,
- )
- }
- </ParentSize>
- </BoundingBox>
- );
- }
-
- return this.renderChart(widthInfo.value, heightInfo.value);
- }
+ return renderChart(widthInfo.value, heightInfo.value);
}
-export default withTheme(SuperChart);
+export default SuperChart;
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 844dbae7c6a..c999d1d3347 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
@@ -17,9 +17,14 @@
* under the License.
*/
-/* eslint-disable react/jsx-sort-default-props */
-import { PureComponent } from 'react';
-import { t } from '@apache-superset/core/translation';
+import {
+ forwardRef,
+ useCallback,
+ useImperativeHandle,
+ useMemo,
+ useRef,
+} from 'react';
+import { t } from '@apache-superset/core';
import { createSelector } from 'reselect';
import getChartComponentRegistry from '../registries/ChartComponentRegistrySingleton';
import getChartTransformPropsRegistry from '../registries/ChartTransformPropsRegistrySingleton';
@@ -39,16 +44,6 @@ function IDENTITY<T>(x: T) {
const EMPTY = () => null;
-const defaultProps = {
- id: '',
- className: '',
- preTransformProps: IDENTITY,
- overrideTransformProps: undefined,
- postTransformProps: IDENTITY,
- onRenderSuccess() {},
- onRenderFailure() {},
-};
-
interface LoadingProps {
error: { toString(): string };
}
@@ -78,174 +73,231 @@ export type Props = {
onRenderFailure?: HandlerFunction;
};
-export default class SuperChartCore extends PureComponent<Props, {}> {
- /**
- * The HTML element that wraps all chart content
- */
- container?: HTMLElement | null;
+export interface SuperChartCoreRef {
+ container: HTMLElement | null;
+}
- /**
- * memoized function so it will not recompute and return previous value
- * unless one of
- * - preTransformProps
- * - chartProps
- * is changed.
- */
- preSelector = createSelector(
- [
- (input: {
- chartProps: ChartProps;
- preTransformProps?: PreTransformProps;
- }) => input.chartProps,
- input => input.preTransformProps,
- ],
- (chartProps, pre = IDENTITY) => pre(chartProps),
- );
+const SuperChartCore = forwardRef<SuperChartCoreRef, Props>(
+ function SuperChartCore(
+ {
+ id = '',
+ className = '',
+ chartProps = BLANK_CHART_PROPS,
+ chartType,
+ preTransformProps = IDENTITY,
+ overrideTransformProps,
+ postTransformProps = IDENTITY,
+ onRenderSuccess = () => {},
+ onRenderFailure = () => {},
+ },
+ ref,
+ ) {
+ const containerRef = useRef<HTMLElement | null>(null);
- /**
- * 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),
- );
+ // Expose container via ref
+ useImperativeHandle(
+ ref,
+ () => ({
+ get container() {
+ return containerRef.current;
+ },
+ }),
+ [],
+ );
- /**
- * 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,
- ],
- (transformedChartProps, post = IDENTITY) => post(transformedChartProps),
- );
+ /**
+ * memoized function so it will not recompute and return previous value
+ * unless one of
+ * - preTransformProps
+ * - chartProps
+ * is changed.
+ */
+ const preSelector = useMemo(
+ () =>
+ createSelector(
+ [
+ (input: {
+ chartProps: ChartProps;
+ preTransformProps?: PreTransformProps;
+ }) => input.chartProps,
+ input => input.preTransformProps,
+ ],
+ (inputChartProps, pre = IDENTITY) => pre(inputChartProps),
+ ),
+ [],
+ );
- /**
- * 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
+ * unless one of the input arguments have changed.
+ */
+ const transformSelector = useMemo(
+ () =>
+ 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
- * - chartType
- * - overrideTransformProps
- * is changed.
- */
- private createLoadableRenderer = createSelector(
- [
- (input: { chartType: string; overrideTransformProps?: TransformProps }) =>
- input.chartType,
- input => input.overrideTransformProps,
- ],
- (chartType, overrideTransformProps) => {
- if (chartType) {
- const Renderer = createLoadableRenderer({
- loader: {
- Chart: () => getChartComponentRegistry().getAsPromise(chartType),
- transformProps: overrideTransformProps
- ? () => Promise.resolve(overrideTransformProps)
- : () => getChartTransformPropsRegistry().getAsPromise(chartType),
- },
- loading: (loadingProps: LoadingProps) =>
- this.renderLoading(loadingProps, chartType),
- render: this.renderChart,
- });
+ /**
+ * memoized function so it will not recompute and return previous value
+ * unless one of the input arguments have changed.
+ */
+ const postSelector = useMemo(
+ () =>
+ createSelector(
+ [
+ (input: {
+ chartProps: ChartProps;
+ postTransformProps?: PostTransformProps;
+ }) => input.chartProps,
+ input => input.postTransformProps,
+ ],
+ (transformedChartProps, post = IDENTITY) =>
+ post(transformedChartProps),
+ ),
+ [],
+ );
- // Trigger preloading.
- Renderer.preload();
+ /**
+ * Using each memoized function to retrieve the computed chartProps
+ */
+ const processChartProps = useCallback(
+ ({
+ chartProps: inputChartProps,
+ preTransformProps: pre,
+ transformProps,
+ postTransformProps: post,
+ }: {
+ chartProps: ChartProps;
+ preTransformProps?: PreTransformProps;
+ transformProps?: TransformProps;
+ postTransformProps?: PostTransformProps;
+ }) =>
+ postSelector({
+ chartProps: transformSelector({
+ chartProps: preSelector({
+ chartProps: inputChartProps,
+ preTransformProps: pre,
+ }),
+ transformProps,
+ }),
+ postTransformProps: post,
+ }),
+ [preSelector, transformSelector, postSelector],
+ );
- return Renderer;
- }
+ const renderLoading = useCallback(
+ (loadingProps: LoadingProps, loadingChartType: string) => {
+ const { error } = loadingProps;
- return EMPTY;
- },
- );
+ if (error) {
+ return (
+ <div className="alert alert-warning" role="alert">
+ <strong>{t('ERROR')}</strong>
+ <code>chartType="{loadingChartType}"</code> —
+ {error.toString()}
+ </div>
+ );
+ }
- static defaultProps = defaultProps;
+ return null;
+ },
+ [],
+ );
- private renderChart = (loaded: LoadedModules, props: RenderProps) => {
- const { Chart, transformProps } = loaded;
- const { chartProps, preTransformProps, postTransformProps } = props;
+ const renderChart = useCallback(
+ (loaded: LoadedModules, props: RenderProps) => {
+ const { Chart, transformProps } = loaded;
+ const {
+ chartProps: renderChartProps,
+ preTransformProps: pre,
+ postTransformProps: post,
+ } = props;
- return (
- <Chart
- {...this.processChartProps({
- chartProps,
- preTransformProps,
- transformProps,
- postTransformProps,
- })}
- />
+ return (
+ <Chart
+ {...processChartProps({
+ chartProps: renderChartProps,
+ preTransformProps: pre,
+ transformProps,
+ postTransformProps: post,
+ })}
+ />
+ );
+ },
+ [processChartProps],
);
- };
- private renderLoading = (loadingProps: LoadingProps, chartType: string) => {
- const { error } = loadingProps;
+ /**
+ * memoized function so it will not recompute
+ * and return previous value
+ * unless one of
+ * - chartType
+ * - overrideTransformProps
+ * is changed.
+ */
+ const createLoadableRendererSelector = useMemo(
+ () =>
+ createSelector(
+ [
+ (input: {
+ chartType: string;
+ overrideTransformProps?: TransformProps;
+ }) => input.chartType,
+ input => input.overrideTransformProps,
+ ],
+ (selectorChartType, selectorOverrideTransformProps) => {
+ if (selectorChartType) {
+ const Renderer = createLoadableRenderer({
+ loader: {
+ Chart: () =>
+ getChartComponentRegistry().getAsPromise(selectorChartType),
+ transformProps: selectorOverrideTransformProps
+ ? () => Promise.resolve(selectorOverrideTransformProps)
+ : () =>
+ getChartTransformPropsRegistry().getAsPromise(
+ selectorChartType,
+ ),
+ },
+ loading: (loadingProps: LoadingProps) =>
+ renderLoading(loadingProps, selectorChartType),
+ render: renderChart,
+ });
- if (error) {
- return (
- <div className="alert alert-warning" role="alert">
- <strong>{t('ERROR')}</strong>
- <code>chartType="{chartType}"</code> —
- {error.toString()}
- </div>
- );
- }
+ // Trigger preloading.
+ Renderer.preload();
- return null;
- };
+ return Renderer;
+ }
- private setRef = (container: HTMLElement | null) => {
- this.container = container;
- };
+ return EMPTY;
+ },
+ ),
+ [renderLoading, renderChart],
+ );
- render() {
- const {
- id,
- className,
- preTransformProps,
- postTransformProps,
- chartProps = BLANK_CHART_PROPS,
- onRenderSuccess,
- onRenderFailure,
- } = this.props;
+ const setRef = useCallback((container: HTMLElement | null) => {
+ containerRef.current = container;
+ }, []);
// Create LoadableRenderer and start preloading
// the lazy-loaded Chart components
- const Renderer = this.createLoadableRenderer(this.props);
+ const Renderer = createLoadableRendererSelector({
+ chartType,
+ overrideTransformProps,
+ });
// Do not render if chartProps is set to null.
- // but the pre-loading has been started in this.createLoadableRenderer
+ // but the pre-loading has been started in createLoadableRendererSelector
// to prepare for rendering once chartProps becomes available.
if (chartProps === null) {
return null;
@@ -263,7 +315,7 @@ export default class SuperChartCore extends PureComponent<Props, {}> {
}
return (
- <div {...containerProps} ref={this.setRef}>
+ <div {...containerProps} ref={setRef}>
<Renderer
preTransformProps={preTransformProps}
postTransformProps={postTransformProps}
@@ -273,5 +325,7 @@ export default class SuperChartCore extends PureComponent<Props, {}> {
/>
</div>
);
- }
-}
+ },
+);
+
+export default SuperChartCore;
diff --git a/superset-frontend/packages/superset-ui-core/test/chart/components/SuperChart.test.tsx b/superset-frontend/packages/superset-ui-core/test/chart/components/SuperChart.test.tsx
index 86203ed8643..e42accaee48 100644
--- a/superset-frontend/packages/superset-ui-core/test/chart/components/SuperChart.test.tsx
+++ b/superset-frontend/packages/superset-ui-core/test/chart/components/SuperChart.test.tsx
@@ -24,6 +24,7 @@ import { triggerResizeObserver } from 'resize-observer-polyfill';
import { ErrorBoundary } from 'react-error-boundary';
import { promiseTimeout, SuperChart } from '@superset-ui/core';
+import { supersetTheme } from '@apache-superset/core/ui';
import { WrapperProps } from '../../../src/chart/components/SuperChart';
import {
@@ -118,6 +119,7 @@ describe('SuperChart', () => {
queriesData={[DEFAULT_QUERY_DATA]}
width="200"
height="200"
+ theme={supersetTheme}
/>,
);
@@ -138,6 +140,7 @@ describe('SuperChart', () => {
queriesData={[DEFAULT_QUERY_DATA]}
width="200"
height="200"
+ theme={supersetTheme}
FallbackComponent={CustomFallbackComponent}
/>,
);
@@ -154,6 +157,7 @@ describe('SuperChart', () => {
queriesData={[DEFAULT_QUERY_DATA]}
width="200"
height="200"
+ theme={supersetTheme}
onErrorBoundary={handleError}
/>,
);
@@ -178,6 +182,7 @@ describe('SuperChart', () => {
queriesData={[DEFAULT_QUERY_DATA]}
width="200"
height="200"
+ theme={supersetTheme}
onErrorBoundary={inactiveErrorHandler}
/>
</ErrorBoundary>,
@@ -205,6 +210,7 @@ describe('SuperChart', () => {
queriesData={[DEFAULT_QUERY_DATA]}
width={101}
height={118}
+ theme={supersetTheme}
formData={{ abc: 1 }}
/>,
);
@@ -285,6 +291,7 @@ describe('SuperChart', () => {
debounceTime={1}
width="100%"
height="100%"
+ theme={supersetTheme}
/>,
);
@@ -332,6 +339,7 @@ describe('SuperChart', () => {
queriesData={DEFAULT_QUERIES_DATA}
width={101}
height={118}
+ theme={supersetTheme}
formData={{ abc: 1 }}
/>,
);
@@ -347,7 +355,12 @@ describe('SuperChart', () => {
describe('supports NoResultsComponent', () => {
test('renders NoResultsComponent when queriesData is missing', () => {
render(
- <SuperChart chartType={ChartKeys.DILIGENT} width="200" height="200" />,
+ <SuperChart
+ chartType={ChartKeys.DILIGENT}
+ width="200"
+ height="200"
+ theme={supersetTheme}
+ />,
);
expect(screen.getByText('No Results')).toBeInTheDocument();
@@ -360,6 +373,7 @@ describe('SuperChart', () => {
queriesData={[{ data: null }]}
width="200"
height="200"
+ theme={supersetTheme}
/>,
);
@@ -387,6 +401,7 @@ describe('SuperChart', () => {
queriesData={[DEFAULT_QUERY_DATA]}
width={100}
height={100}
+ theme={supersetTheme}
/>,
);
@@ -411,6 +426,7 @@ describe('SuperChart', () => {
debounceTime={1}
width="100%"
height="100%"
+ theme={supersetTheme}
Wrapper={MyWrapper}
/>
</div>,
@@ -475,6 +491,7 @@ describe('SuperChart', () => {
chartType={ChartKeys.DILIGENT}
width="200"
height="200"
+ theme={supersetTheme}
queriesData={[{ data: [] }]}
enableNoResults
/>,
@@ -500,6 +517,7 @@ describe('SuperChart', () => {
chartType={ChartKeys.DILIGENT}
width="200"
height="200"
+ theme={supersetTheme}
queriesData={[{ data: null }]}
enableNoResults
/>,
@@ -527,6 +545,7 @@ describe('SuperChart', () => {
chartType={ChartKeys.DILIGENT}
width="200"
height="200"
+ theme={supersetTheme}
queriesData={[{ data: [] }]}
enableNoResults
noResults={<CustomNoResults />}
@@ -556,6 +575,7 @@ describe('SuperChart', () => {
chartType={ChartKeys.DILIGENT}
width="200"
height="200"
+ theme={supersetTheme}
queriesData={[{ data: [] }]}
enableNoResults
onErrorBoundary={onErrorBoundary}
diff --git a/superset-frontend/packages/superset-ui-core/test/chart/components/SuperChartCore.test.tsx b/superset-frontend/packages/superset-ui-core/test/chart/components/SuperChartCore.test.tsx
index 0ec17ed6f2f..809ff90564c 100644
--- a/superset-frontend/packages/superset-ui-core/test/chart/components/SuperChartCore.test.tsx
+++ b/superset-frontend/packages/superset-ui-core/test/chart/components/SuperChartCore.test.tsx
@@ -20,7 +20,7 @@
import '@testing-library/jest-dom';
import mockConsole, { RestoreConsole } from 'jest-mock-console';
import { ChartProps } from '@superset-ui/core';
-import { supersetTheme } from '@apache-superset/core/theme';
+import { supersetTheme } from '@apache-superset/core/ui';
import { render, screen, waitFor } from '@superset-ui/core/spec';
import SuperChartCore from '../../../src/chart/components/SuperChartCore';
import {
@@ -227,15 +227,27 @@ describe('SuperChartCore', () => {
});
});
- describe('.processChartProps()', () => {
- test('use identity functions for unspecified transforms', () => {
- const chart = new SuperChartCore({
- chartType: ChartKeys.DILIGENT,
+ describe('processChartProps behavior', () => {
+ test('passes through chartProps unchanged when no transforms are specified', async () => {
+ // When no pre/post transform props are specified, the identity function is used
+ // which means chartProps should pass through to the chart unchanged.
+ // We verify this by checking that the chart renders correctly without transforms.
+ const chartProps2 = new ChartProps({
+ queriesData: [{ message: 'identity-test' }],
+ theme: supersetTheme,
});
- const chartProps2 = new ChartProps();
- expect(chart.processChartProps({ chartProps: chartProps2 })).toBe(
- chartProps2,
+
+ render(
+ <SuperChartCore
+ chartType={ChartKeys.DILIGENT}
+ chartProps={chartProps2}
+ overrideTransformProps={props => props.queriesData[0]}
+ />,
);
+
+ await waitFor(() => {
+ expect(screen.getByText('identity-test')).toBeInTheDocument();
+ });
});
});
});