From e8f20a67e4cefd4559a1dc3986cdc1e09295feb7 Mon Sep 17 00:00:00 2001
From: Claude <claude@anthropic.com>
Date: Tue, 12 May 2026 14:31:29 -0700
Subject: [PATCH] fix(AnnotationLayer): stop double-fetching chart on hydration
 + drop dead useCallback
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two fixes from @sadpandajoe's review.

1. **Double-fetch.** `fetchAppliedChart` synchronously sets both `value`
   and `slice` from one API response. The value-change watcher then saw
   `value` changed and called `fetchSliceData(value.value)` — re-resolving
   the same chart and overwriting the slice we just set.

   Fix: gate the watcher's `fetchSliceData` on `!slice`. When
   `fetchAppliedChart` populated slice in lockstep, the gate skips. When
   the user selects a different chart from the dropdown
   (`handleSelectValue`), `slice` is now cleared to null first, so the
   watcher fires and fetches correctly.

2. **Dead `useCallback`.** `renderChartHeader` (empty deps) only built
   JSX from its arguments and was called inline as `renderChartHeader(…)`
   — neither the produced node nor the function identity was observed by
   a memoized consumer, so the useCallback was overhead with no benefit.
   Inline as a plain helper named `buildChartHeader`.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 .../AnnotationLayer.tsx                       | 52 ++++++++++++-------
 1 file changed, 33 insertions(+), 19 deletions(-)

diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.tsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.tsx
index 29e2f234c80..d8b2963f645 100644
--- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.tsx
+++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.tsx
@@ -453,18 +453,28 @@ function AnnotationLayer({
     string | number | SelectOption | null
   >(value);
 
-  // componentDidUpdate - fetch slice data when value changes
+  // Fetch slice data when value changes — but skip when slice is already
+  // populated, which means the change came from `fetchAppliedChart` (the
+  // mount-time hydration path that sets both value and slice in lockstep).
+  // The user-selection path (`handleSelectValue`) clears slice to null
+  // before setting the new value, so this watcher fires correctly there.
   useEffect(() => {
     if (value !== prevValue) {
       const isChart =
         sourceType !== ANNOTATION_SOURCE_TYPES.NATIVE &&
         requiresQuery(sourceType ?? undefined);
-      if (isChart && value && typeof value === 'object' && 'value' in value) {
+      if (
+        isChart &&
+        value &&
+        typeof value === 'object' &&
+        'value' in value &&
+        !slice
+      ) {
         fetchSliceData(value.value);
       }
       setPrevValue(value);
     }
-  }, [value, prevValue, sourceType, fetchSliceData]);
+  }, [value, prevValue, sourceType, fetchSliceData, slice]);
 
   const isValidFormulaAnnotation = useCallback(
     (
@@ -529,6 +539,9 @@ function AnnotationLayer({
   const handleSelectValue = useCallback(
     (selectedValueObject: SelectOption): void => {
       setValue(selectedValueObject);
+      // Clear slice so the value-change watcher refetches for the new chart;
+      // see the watcher block above for why this gate exists.
+      setSlice(null);
       setDescriptionColumns([]);
       setIntervalEndColumn('');
       setTimeColumn('');
@@ -618,20 +631,22 @@ function AnnotationLayer({
     close?.();
   }, [applyAnnotation, close]);
 
-  const renderChartHeader = useCallback(
-    (
-      label: string,
-      description: string,
-      val: string | number | SelectOption | null,
-    ): React.ReactNode => (
-      <ControlHeader
-        hovered
-        label={label}
-        description={description}
-        validationErrors={!val ? ['Mandatory'] : []}
-      />
-    ),
-    [],
+  // Inlined: this is a pure helper that produces JSX from its args. The
+  // previous useCallback wrapper added no value — `renderChartHeader(label,
+  // description, value)` was called inline, so the produced ReactNode was
+  // recreated each call anyway, and no downstream consumer depended on the
+  // function's identity.
+  const buildChartHeader = (
+    label: string,
+    description: string,
+    val: string | number | SelectOption | null,
+  ): React.ReactNode => (
+    <ControlHeader
+      hovered
+      label={label}
+      description={description}
+      validationErrors={!val ? ['Mandatory'] : []}
+    />
   );
 
   const renderValueConfiguration = useCallback((): React.ReactNode => {
@@ -664,7 +679,7 @@ function AnnotationLayer({
           key={sourceType}
           ariaLabel={t('Annotation layer value')}
           name="annotation-layer-value"
-          header={renderChartHeader(label, description, value)}
+          header={buildChartHeader(label, description, value)}
           options={fetchOptions}
           value={value || null}
           onChange={handleSelectValue}
@@ -699,7 +714,6 @@ function AnnotationLayer({
     annotationType,
     value,
     getSupportedSourceTypes,
-    renderChartHeader,
     fetchOptions,
     handleSelectValue,
     handleTextValue,