From 664a891f8484f8ec593cd03b3fe20155be05a8cc Mon Sep 17 00:00:00 2001
From: Claude Code <noreply@anthropic.com>
Date: Tue, 19 May 2026 00:30:50 -0500
Subject: [PATCH] =?UTF-8?q?fix(ChartRenderer):=20address=20review=20on=20c?=
 =?UTF-8?q?lass=E2=86=92function=20refactor?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Per @Copilot's review on #39459:

1. **showContextMenu staleness.** Moved out of `useState` (where it was
   computed once at initializer time and would go stale on source / viz-
   type changes for the same mounted instance) into a `useMemo` derived
   from props + feature flags. Renames `state.showContextMenu` to plain
   `showContextMenu` at all four call sites. Matches the pre-refactor
   semantic of recomputing on every render.

2. **cloneDeep runs during loading.** Gated the deep clone of
   `queriesResponse` on `resultsReady` so it only runs when results are
   about to render. Previously it ran on every `queriesResponse`
   identity change including loading/idle transitions.

Not changed in this commit (responses to come in separate replies):
- Chart.tsx prop spreading change — needs an audit of `ChartProps`
  surface to confirm nothing dropped silently.
- `memo()` vs custom `shouldComponentUpdate` — would need a tailored
  `areEqual` to faithfully port the old SCU; deferring as a follow-up
  since the default-memo behavior is the more idiomatic React 18+ shape.
- Test cast `props as ChartRendererProps` — fixture re-typing is a
  test-cleanup pass that's out of scope here.
---
 .../src/components/Chart/ChartRenderer.tsx    | 28 +++++++++++++------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/superset-frontend/src/components/Chart/ChartRenderer.tsx b/superset-frontend/src/components/Chart/ChartRenderer.tsx
index d0d85812287..594d758efca 100644
--- a/superset-frontend/src/components/Chart/ChartRenderer.tsx
+++ b/superset-frontend/src/components/Chart/ChartRenderer.tsx
@@ -178,7 +178,6 @@ const BIG_NO_RESULT_MIN_HEIGHT = 220;
 const behaviors = [Behavior.InteractiveChart];
 
 interface ChartRendererState {
-  showContextMenu: boolean;
   inContextMenu: boolean;
   legendState: LegendState | undefined;
   legendIndex: number;
@@ -221,11 +220,19 @@ function ChartRendererComponent({
     formData.viz_type ?? propVizType,
   )?.suppressContextMenu;
 
-  const [state, setState] = useState<ChartRendererState>({
-    showContextMenu:
+  // Derived from props/feature-flags: must NOT live in state, otherwise a
+  // `source` or viz-type change on the same mounted instance would leave
+  // it stale. (Pre-refactor this was a class-instance field recomputed on
+  // every render — preserve that semantic by using a memo here.)
+  const showContextMenu = useMemo(
+    () =>
       source === ChartSource.Dashboard &&
       !suppressContextMenu &&
       isFeatureEnabled(FeatureFlag.DrillToDetail),
+    [source, suppressContextMenu],
+  );
+
+  const [state, setState] = useState<ChartRendererState>({
     inContextMenu: false,
     legendState: undefined,
     legendIndex: 0,
@@ -259,10 +266,13 @@ function ChartRendererComponent({
   }, [queriesResponse]);
 
   // Clone queriesResponse to protect against plugin mutation of Redux state.
+  // Gate on `resultsReady` so the deep clone doesn't run for every
+  // queriesResponse identity change during loading/idle (only when results
+  // are actually about to render). Matches the pre-refactor gating.
   // TODO: remove once reducers use Redux Toolkit with Immer.
   const mutableQueriesResponse = useMemo(
-    () => cloneDeep(queriesResponse),
-    [queriesResponse],
+    () => (resultsReady ? cloneDeep(queriesResponse) : undefined),
+    [queriesResponse, resultsReady],
   );
 
   // Handler functions
@@ -373,7 +383,7 @@ function ChartRendererComponent({
   const hooks = useMemo<ChartHooks>(
     () => ({
       onAddFilter: handleAddFilter,
-      onContextMenu: state.showContextMenu ? handleOnContextMenu : undefined,
+      onContextMenu: showContextMenu ? handleOnContextMenu : undefined,
       onError: handleRenderFailure,
       setControlValue: handleSetControlValue,
       onFilterMenuOpen,
@@ -394,7 +404,7 @@ function ChartRendererComponent({
       onFilterMenuClose,
       onFilterMenuOpen,
       setDataMaskCallback,
-      state.showContextMenu,
+      showContextMenu,
     ],
   );
 
@@ -490,7 +500,7 @@ function ChartRendererComponent({
 
   return (
     <>
-      {state.showContextMenu && (
+      {showContextMenu && (
         <ChartContextMenu
           ref={contextMenuRef}
           id={chartId}
@@ -501,7 +511,7 @@ function ChartRendererComponent({
       )}
       <div
         onContextMenu={
-          state.showContextMenu ? onContextMenuFallback : undefined
+          showContextMenu ? onContextMenuFallback : undefined
         }
       >
         <SuperChart