You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2025/08/27 06:46:11 UTC
(superset) 01/01: fix(ConfirmStatusChange): remove deprecated event.persist() to fix Chrome 139 crashes
This is an automated email from the ASF dual-hosted git repository.
jli pushed a commit to branch joe/fix-confirm-status-change-persist
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 1ee8b15f7d5920caf97f172f9255e5f4005c352a
Author: Joe Li <jo...@preset.io>
AuthorDate: Tue Aug 26 23:45:37 2025 -0700
fix(ConfirmStatusChange): remove deprecated event.persist() to fix Chrome 139 crashes
Remove unnecessary event.persist() call that causes browser crashes in headless Chrome 139.
React 17 doesn't require event persistence as events are no longer pooled.
- Remove event.persist() call from showConfirm function
- Add comprehensive test suite (8 tests) covering all component functionality
- Convert test file from .jsx to .tsx for TypeScript consistency
- Fix crashes in bulk delete operations across 14+ list components
Fixes browser crashes when using bulk delete in headless environments.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <no...@anthropic.com>
---
.../ConfirmStatusChange.test.jsx | 66 --------
.../ConfirmStatusChange.test.tsx | 177 +++++++++++++++++++++
.../src/components/ConfirmStatusChange/index.tsx | 5 +-
3 files changed, 178 insertions(+), 70 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-core/src/components/ConfirmStatusChange/ConfirmStatusChange.test.jsx b/superset-frontend/packages/superset-ui-core/src/components/ConfirmStatusChange/ConfirmStatusChange.test.jsx
deleted file mode 100644
index bd84ca1a28..0000000000
--- a/superset-frontend/packages/superset-ui-core/src/components/ConfirmStatusChange/ConfirmStatusChange.test.jsx
+++ /dev/null
@@ -1,66 +0,0 @@
-/**
- * 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 { fireEvent, render, waitFor } from '@superset-ui/core/spec';
-import { Button } from '../Button';
-import { ConfirmStatusChange } from '.';
-
-const mockedProps = {
- title: 'please confirm',
- description: 'are you sure?',
- onConfirm: jest.fn(),
-};
-
-test('opens a confirm modal', () => {
- const { getByTestId } = render(
- <ConfirmStatusChange {...mockedProps}>
- {confirm => (
- <>
- <Button data-test="btn1" onClick={confirm} />
- </>
- )}
- </ConfirmStatusChange>,
- );
-
- fireEvent.click(getByTestId('btn1'));
-
- expect(getByTestId(`${mockedProps.title}-modal`)).toBeInTheDocument();
-});
-
-test('calls the function on confirm', async () => {
- const { getByTestId, getByRole } = render(
- <ConfirmStatusChange {...mockedProps}>
- {confirm => (
- <>
- <Button data-test="btn1" onClick={() => confirm('foo')} />
- </>
- )}
- </ConfirmStatusChange>,
- );
-
- fireEvent.click(getByTestId('btn1'));
-
- const confirmInput = getByTestId('delete-modal-input');
- fireEvent.change(confirmInput, { target: { value: 'DELETE' } });
-
- const confirmButton = getByRole('button', { name: 'Delete' });
- fireEvent.click(confirmButton);
-
- await waitFor(() => expect(mockedProps.onConfirm).toHaveBeenCalledTimes(1));
- expect(mockedProps.onConfirm).toHaveBeenCalledWith('foo');
-});
diff --git a/superset-frontend/packages/superset-ui-core/src/components/ConfirmStatusChange/ConfirmStatusChange.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/ConfirmStatusChange/ConfirmStatusChange.test.tsx
new file mode 100644
index 0000000000..5ac2036f2b
--- /dev/null
+++ b/superset-frontend/packages/superset-ui-core/src/components/ConfirmStatusChange/ConfirmStatusChange.test.tsx
@@ -0,0 +1,177 @@
+/**
+ * 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 { fireEvent, render, waitFor } from '@superset-ui/core/spec';
+import { Button } from '../Button';
+import { ConfirmStatusChange } from '.';
+import type { ConfirmStatusChangeProps } from './types';
+
+const mockedProps: Omit<ConfirmStatusChangeProps, 'children'> = {
+ title: 'please confirm',
+ description: 'are you sure?',
+ onConfirm: jest.fn(),
+};
+
+test('renders children with showConfirm function', () => {
+ const childrenSpy = jest.fn().mockReturnValue(<div>test content</div>);
+
+ render(
+ <ConfirmStatusChange {...mockedProps}>{childrenSpy}</ConfirmStatusChange>,
+ );
+
+ expect(childrenSpy).toHaveBeenCalledWith(expect.any(Function));
+});
+
+test('opens modal when showConfirm is called', () => {
+ const { getByTestId } = render(
+ <ConfirmStatusChange {...mockedProps}>
+ {confirm => <Button data-test="trigger" onClick={confirm} />}
+ </ConfirmStatusChange>,
+ );
+
+ fireEvent.click(getByTestId('trigger'));
+
+ expect(getByTestId(`${mockedProps.title}-modal`)).toBeInTheDocument();
+});
+
+test('stores and passes arguments to onConfirm callback', async () => {
+ const testArgs = ['arg1', { data: 'test' }, 42];
+ const { getByTestId, getByRole } = render(
+ <ConfirmStatusChange {...mockedProps}>
+ {confirm => (
+ <Button data-test="trigger" onClick={() => confirm(...testArgs)} />
+ )}
+ </ConfirmStatusChange>,
+ );
+
+ fireEvent.click(getByTestId('trigger'));
+
+ const confirmInput = getByTestId('delete-modal-input');
+ fireEvent.change(confirmInput, { target: { value: 'DELETE' } });
+
+ const confirmButton = getByRole('button', { name: 'Delete' });
+ fireEvent.click(confirmButton);
+
+ await waitFor(() => expect(mockedProps.onConfirm).toHaveBeenCalledTimes(1));
+ expect(mockedProps.onConfirm).toHaveBeenCalledWith(...testArgs);
+});
+
+test('calls preventDefault on event-like arguments', () => {
+ const mockEvent = {
+ preventDefault: jest.fn(),
+ stopPropagation: jest.fn(),
+ };
+
+ const { getByTestId } = render(
+ <ConfirmStatusChange {...mockedProps}>
+ {confirm => (
+ <Button data-test="trigger" onClick={() => confirm(mockEvent)} />
+ )}
+ </ConfirmStatusChange>,
+ );
+
+ fireEvent.click(getByTestId('trigger'));
+
+ expect(mockEvent.preventDefault).toHaveBeenCalled();
+ expect(mockEvent.stopPropagation).toHaveBeenCalled();
+});
+
+test('skips event handling on non-event arguments', () => {
+ const regularArg = { someData: 'value' };
+ const mockFunc = jest.fn();
+
+ const { getByTestId } = render(
+ <ConfirmStatusChange {...mockedProps}>
+ {confirm => (
+ <Button
+ data-test="trigger"
+ onClick={() => confirm(regularArg, mockFunc)}
+ />
+ )}
+ </ConfirmStatusChange>,
+ );
+
+ // Should not throw when processing non-event arguments
+ expect(() => {
+ fireEvent.click(getByTestId('trigger'));
+ }).not.toThrow();
+
+ expect(getByTestId(`${mockedProps.title}-modal`)).toBeInTheDocument();
+});
+
+test('ignores null and undefined arguments', () => {
+ const { getByTestId } = render(
+ <ConfirmStatusChange {...mockedProps}>
+ {confirm => (
+ <Button
+ data-test="trigger"
+ onClick={() => confirm(null, undefined, 'valid')}
+ />
+ )}
+ </ConfirmStatusChange>,
+ );
+
+ expect(() => {
+ fireEvent.click(getByTestId('trigger'));
+ }).not.toThrow();
+
+ expect(getByTestId(`${mockedProps.title}-modal`)).toBeInTheDocument();
+});
+
+test('handles partial event objects gracefully', () => {
+ const partialEvent1 = { preventDefault: jest.fn() }; // Only preventDefault
+ const partialEvent2 = { stopPropagation: jest.fn() }; // Only stopPropagation
+
+ const { getByTestId } = render(
+ <ConfirmStatusChange {...mockedProps}>
+ {confirm => (
+ <Button
+ data-test="trigger"
+ onClick={() => confirm(partialEvent1, partialEvent2)}
+ />
+ )}
+ </ConfirmStatusChange>,
+ );
+
+ fireEvent.click(getByTestId('trigger'));
+
+ expect(partialEvent1.preventDefault).toHaveBeenCalled();
+ expect(partialEvent2.stopPropagation).toHaveBeenCalled();
+ expect(getByTestId(`${mockedProps.title}-modal`)).toBeInTheDocument();
+});
+
+test('closes modal when onHide is called', () => {
+ const { getByTestId, getByRole } = render(
+ <ConfirmStatusChange {...mockedProps}>
+ {confirm => <Button data-test="trigger" onClick={confirm} />}
+ </ConfirmStatusChange>,
+ );
+
+ // Open modal
+ fireEvent.click(getByTestId('trigger'));
+ const modal = getByTestId(`${mockedProps.title}-modal`);
+ expect(modal).toBeInTheDocument();
+ expect(modal).toBeVisible();
+
+ // Close modal
+ const cancelButton = getByRole('button', { name: 'Cancel' });
+ fireEvent.click(cancelButton);
+
+ // Modal should be hidden (not visible)
+ expect(modal).not.toBeVisible();
+});
diff --git a/superset-frontend/packages/superset-ui-core/src/components/ConfirmStatusChange/index.tsx b/superset-frontend/packages/superset-ui-core/src/components/ConfirmStatusChange/index.tsx
index dd52b99fd6..9aac9c01e3 100644
--- a/superset-frontend/packages/superset-ui-core/src/components/ConfirmStatusChange/index.tsx
+++ b/superset-frontend/packages/superset-ui-core/src/components/ConfirmStatusChange/index.tsx
@@ -31,14 +31,11 @@ export function ConfirmStatusChange({
const [currentCallbackArgs, setCurrentCallbackArgs] = useState<any[]>([]);
const showConfirm = (...callbackArgs: any[]) => {
- // check if any args are DOM events, if so, call persist
+ // check if any args are DOM events, if so, handle them
callbackArgs.forEach(arg => {
if (!arg) {
return;
}
- if (typeof arg.persist === 'function') {
- arg.persist();
- }
if (typeof arg.preventDefault === 'function') {
arg.preventDefault();
}