ref(cmdk): Remove onAction prop in favor of internal action execution

JonasBa · claude · JonasBa · commit a389f2774420 · 2026-04-10T06:32:25.000+02:00
CommandPalette now handles all action execution internally — navigating
for 'to' actions (with shift-key new-tab support) and invoking the
action-defined onAction callback for callback actions. Callers no longer
need to implement action dispatch logic; the only integration point is
the optional closeModal prop.

This removes the indirection of consumers inspecting CollectionTreeNode
shapes at call sites and centralises the execution model inside the
component.

Co-Authored-By: Claude Sonnet 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/static/app/components/commandPalette/__stories__/components.tsx b/static/app/components/commandPalette/__stories__/components.tsx
@@ -1,31 +1,11 @@
-import {useCallback} from 'react';
-
 import {addSuccessMessage} from 'sentry/actionCreators/indicator';
-import {CommandPaletteProvider} from 'sentry/components/commandPalette/ui/cmdk';
-import {CMDKAction} from 'sentry/components/commandPalette/ui/cmdk';
-import type {CMDKActionData} from 'sentry/components/commandPalette/ui/cmdk';
-import type {CollectionTreeNode} from 'sentry/components/commandPalette/ui/collection';
+import {
+  CMDKAction,
+  CommandPaletteProvider,
+} from 'sentry/components/commandPalette/ui/cmdk';
 import {CommandPalette} from 'sentry/components/commandPalette/ui/commandPalette';
