fix(bug): Error when adding a filter using custom sql (#38246)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

superset-frontend/package-lock.json

@@ -287,7 +287,7 @@
         "unzipper": "^0.12.3",
         "vm-browserify": "^1.1.2",
         "wait-on": "^9.0.4",
-        "webpack": "^5.105.3",
+        "webpack": "^5.105.4",
         "webpack-bundle-analyzer": "^5.2.0",
         "webpack-cli": "^6.0.1",
         "webpack-dev-server": "^5.2.3",
@@ -23496,9 +23496,9 @@
       }
     },
     "node_modules/enhanced-resolve": {
-      "version": "5.19.0",
-      "resolved": "https://registry.npmjs.org/enhanced-resolve/-/enhanced-resolve-5.19.0.tgz",
-      "integrity": "sha512-phv3E1Xl4tQOShqSte26C7Fl84EwUdZsyOuSSk9qtAGyyQs2s3jJzComh+Abf4g187lUUAvH+H26omrqia2aGg==",
+      "version": "5.20.0",
+      "resolved": "https://registry.npmjs.org/enhanced-resolve/-/enhanced-resolve-5.20.0.tgz",
+      "integrity": "sha512-/ce7+jQ1PQ6rVXwe+jKEg5hW5ciicHwIQUagZkp6IufBoY3YDgdTTY1azVs0qoRgVmvsNB+rbjLJxDAeHHtwsQ==",
       "dev": true,
       "license": "MIT",
       "dependencies": {
@@ -49340,9 +49340,9 @@
       }
     },
     "node_modules/webpack": {
-      "version": "5.105.3",
-      "resolved": "https://registry.npmjs.org/webpack/-/webpack-5.105.3.tgz",
-      "integrity": "sha512-LLBBA4oLmT7sZdHiYE/PeVuifOxYyE2uL/V+9VQP7YSYdJU7bSf7H8bZRRxW8kEPMkmVjnrXmoR3oejIdX0xbg==",
+      "version": "5.105.4",
+      "resolved": "https://registry.npmjs.org/webpack/-/webpack-5.105.4.tgz",
+      "integrity": "sha512-jTywjboN9aHxFlToqb0K0Zs9SbBoW4zRUlGzI2tYNxVYcEi/IPpn+Xi4ye5jTLvX2YeLuic/IvxNot+Q1jMoOw==",
       "dev": true,
       "license": "MIT",
       "dependencies": {
@@ -49356,7 +49356,7 @@
         "acorn-import-phases": "^1.0.3",
         "browserslist": "^4.28.1",
         "chrome-trace-event": "^1.0.2",
-        "enhanced-resolve": "^5.19.0",
+        "enhanced-resolve": "^5.20.0",
         "es-module-lexer": "^2.0.0",
         "eslint-scope": "5.1.1",
         "events": "^3.2.0",
@@ -49368,7 +49368,7 @@
         "neo-async": "^2.6.2",
         "schema-utils": "^4.3.3",
         "tapable": "^2.3.0",
-        "terser-webpack-plugin": "^5.3.16",
+        "terser-webpack-plugin": "^5.3.17",
         "watchpack": "^2.5.1",
         "webpack-sources": "^3.3.4"
       },

superset-frontend/package.json

@@ -368,7 +368,7 @@
     "unzipper": "^0.12.3",
     "vm-browserify": "^1.1.2",
     "wait-on": "^9.0.4",
-    "webpack": "^5.105.3",
+    "webpack": "^5.105.4",
     "webpack-bundle-analyzer": "^5.2.0",
     "webpack-cli": "^6.0.1",
     "webpack-dev-server": "^5.2.3",

superset-frontend/packages/superset-core/src/api/editors.ts

@@ -480,6 +480,18 @@ export interface EditorHandle {
    * @returns A Disposable that removes the provider when disposed
    */
   registerCompletionProvider(provider: CompletionProvider): Disposable;
+
+  /**
+   * Force the editor to recalculate its dimensions.
+   * Called when the container size changes or when the editor becomes
+   * visible after being hidden (e.g., in a tab).
+   *
+   * Each editor implementation maps this to their equivalent:
+   * - Ace: editor.resize()
+   * - Monaco: editor.layout()
+   * - CodeMirror: editor.requestMeasure()
+   */
+  resize(): void;
 }
 
 /**

superset-frontend/src/core/editors/AceEditorProvider.tsx

@@ -182,6 +182,10 @@ const createAceEditorHandle = (
       completionProviders.current.delete(provider.id);
     });
   },
+
+  resize: () => {
+    aceEditorRef.current?.editor?.resize();
+  },
 });
 
 /**

superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/AdhocFilterEditPopoverSqlTabContent.test.tsx

@@ -22,30 +22,52 @@ import {
   screen,
   selectOption,
   userEvent,
+  waitFor,
 } from 'spec/helpers/testing-library';
 import AdhocFilter from '../AdhocFilter';
 import { Clauses, ExpressionTypes } from '../types';
 import AdhocFilterEditPopoverSqlTabContent from '.';
 
+// Track resize calls for testing
+const mockResize = jest.fn();
+
+// Mock EditorHost with ref support for resize
+jest.mock('src/core/editors', () => {
+  const React = require('react');
+  return {
+    EditorHost: React.forwardRef(
+      (
+        {
+          value,
+          onChange,
+        }: {
+          value: string;
+          onChange: (v: string) => void;
+        },
+        ref: React.Ref<{ resize: () => void }>,
+      ) => {
+        React.useImperativeHandle(ref, () => ({
+          resize: mockResize,
+        }));
+        return (
+          <textarea
+            defaultValue={value}
+            onChange={e => onChange?.(e.target.value)}
+          />
+        );
+      },
+    ),
+  };
+});
+
 // Add cleanup after each test
 afterEach(async () => {
   cleanup();
+  mockResize.mockClear();
   // Wait for any pending effects to complete
   await new Promise(resolve => setTimeout(resolve, 0));
 });
 
-jest.mock('src/core/editors', () => ({
-  EditorHost: ({
-    value,
-    onChange,
-  }: {
-    value: string;
-    onChange: (v: string) => void;
-  }) => (
-    <textarea defaultValue={value} onChange={e => onChange?.(e.target.value)} />
-  ),
-}));
-
 const adhocFilter = new AdhocFilter({
   expressionType: ExpressionTypes.Sql,
   sqlExpression: 'value > 10',
@@ -89,3 +111,41 @@ test('calls onChange when the SQL expression changes', async () => {
     expect.objectContaining({ sqlExpression: input }),
   );
 });
+
+test('calls editor resize when adhocFilter changes', async () => {
+  const onChange = jest.fn();
+  const { rerender } = render(
+    <AdhocFilterEditPopoverSqlTabContent
+      adhocFilter={adhocFilter}
+      onChange={onChange}
+      options={[]}
+      height={100}
+    />,
+  );
+
+  // Initial render should call resize
+  await waitFor(() => {
+    expect(mockResize).toHaveBeenCalled();
+  });
+  mockResize.mockClear();
+
+  // Create a new filter to trigger the useEffect
+  const newFilter = new AdhocFilter({
+    expressionType: ExpressionTypes.Sql,
+    sqlExpression: 'value > 20',
+    clause: Clauses.Where,
+  });
+
+  rerender(
+    <AdhocFilterEditPopoverSqlTabContent
+      adhocFilter={newFilter}
+      onChange={onChange}
+      options={[]}
+      height={100}
+    />,
+  );
+
+  await waitFor(() => {
+    expect(mockResize).toHaveBeenCalled();
+  });
+});

superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/index.tsx

@@ -17,6 +17,7 @@
  * under the License.
  */
 import { useEffect, useRef, useMemo } from 'react';
+import type { editors } from '@apache-superset/core';
 import { Select } from '@superset-ui/core/components';
 import { t } from '@apache-superset/core';
 import { css, styled, useTheme } from '@apache-superset/core/ui';
@@ -49,12 +50,11 @@ export default function AdhocFilterEditPopoverSqlTabContent({
   height: number;
   datasource?: any;
 }) {
-  const aceEditorRef = useRef(null);
+  const editorRef = useRef<editors.EditorHandle>(null);
   const theme = useTheme();
 
   useEffect(() => {
-    // @ts-expect-error - AceEditor ref type doesn't expose editor.resize()
-    aceEditorRef?.current?.editor.resize();
+    editorRef.current?.resize();
   }, [adhocFilter]);
 
   const onSqlExpressionClauseChange = (clause: string) => {
@@ -125,7 +125,7 @@ export default function AdhocFilterEditPopoverSqlTabContent({
         `}
       >
         <SQLEditorWithValidation
-          ref={aceEditorRef}
+          ref={editorRef}
           keywords={keywords}
           height={`${height - 130}px`}
           onChange={onSqlExpressionChange}

superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.tsx

@@ -130,7 +130,7 @@ export default class AdhocMetricEditPopover extends PureComponent<
   // "Saved" is a default tab unless there are no saved metrics for dataset
   defaultActiveTabKey = this.getDefaultTab();
 
-  aceEditorRef: RefObject<editors.EditorHandle>;
+  editorRef: RefObject<editors.EditorHandle>;
 
   dragStartX = 0;
 
@@ -152,8 +152,8 @@ export default class AdhocMetricEditPopover extends PureComponent<
     this.onMouseMove = this.onMouseMove.bind(this);
     this.onMouseUp = this.onMouseUp.bind(this);
     this.onTabChange = this.onTabChange.bind(this);
-    this.aceEditorRef = createRef();
-    this.refreshAceEditor = this.refreshAceEditor.bind(this);
+    this.editorRef = createRef();
+    this.refreshEditor = this.refreshEditor.bind(this);
     this.getDefaultTab = this.getDefaultTab.bind(this);
 
     this.state = {
@@ -313,20 +313,13 @@ export default class AdhocMetricEditPopover extends PureComponent<
   }
 
   onTabChange(tab: string): void {
-    this.refreshAceEditor();
+    this.refreshEditor();
     this.props.getCurrentTab?.(tab);
   }
 
-  refreshAceEditor(): void {
+  refreshEditor(): void {
     setTimeout(() => {
-      if (this.aceEditorRef.current) {
-        // Cast to access ace editor API
-        (
-          this.aceEditorRef.current as unknown as {
-            editor?: { resize?: () => void };
-          }
-        ).editor?.resize?.();
-      }
+      this.editorRef.current?.resize();
     }, 0);
   }
 
@@ -549,7 +542,7 @@ export default class AdhocMetricEditPopover extends PureComponent<
               children: (
                 <SQLEditorWithValidation
                   data-test="sql-editor"
-                  ref={this.aceEditorRef}
+                  ref={this.editorRef}
                   keywords={keywords}
                   height={`${this.state.height - 120}px`}
                   onChange={this.onSqlExpressionChange}