fix(DatasourceModal): replace imperative modal updates with declarative state

Fixes DOM error "Failed to execute 'insertBefore' on 'Node'" by replacing
imperative modal.update() calls with React state management.

Changes:
- Convert syncColumnsRef to syncColumns state variable
- Remove confirmModal state and imperative update calls
- Simplify modal creation to not store instance
- Add tests to verify useModal hook usage and prevent regressions
- Remove unused useRef import

The modal content now updates declaratively through React re-renders
instead of direct DOM manipulation, eliminating insertion conflicts.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

Author: eschutho

superset-frontend/src/components/Datasource/DatasourceModal/DatasourceModal.useModal.test.tsx

@@ -0,0 +1,112 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+  render,
+  screen,
+  fireEvent,
+  act,
+  defaultStore as store,
+} from 'spec/helpers/testing-library';
+import fetchMock from 'fetch-mock';
+import { Modal } from '@superset-ui/core/components';
+import mockDatasource from 'spec/fixtures/mockDatasource';
+import DatasourceModal from '.';
+
+const mockedProps = {
+  datasource: mockDatasource['7__table'],
+  addSuccessToast: jest.fn(),
+  addDangerToast: jest.fn(),
+  onChange: jest.fn(),
+  onHide: jest.fn(),
+  show: true,
+  onDatasourceSave: jest.fn(),
+};
+
+describe('DatasourceModal - useModal Hook Tests', () => {
+  beforeEach(() => {
+    fetchMock.reset();
+    fetchMock.put('glob:*/api/v1/dataset/7?override_columns=*', {});
+    fetchMock.get('glob:*/api/v1/dataset/7', { result: {} });
+    fetchMock.get('glob:*/api/v1/database/?q=*', { result: [] });
+  });
+
+  afterEach(() => {
+    fetchMock.reset();
+    jest.clearAllMocks();
+  });
+
+  test('should use Modal.useModal hook instead of Modal.confirm directly', () => {
+    const useModalSpy = jest.spyOn(Modal, 'useModal');
+    const confirmSpy = jest.spyOn(Modal, 'confirm');
+
+    render(<DatasourceModal {...mockedProps} />, { store });
+
+    // Should use the useModal hook
+    expect(useModalSpy).toHaveBeenCalled();
+
+    // Should not call Modal.confirm during initial render
+    expect(confirmSpy).not.toHaveBeenCalled();
+
+    useModalSpy.mockRestore();
+    confirmSpy.mockRestore();
+  });
+
+  test('should handle sync columns state without imperative modal updates', async () => {
+    // Test that we can successfully click the save button without DOM errors
+    // The actual checkbox is only visible when SQL has changed
+    render(<DatasourceModal {...mockedProps} />, { store });
+
+    const saveButton = screen.getByTestId('datasource-modal-save');
+
+    // This should not throw any DOM errors
+    await act(async () => {
+      fireEvent.click(saveButton);
+    });
+
+    // Should show confirmation modal
+    expect(screen.getByText('Confirm save')).toBeInTheDocument();
+
+    // Should show the confirmation message
+    expect(
+      screen.getByText('Are you sure you want to save and apply changes?'),
+    ).toBeInTheDocument();
+  });
+
+  test('should not store modal instance in state', () => {
+    // Mock console.warn to catch any warnings about refs or imperatives
+    const consoleWarn = jest.spyOn(console, 'warn').mockImplementation();
+
+    render(<DatasourceModal {...mockedProps} />, { store });
+
+    // No warnings should be generated about improper React patterns
+    const reactWarnings = consoleWarn.mock.calls.filter(call =>
+      call.some(
+        arg =>
+          typeof arg === 'string' &&
+          (arg.includes('findDOMNode') ||
+            arg.includes('ref') ||
+            arg.includes('instance')),
+      ),
+    );
+
+    expect(reactWarnings).toHaveLength(0);
+
+    consoleWarn.mockRestore();
+  });
+});

superset-frontend/src/components/Datasource/DatasourceModal/index.tsx

@@ -16,13 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import {
-  FunctionComponent,
-  useState,
-  useRef,
-  useEffect,
-  useCallback,
-} from 'react';
+import { FunctionComponent, useState, useEffect, useCallback } from 'react';
 import { useSelector } from 'react-redux';
 import {
   styled,
@@ -101,8 +95,7 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
 }) => {
   const theme = useTheme();
   const [currentDatasource, setCurrentDatasource] = useState(datasource);
-  const syncColumnsRef = useRef(false);
-  const [confirmModal, setConfirmModal] = useState<any>(null);
+  const [syncColumns, setSyncColumns] = useState(false);
   const currencies = useSelector<
     {
       common: {
@@ -114,7 +107,6 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
   const [errors, setErrors] = useState<any[]>([]);
   const [isSaving, setIsSaving] = useState(false);
   const [isEditing, setIsEditing] = useState<boolean>(false);
-  const dialog = useRef<any>(null);
   const [modal, contextHolder] = Modal.useModal();
   const buildPayload = (datasource: Record<string, any>) => {
     const payload: Record<string, any> = {
@@ -196,7 +188,7 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
     setIsSaving(true);
     try {
       await SupersetClient.put({
-        endpoint: `/api/v1/dataset/${currentDatasource.id}?override_columns=${syncColumnsRef.current}`,
+        endpoint: `/api/v1/dataset/${currentDatasource.id}?override_columns=${syncColumns}`,
         jsonPayload: buildPayload(currentDatasource),
       });
 
@@ -281,14 +273,9 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
               impact the column definitions, you might want to skip this step.`)}
             />
             <Checkbox
-              checked={syncColumnsRef.current}
+              checked={syncColumns}
               onChange={() => {
-                syncColumnsRef.current = !syncColumnsRef.current;
-                if (confirmModal) {
-                  confirmModal.update({
-                    content: getSaveDialog(),
-                  });
-                }
+                setSyncColumns(!syncColumns);
               }}
             />
             <span
@@ -303,34 +290,24 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
         {t('Are you sure you want to save and apply changes?')}
       </div>
     ),
-    [currentDatasource.sql, datasource.sql, confirmModal],
+    [currentDatasource.sql, datasource.sql, syncColumns],
   );
 
-  useEffect(() => {
-    if (confirmModal) {
-      confirmModal.update({
-        content: getSaveDialog(),
-      });
-    }
-  }, [confirmModal, getSaveDialog]);
-
   useEffect(() => {
     if (datasource.sql !== currentDatasource.sql) {
-      syncColumnsRef.current = true;
+      setSyncColumns(true);
     }
   }, [datasource.sql, currentDatasource.sql]);
 
   const onClickSave = () => {
-    const modalInstance = modal.confirm({
+    modal.confirm({
       title: t('Confirm save'),
       content: getSaveDialog(),
       onOk: onConfirmSave,
       icon: null,
       okText: t('OK'),
       cancelText: t('Cancel'),
     });
-    setConfirmModal(modalInstance);
-    dialog.current = modalInstance;
   };
 
   return (