-import {normalizeUrl} from 'sentry/utils/url/normalizeUrl';
-import {useNavigate} from 'sentry/utils/useNavigate';
 
 export function CommandPaletteDemo() {
-  const navigate = useNavigate();
-
-  const handleAction = useCallback(
-    (
-      action: CollectionTreeNode<CMDKActionData>,
-      _options?: {modifierKeys?: {shiftKey: boolean}}
-    ) => {
-      if ('to' in action) {
-        navigate(normalizeUrl(action.to));
-      } else if ('onAction' in action) {
-        action.onAction();
-      }
-    },
-    [navigate]
-  );
-
   return (
     <CommandPaletteProvider>
       <CMDKAction display={{label: 'Go to Flex story'}} to="/stories/layout/flex/" />
@@ -39,7 +19,7 @@ export function CommandPaletteDemo() {
           onAction={() => addSuccessMessage('Child action executed')}
         />
       </CMDKAction>
-      <CommandPalette onAction={handleAction}>
+      <CommandPalette>
         <CMDKAction display={{label: 'Issues List'}}>
           <CMDKAction
             display={{label: 'Select all'}}
diff --git a/static/app/components/commandPalette/ui/commandPalette.spec.tsx b/static/app/components/commandPalette/ui/commandPalette.spec.tsx
@@ -1,4 +1,4 @@
-import {Fragment, useCallback} from 'react';
+import {Fragment} from 'react';
 
 import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary';
 
@@ -24,47 +24,18 @@ jest.mock('@tanstack/react-virtual', () => ({
 
 import {closeModal} from 'sentry/actionCreators/modal';
 import * as modalActions from 'sentry/actionCreators/modal';
-import {CommandPaletteProvider} from 'sentry/components/commandPalette/ui/cmdk';
-import {CMDKAction} from 'sentry/components/commandPalette/ui/cmdk';
-import type {CMDKActionData} from 'sentry/components/commandPalette/ui/cmdk';
-import type {CollectionTreeNode} from 'sentry/components/commandPalette/ui/collection';
+import {
+  CMDKAction,
+  CommandPaletteProvider,
+} from 'sentry/components/commandPalette/ui/cmdk';
 import {CommandPalette} from 'sentry/components/commandPalette/ui/commandPalette';
 import {CommandPaletteSlot} from 'sentry/components/commandPalette/ui/commandPaletteSlot';
-import {useNavigate} from 'sentry/utils/useNavigate';
-
-function GlobalActionsComponent({
-  children,
-  onAction,
-}: {
-  children?: React.ReactNode;
-  onAction?: (
-    action: CollectionTreeNode<CMDKActionData>,
-    options?: {modifierKeys?: {shiftKey: boolean}}
-  ) => void;
-}) {
-  const navigate = useNavigate();
-
-  const handleAction = useCallback(
-    (
-      action: CollectionTreeNode<CMDKActionData>,
-      options?: {modifierKeys?: {shiftKey: boolean}}
-    ) => {
-      if (onAction) {
-        onAction(action, options);
-      } else if ('to' in action) {
-        navigate(action.to);
-      } else if ('onAction' in action) {
-        action.onAction();
-      }
-      closeModal();
-    },
-    [navigate, onAction]
-  );
 
+function GlobalActionsComponent({children}: {children?: React.ReactNode}) {
   return (
     <CommandPaletteProvider>
       {children}
-      <CommandPalette onAction={handleAction} />
+      <CommandPalette closeModal={closeModal} />
     </CommandPaletteProvider>
   );
 }
@@ -118,23 +89,27 @@ describe('CommandPalette', () => {
     expect(closeSpy).toHaveBeenCalledTimes(1);
   });
 
-  it('shift-enter on an internal link forwards modifier keys and closes modal', async () => {
+  it('shift-enter on an internal link opens in a new tab and closes modal', async () => {
     const closeSpy = jest.spyOn(modalActions, 'closeModal');
-    const onAction = jest.fn();
+    const openSpy = jest.spyOn(window, 'open').mockReturnValue(null);
 
     render(
-      <GlobalActionsComponent onAction={onAction}>
+      <GlobalActionsComponent>
         <CMDKAction to="/target/" display={{label: 'Go to route'}} />
       </GlobalActionsComponent>
     );
 
     await screen.findByRole('textbox', {name: 'Search commands'});
     await userEvent.keyboard('{Shift>}{Enter}{/Shift}');
 
-    expect(onAction).toHaveBeenCalledWith(expect.objectContaining({to: '/target/'}), {
-      modifierKeys: {shiftKey: true},
-    });
+    expect(openSpy).toHaveBeenCalledWith(
+      expect.stringContaining('target'),
+      '_blank',
+      'noreferrer'
+    );
     expect(closeSpy).toHaveBeenCalledTimes(1);
+
+    openSpy.mockRestore();
   });
 
   it('shows internal and external trailing link indicators for link actions', async () => {
@@ -398,18 +373,6 @@ describe('CommandPalette', () => {
       const secondaryCallback = jest.fn();
       const closeSpy = jest.spyOn(modalActions, 'closeModal');
 
-      // Mirror the updated modal.tsx handleSelect: invoke callback, skip close when
-      // action has children so the palette can push into the secondary actions.
-      const handleAction = (action: CollectionTreeNode<CMDKActionData>) => {
-        if ('onAction' in action) {
-          action.onAction();
-          if (action.children.length > 0) {
-            return;
-          }
-        }
-        closeModal();
-      };
-
       // Top-level groups become section headers (disabled), so the action-with-callback
       // must be a child item — matching how "Parent Group Action" works in allActions.
       render(
@@ -422,7 +385,7 @@ describe('CommandPalette', () => {
               />
             </CMDKAction>
           </CMDKAction>
-          <CommandPalette onAction={handleAction} />
+          <CommandPalette closeModal={closeModal} />
         </CommandPaletteProvider>
       );
 
@@ -522,7 +485,7 @@ describe('CommandPalette', () => {
             <CommandPaletteSlot name="task">
               <CMDKAction display={{label: 'Task Action'}} onAction={jest.fn()} />
             </CommandPaletteSlot>
-            <CommandPalette onAction={jest.fn()} />
+            <CommandPalette closeModal={jest.fn()} />
           </CommandPaletteSlot.Provider>
         </CommandPaletteProvider>
       );
@@ -541,9 +504,7 @@ describe('CommandPalette', () => {
             <CommandPaletteSlot name="task">
               <CMDKAction display={{label: 'Task Action'}} onAction={onAction} />
             </CommandPaletteSlot>
-            <CommandPalette
-              onAction={node => ('onAction' in node ? node.onAction() : null)}
-            />
+            <CommandPalette closeModal={jest.fn()} />
           </CommandPaletteSlot.Provider>
         </CommandPaletteProvider>
       );
@@ -559,7 +520,7 @@ describe('CommandPalette', () => {
             <CommandPaletteSlot name="page">
               <CMDKAction display={{label: 'Page Action'}} onAction={jest.fn()} />
             </CommandPaletteSlot>
-            <CommandPalette onAction={jest.fn()} />
+            <CommandPalette closeModal={jest.fn()} />
           </CommandPaletteSlot.Provider>
         </CommandPaletteProvider>
       );
@@ -578,9 +539,7 @@ describe('CommandPalette', () => {
             <CommandPaletteSlot name="page">
               <CMDKAction display={{label: 'Page Action'}} onAction={onAction} />
             </CommandPaletteSlot>
-            <CommandPalette
-              onAction={node => ('onAction' in node ? node.onAction() : null)}
-            />
+            <CommandPalette closeModal={jest.fn()} />
           </CommandPaletteSlot.Provider>
         </CommandPaletteProvider>
       );
@@ -606,7 +565,7 @@ describe('CommandPalette', () => {
             <CommandPaletteSlot name="page">
               <CMDKAction display={{label: 'Page Action'}} onAction={jest.fn()} />
             </CommandPaletteSlot>
-            <CommandPalette onAction={jest.fn()} />
+            <CommandPalette closeModal={jest.fn()} />
           </CommandPaletteSlot.Provider>
         </CommandPaletteProvider>
       );
@@ -630,7 +589,7 @@ describe('CommandPalette', () => {
             <CommandPaletteSlot name="task">
               <CMDKAction display={{label: 'Task Action'}} onAction={jest.fn()} />
             </CommandPaletteSlot>
-            <CommandPalette onAction={jest.fn()} />
+            <CommandPalette closeModal={jest.fn()} />
           </CommandPaletteSlot.Provider>
         </CommandPaletteProvider>
       );
@@ -659,7 +618,7 @@ describe('CommandPalette', () => {
 
       render(
         <CommandPaletteProvider>
-          <CommandPalette onAction={jest.fn()}>
+          <CommandPalette closeModal={jest.fn()}>
             <ActionsViaGlobalSlot />
           </CommandPalette>
         </CommandPaletteProvider>
@@ -678,7 +637,7 @@ describe('CommandPalette', () => {
         <CommandPaletteProvider>
           <CMDKAction display={{label: 'Empty Group'}} />
           <CMDKAction display={{label: 'Real Action'}} onAction={jest.fn()} />
-          <CommandPalette onAction={jest.fn()} />
+          <CommandPalette closeModal={jest.fn()} />
         </CommandPaletteProvider>
       );
 
@@ -692,7 +651,7 @@ describe('CommandPalette', () => {
       render(
         <CommandPaletteProvider>
           <CMDKAction display={{label: 'Direct Action'}} onAction={jest.fn()} />
-          <CommandPalette onAction={jest.fn()} />
+          <CommandPalette closeModal={jest.fn()} />
         </CommandPaletteProvider>
       );
 
diff --git a/static/app/components/commandPalette/ui/commandPalette.tsx b/static/app/components/commandPalette/ui/commandPalette.tsx
@@ -27,7 +27,10 @@ import {
   useCommandPaletteDispatch,
   useCommandPaletteState,
 } from 'sentry/components/commandPalette/ui/commandPaletteStateContext';
-import {isExternalLocation} from 'sentry/components/commandPalette/ui/locationUtils';
+import {
+  getLocationHref,
+  isExternalLocation,
+} from 'sentry/components/commandPalette/ui/locationUtils';
 import {useCommandPaletteAnalytics} from 'sentry/components/commandPalette/useCommandPaletteAnalytics';
 import {FeedbackButton} from 'sentry/components/feedbackButton/feedbackButton';
 import {LoadingIndicator} from 'sentry/components/loadingIndicator';
@@ -36,7 +39,9 @@ import {IconDefaultsProvider} from 'sentry/icons/useIconDefaults';
 import {t} from 'sentry/locale';
 import {fzf} from 'sentry/utils/search/fzf';
 import type {Theme} from 'sentry/utils/theme';
+import {normalizeUrl} from 'sentry/utils/url/normalizeUrl';
 import {useDebouncedValue} from 'sentry/utils/useDebouncedValue';
+import {useNavigate} from 'sentry/utils/useNavigate';
 
 const MotionButton = motion.create(Button);
 const MotionIconSearch = motion.create(IconSearch);
@@ -66,15 +71,13 @@ type CMDKFlatItem = CollectionTreeNode<CMDKActionData> & {
 };
 
 interface CommandPaletteProps {
-  onAction: (
-    action: CollectionTreeNode<CMDKActionData>,
-    options?: {modifierKeys?: {shiftKey: boolean}}
-  ) => void;
   children?: React.ReactNode;
+  closeModal?: () => void;
 }
 
 export function CommandPalette(props: CommandPaletteProps) {
   const theme = useTheme();
+  const navigate = useNavigate();
   const store = CMDKCollection.useStore();
 
   const state = useCommandPaletteState();
@@ -205,6 +208,7 @@ export function CommandPalette(props: CommandPaletteProps) {
     disallowTypeAhead: true,
   });
 
+  const {closeModal} = props;
   const onActionSelection = useCallback(
     (
       key: string | number | null,
@@ -224,17 +228,29 @@ export function CommandPalette(props: CommandPaletteProps) {
         if ('onAction' in action) {
           // Invoke the callback but keep the modal open so users can select
           // secondary actions from the children that follow.
-          props.onAction(action, options);
+          action.onAction();
         }
         dispatch({type: 'push action', key: action.key, label: action.display.label});
         return;
       }
 
       analytics.recordAction(action, resultIndex, '');
       dispatch({type: 'trigger action'});
-      props.onAction(action, options);
+
+      if ('to' in action) {
+        const normalizedTo = normalizeUrl(action.to);
+        if (isExternalLocation(normalizedTo) || options?.modifierKeys?.shiftKey) {
+          window.open(getLocationHref(normalizedTo), '_blank', 'noreferrer');
+        } else {
+          navigate(normalizedTo);
+        }
+      } else if ('onAction' in action) {
+        action.onAction();
+      }
+
+      closeModal?.();
     },
-    [actions, analytics, dispatch, props]
+    [actions, analytics, closeModal, dispatch, navigate]
   );
 
   const resultsListRef = useRef<HTMLDivElement>(null);
diff --git a/static/app/components/commandPalette/ui/modal.tsx b/static/app/components/commandPalette/ui/modal.tsx
@@ -1,50 +1,14 @@
-import {useCallback} from 'react';
 import {css} from '@emotion/react';
 
 import type {ModalRenderProps} from 'sentry/actionCreators/modal';
-import type {CMDKActionData} from 'sentry/components/commandPalette/ui/cmdk';
-import type {CollectionTreeNode} from 'sentry/components/commandPalette/ui/collection';
 import {CommandPalette} from 'sentry/components/commandPalette/ui/commandPalette';
 import {GlobalCommandPaletteActions} from 'sentry/components/commandPalette/ui/commandPaletteGlobalActions';
-import {
-  getLocationHref,
-  isExternalLocation,
-} from 'sentry/components/commandPalette/ui/locationUtils';
 import type {Theme} from 'sentry/utils/theme';
-import {normalizeUrl} from 'sentry/utils/url/normalizeUrl';
-import {useNavigate} from 'sentry/utils/useNavigate';
 
 export default function CommandPaletteModal({Body, closeModal}: ModalRenderProps) {
-  const navigate = useNavigate();
-
-  const handleSelect = useCallback(
-    (
-      action: CollectionTreeNode<CMDKActionData>,
-      options?: {modifierKeys?: {shiftKey: boolean}}
-    ) => {
-      if ('to' in action) {
-        const normalizedTo = normalizeUrl(action.to);
-        if (isExternalLocation(normalizedTo) || options?.modifierKeys?.shiftKey) {
-          window.open(getLocationHref(normalizedTo), '_blank', 'noreferrer');
-        } else {
-          navigate(normalizedTo);
-        }
-      } else if ('onAction' in action) {
-        action.onAction();
-        // When the action has children, the palette will push into them so the
-        // user can select a secondary action — keep the modal open.
-        if (action.children.length > 0) {
-          return;
-        }
-      }
-      closeModal();
-    },
-    [navigate, closeModal]
-  );
-
   return (
     <Body>
-      <CommandPalette onAction={handleSelect}>
+      <CommandPalette closeModal={closeModal}>
         <GlobalCommandPaletteActions />
       </CommandPalette>
     </Body>
diff --git a/static/app/components/commandPalette/useCommandPaletteActions.mdx b/static/app/components/commandPalette/useCommandPaletteActions.mdx
