From c8a6059eaa0c8c226f83176ae2d349ddb0e15e98 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 12 May 2026 14:32:26 -0700 Subject: [PATCH] perf(DatasourceEditor): stop rebinding ctrl+shift+f on every SQL keystroke The Mousetrap binding for ctrl+shift+f depended on `onQueryFormat`, which itself depends on `datasource.sql`. Every keystroke in the SQL editor recreated `onQueryFormat`, which re-fired the update effect, which unbound and rebound the global keyboard shortcut. That's a lot of Mousetrap churn for a 'format SQL' shortcut. Bind once on mount; route through refs (`isEditModeRef`, `onQueryFormatRef`) that the per-render effect keeps current. Per @sadpandajoe's review comment on the FC conversion PR. Co-Authored-By: Claude Sonnet 4.6 --- .../DatasourceEditor/DatasourceEditor.tsx | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx index e899c849ff9..f6b5169fcc6 100644 --- a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx +++ b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx @@ -1366,10 +1366,13 @@ function DatasourceEditor({ // Mark initial mount as complete after first render cycle // This prevents useEffect hooks from firing on mount isInitialMount.current = false; + // Bind ctrl+shift+f once on mount and route through refs so we don't + // unbind/rebind on every SQL-editor keystroke (onQueryFormat's identity + // changes with datasource.sql). Mousetrap.bind('ctrl+shift+f', e => { e.preventDefault(); - if (isEditMode) { - onQueryFormat(); + if (isEditModeRef.current) { + onQueryFormatRef.current?.(); } return false; }); @@ -1391,16 +1394,12 @@ function DatasourceEditor({ }; }, []); - // Update Mousetrap binding when isEditMode changes + // Keep the refs read by the (one-shot) Mousetrap handler up to date. + const isEditModeRef = useRef(isEditMode); + const onQueryFormatRef = useRef(onQueryFormat); useEffect(() => { - Mousetrap.unbind('ctrl+shift+f'); - Mousetrap.bind('ctrl+shift+f', e => { - e.preventDefault(); - if (isEditMode) { - onQueryFormat(); - } - return false; - }); + isEditModeRef.current = isEditMode; + onQueryFormatRef.current = onQueryFormat; }, [isEditMode, onQueryFormat]); // componentDidUpdate for props.datasource changes