You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ms...@apache.org on 2026/05/12 14:41:23 UTC
(superset) branch msyavuz/fix/chart-rendering-strict created (now 48461f04396)
This is an automated email from the ASF dual-hosted git repository.
msyavuz pushed a change to branch msyavuz/fix/chart-rendering-strict
in repository https://gitbox.apache.org/repos/asf/superset.git
at 48461f04396 fix: chart rendering
This branch includes the following new commits:
new 48461f04396 fix: chart rendering
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: fix: chart rendering
Posted by ms...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
msyavuz pushed a commit to branch msyavuz/fix/chart-rendering-strict
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 48461f04396ac59cb11b8601f6b1df2b1749b65e
Author: Mehmet Salih Yavuz <sa...@proton.me>
AuthorDate: Tue May 12 17:40:42 2026 +0300
fix: chart rendering
---
.../src/chart/components/createLoadableRenderer.ts | 130 ++++++++++++++-------
.../src/components/Chart/chartAction.ts | 20 ++++
superset-frontend/src/embedded/index.tsx | 8 +-
superset-frontend/src/views/index.tsx | 7 +-
superset-frontend/src/views/menu.tsx | 39 +++----
5 files changed, 131 insertions(+), 73 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/createLoadableRenderer.ts b/superset-frontend/packages/superset-ui-core/src/chart/components/createLoadableRenderer.ts
index 535d0129748..c96349eb709 100644
--- a/superset-frontend/packages/superset-ui-core/src/chart/components/createLoadableRenderer.ts
+++ b/superset-frontend/packages/superset-ui-core/src/chart/components/createLoadableRenderer.ts
@@ -17,59 +17,109 @@
* under the License.
*/
-import Loadable from 'react-loadable';
-import { ComponentClass } from 'react';
+import { ReactElement, useEffect, useRef, useState } from 'react';
export type LoadableRendererProps = {
- onRenderFailure?: Function;
- onRenderSuccess?: Function;
+ onRenderFailure?: (error: Error) => void;
+ onRenderSuccess?: () => void;
};
-const defaultProps = {
- onRenderFailure() {},
- onRenderSuccess() {},
+type LoaderMap<Exports> = {
+ [K in keyof Exports]: () => Promise<Exports[K]> | Exports[K];
};
-export interface LoadableRenderer<Props>
- extends
- ComponentClass<Props & LoadableRendererProps>,
- Loadable.LoadableComponent {}
+export interface LoadingProps {
+ error?: { toString(): string };
+}
-export default function createLoadableRenderer<
- Props,
- Exports extends { [key: string]: any },
->(options: Loadable.OptionsWithMap<Props, Exports>): LoadableRenderer<Props> {
- const LoadableRenderer = Loadable.Map<Props, Exports>(
- options,
- ) as LoadableRenderer<Props>;
+export interface LoadableOptions<Props, Exports> {
+ loader: LoaderMap<Exports>;
+ loading: (loadingProps: LoadingProps) => ReactElement | null;
+ render: (loaded: Exports, props: Props) => ReactElement;
+}
- // Extends the behavior of LoadableComponent to provide post-render listeners
- class CustomLoadableRenderer extends LoadableRenderer {
- static defaultProps: object;
+export interface LoadableRenderer<Props> {
+ (props: Props & LoadableRendererProps): ReactElement | null;
+ preload: () => Promise<unknown>;
+ displayName?: string;
+}
- componentDidMount() {
- this.afterRender();
- }
+export default function createLoadableRenderer<Props, Exports>(
+ options: LoadableOptions<Props, Exports>,
+): LoadableRenderer<Props> {
+ let promise: Promise<Exports> | null = null;
+ let cachedResult: Exports | null = null;
+ let cachedError: Error | null = null;
- componentDidUpdate() {
- this.afterRender();
- }
+ const load = (): Promise<Exports> => {
+ if (promise) return promise;
+ const keys = Object.keys(options.loader) as (keyof Exports)[];
+ promise = Promise.all(
+ keys.map(key => Promise.resolve(options.loader[key]())),
+ ).then(
+ values => {
+ const loaded = {} as Exports;
+ keys.forEach((key, i) => {
+ loaded[key] = values[i] as Exports[typeof key];
+ });
+ cachedResult = loaded;
+ return loaded;
+ },
+ err => {
+ cachedError = err instanceof Error ? err : new Error(String(err));
+ throw cachedError;
+ },
+ );
+ return promise;
+ };
- afterRender() {
- const { loaded, loading, error } = this.state;
- const { onRenderFailure, onRenderSuccess } = this.props;
- if (!loading) {
- if (error) {
- (onRenderFailure as Function)(error);
- } else if (loaded && Object.keys(loaded).length > 0) {
- (onRenderSuccess as Function)();
- }
+ const Renderer: LoadableRenderer<Props> = props => {
+ const [state, setState] = useState<{
+ loaded: Exports | null;
+ error: Error | null;
+ }>(() => ({ loaded: cachedResult, error: cachedError }));
+
+ useEffect(() => {
+ if (state.loaded || state.error) return undefined;
+ let cancelled = false;
+ load().then(
+ loaded => {
+ if (!cancelled) setState({ loaded, error: null });
+ },
+ err => {
+ if (!cancelled) setState({ loaded: null, error: err });
+ },
+ );
+ return () => {
+ cancelled = true;
+ };
+ }, [state.loaded, state.error]);
+
+ // Keep callback refs current without retriggering the post-load effect on
+ // every prop update.
+ const onRenderSuccessRef = useRef(props.onRenderSuccess);
+ const onRenderFailureRef = useRef(props.onRenderFailure);
+ onRenderSuccessRef.current = props.onRenderSuccess;
+ onRenderFailureRef.current = props.onRenderFailure;
+
+ useEffect(() => {
+ if (state.error) {
+ onRenderFailureRef.current?.(state.error);
+ } else if (state.loaded && Object.keys(state.loaded).length > 0) {
+ onRenderSuccessRef.current?.();
}
+ }, [state.loaded, state.error]);
+
+ if (state.error) {
+ return options.loading({ error: state.error });
+ }
+ if (!state.loaded) {
+ return options.loading({});
}
- }
+ return options.render(state.loaded, props as Props);
+ };
- CustomLoadableRenderer.defaultProps = defaultProps;
- CustomLoadableRenderer.preload = LoadableRenderer.preload;
+ Renderer.preload = load;
- return CustomLoadableRenderer;
+ return Renderer;
}
diff --git a/superset-frontend/src/components/Chart/chartAction.ts b/superset-frontend/src/components/Chart/chartAction.ts
index 40d8ed79b16..17be5f09b2c 100644
--- a/superset-frontend/src/components/Chart/chartAction.ts
+++ b/superset-frontend/src/components/Chart/chartAction.ts
@@ -781,6 +781,16 @@ export function exploreJSON(
handleChartDataResponse(response, json, useLegacyApi),
)
.then(queriesResponse => {
+ // Drop stale responses: if a newer query has started for this chart,
+ // its controller will have replaced ours in state, so ignore this
+ // response to avoid clobbering newer data with older results.
+ if (key != null) {
+ const currentController =
+ getState().charts?.[key]?.queryController;
+ if (currentController && currentController !== controller) {
+ return undefined;
+ }
+ }
(queriesResponse as QueryData[]).forEach(
(resultItem: QueryData & { applied_filters?: JsonObject[] }) =>
dispatch(
@@ -826,6 +836,16 @@ export function exploreJSON(
);
}
+ // Drop stale failures the same way we drop stale successes,
+ // so a slow earlier request can't mark a newer one as failed.
+ if (key != null) {
+ const currentController =
+ getState().charts?.[key]?.queryController;
+ if (currentController && currentController !== controller) {
+ return undefined;
+ }
+ }
+
if (isFeatureEnabled(FeatureFlag.GlobalAsyncQueries)) {
// In async mode we just pass the raw error response through
return dispatch(
diff --git a/superset-frontend/src/embedded/index.tsx b/superset-frontend/src/embedded/index.tsx
index c49b282bda9..1a4a9eebcc9 100644
--- a/superset-frontend/src/embedded/index.tsx
+++ b/superset-frontend/src/embedded/index.tsx
@@ -18,7 +18,7 @@
*/
import 'src/public-path';
-import { lazy, StrictMode, Suspense, useEffect } from 'react';
+import { lazy, Suspense, useEffect } from 'react';
import { createRoot, type Root } from 'react-dom/client';
import { BrowserRouter as Router, Route } from 'react-router-dom';
import { Global } from '@emotion/react';
@@ -197,11 +197,7 @@ function start() {
if (!root) {
root = createRoot(appMountPoint);
}
- root.render(
- <StrictMode>
- <EmbeddedApp />
- </StrictMode>,
- );
+ root.render(<EmbeddedApp />);
},
err => {
// something is most likely wrong with the guest token; reset the guard
diff --git a/superset-frontend/src/views/index.tsx b/superset-frontend/src/views/index.tsx
index 456c1f09e92..e583f2bad86 100644
--- a/superset-frontend/src/views/index.tsx
+++ b/superset-frontend/src/views/index.tsx
@@ -18,7 +18,6 @@
*/
import 'src/public-path';
-import { StrictMode } from 'react';
import { createRoot } from 'react-dom/client';
import { logging } from '@apache-superset/core/utils';
import initPreamble from 'src/preamble';
@@ -32,11 +31,7 @@ if (appMountPoint) {
await initPreamble();
} finally {
const { default: App } = await import(/* webpackMode: "eager" */ './App');
- root.render(
- <StrictMode>
- <App />
- </StrictMode>,
- );
+ root.render(<App />);
}
})().catch(err => {
logging.error('Unhandled error during app initialization', err);
diff --git a/superset-frontend/src/views/menu.tsx b/superset-frontend/src/views/menu.tsx
index 5ba511c7032..dda01a8e27e 100644
--- a/superset-frontend/src/views/menu.tsx
+++ b/superset-frontend/src/views/menu.tsx
@@ -20,7 +20,6 @@ import 'src/public-path';
// Menu App. Used in views that do not already include the Menu component in the layout.
// eg, backend rendered views
-import { StrictMode } from 'react';
import { Provider } from 'react-redux';
import { createRoot } from 'react-dom/client';
import { BrowserRouter } from 'react-router-dom';
@@ -46,26 +45,24 @@ const emotionCache = createCache({
});
const app = (
- <StrictMode>
- <CacheProvider value={emotionCache}>
- <ThemeProvider theme={theme}>
- <Provider store={store}>
- <BrowserRouter>
- <QueryParamProvider
- adapter={ReactRouter5Adapter}
- options={{
- searchStringToObject: querystring.parse,
- objectToSearchString: (object: Record<string, any>) =>
- querystring.stringify(object, { encode: false }),
- }}
- >
- <Menu data={menu} />
- </QueryParamProvider>
- </BrowserRouter>
- </Provider>
- </ThemeProvider>
- </CacheProvider>
- </StrictMode>
+ <CacheProvider value={emotionCache}>
+ <ThemeProvider theme={theme}>
+ <Provider store={store}>
+ <BrowserRouter>
+ <QueryParamProvider
+ adapter={ReactRouter5Adapter}
+ options={{
+ searchStringToObject: querystring.parse,
+ objectToSearchString: (object: Record<string, any>) =>
+ querystring.stringify(object, { encode: false }),
+ }}
+ >
+ <Menu data={menu} />
+ </QueryParamProvider>
+ </BrowserRouter>
+ </Provider>
+ </ThemeProvider>
+ </CacheProvider>
);
const menuMountPoint = document.getElementById('app-menu');