Skip to content

ref(cmdk) jsx powered cmdk#112262

Open
JonasBa wants to merge 40 commits intomasterfrom
jb/cmdk/jsx-poc
Open

ref(cmdk) jsx powered cmdk#112262
JonasBa wants to merge 40 commits intomasterfrom
jb/cmdk/jsx-poc

Conversation

@JonasBa
Copy link
Copy Markdown
Member

@JonasBa JonasBa commented Apr 6, 2026

Refactor CMDK to a composable JSX approach.

Previously, our CMDK required knowing actions aot and use a hook registration process. We want to move to a fully JSX powered approach and eventually also make actions render to the DOM (this is currently not possible because virtualization is required and the listbox API requires us to specify the full list)

This change brings us half way to being able to power the CMDK UI by converting CMDK registration to JSX.

This enables some interesting patterns where the UI logic can be colocated with the UI as seen here.

The way that this works is through the following mechanisms:

  • Collection context responsible for storing nodes and edges (essentially a recursive Context) that we use to maintain a parent<->child mapping.
  • A store with a tree(root | undefined) method which will reconstruct the tree on-demand so that callers can have access to the most complete UI at all times

The slots API mentioned above this are used in CMDK as contextual priority slots that will appear as the first descendants of the CMDK action (similar to how priority int value used to work before). CMDK now has two slots, the first one is the global slot where global actions should be registered, the second is the page where broader page specific actions can be registered and the 3rd is the task slot, a slot that is contextual to what the user is doing (e.g. if they are editing a dashboard, saving or starring it might be a task they are currently interested).

JonasBa and others added 2 commits April 5, 2026 19:35
…gistration

Introduces a makeCollection<T>() factory in ui/collection.tsx that creates isolated,
fully-typed collection instances. Each instance tracks a flat node map and a
parent→children index, exposing a tree(rootKey?) API for structured traversal.

A node becomes a group by wrapping its children in GroupContext.Provider — there
is no separate group/item type distinction. Data fields are spread directly onto
tree nodes so consumers can access them without any cast.

ui/cmdk.tsx builds the CMDK-specific layer on top: CMDKCollection, CMDKGroup,
and CMDKAction. Groups and actions share a single CMDKActionData union type.
CMDK_PLAN.md tracks the remaining migration steps from the old reducer-based
registration system.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wire CMDKQueryContext and CMDKProvider so async CMDKGroup nodes can read
the current search query. Mount CMDKProvider inside CommandPaletteStateProvider
in the existing CommandPaletteProvider so the collection store is live for the
full app lifetime.

Add GlobalCommandPaletteActions component that registers all global actions
into the CMDK collection via JSX (CMDKGroup / CMDKAction) rather than the old
useCommandPaletteActionsRegister reducer hook. Both systems run in parallel
during this transition step.

Swap the CommandPalette UI to read from CMDKCollection.useStore() instead of
the old useCommandPaletteActions() reducer pipeline. Remove collectResourceActions,
asyncQueries, and asyncChildrenMap — async fetching is now handled inside
CMDKGroup before nodes appear in the tree. Rewrite scoreTree and flattenActions
to work with CollectionTreeNode<CMDKActionData> using direct field access.

Replace the linked-list navigation stack (which stored full action objects)
with CMDKNavStack which stores only the group key and label. Push action now
dispatches {key, label} instead of a full CommandPaletteActionWithKey.

Update modal.tsx, stories, analytics hook, and tests to use the new types.
Fix pre-existing duplicate Item declaration in commandPalette.tsx that caused
a Babel parse error in the test suite.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 6, 2026
JonasBa and others added 2 commits April 6, 2026 08:55
- Remove CMDK_PLAN.md (internal planning doc, not production code)
- Rename poc.spec.tsx → collection.spec.tsx (no longer a PoC)
- Remove dead useGlobalCommandPaletteActions() call from navigation — commandPalette.tsx
  now reads from CMDKCollection.useStore(), so the old context registration had no effect
- Rewrite stories demo to use CMDKAction/CMDKGroup JSX API; the old
  useCommandPaletteActionsRegister path fed CommandPaletteActionsContext which nothing
  reads anymore, so the demo was showing an empty palette

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
GlobalCommandPaletteActions covers everything the old hook registered.
The hook had no remaining callers after the previous commit removed the
navigation/index.tsx call site.

Also cleans up cmdk.tsx comments and renames GroupContext to Context in
the collection factory API.

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
JonasBa and others added 2 commits April 6, 2026 09:22
The old context.tsx served as a registration hub for the reducer-based
action system. With that system removed, it only wrapped two providers.
Move CommandPaletteProvider directly into cmdk.tsx alongside the other
palette primitives and delete the now-empty file.

Also remove unused WithKey type variants from types.tsx.

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Move GlobalCommandPaletteActions from globalActions.tsx into the ui/
directory alongside the other palette files, and render it inside the
modal rather than in the navigation layout.

The global actions now mount and unmount with the palette itself instead
of running in the navigation tree at all times. CommandPalette stays a
generic rendering component with no app-specific dependencies, keeping
it fully testable in isolation.

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Add a children prop so callers can pass CMDKGroup/CMDKAction trees
directly to CommandPalette. The modal uses this to inject
GlobalCommandPaletteActions, keeping the component itself free of
app-specific dependencies.

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
CMDKAction now accepts LocationDescriptor for the to prop, so String()
is no longer needed and was triggering a no-base-to-string lint error.

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
JonasBa and others added 2 commits April 6, 2026 09:32
…onContext

The modal renders outside SecondaryNavigationContextProvider, so
useSecondaryNavigation() threw on open. Read the context directly and
skip the sidebar toggle action when the context isn't available.

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Moving it into the modal changed its React scope — the modal renders
in a portal outside SecondaryNavigationContextProvider, causing a crash.

Since CommandPaletteProvider is at the app root, CMDKCollection is shared
across the whole tree. GlobalCommandPaletteActions can register from the
navigation (where it has proper context) and the modal reads the same store.

The children prop on CommandPalette is still useful for page-scoped actions.

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
JonasBa and others added 7 commits April 6, 2026 09:53
…ing providers

Add a test asserting that page slot actions appear before global actions in
the command palette list. The test currently fails — it documents the known
bug where collection insertion order (driven by React mount order) determines
list position instead of slot priority.

Also fixes two pre-existing issues uncovered while writing the test:
- GlobalActionsComponent in the spec was missing CommandPaletteSlot.Provider,
  causing all existing tests to throw 'SlotContext not found' at render time.
- GlobalCommandPaletteActions was registering actions directly into the
  CMDKCollection without going through a slot consumer. It now wraps its
  output in <CommandPaletteSlot name="global">, making all action
  registration slot-aware and setting up the path toward DOM-order-based
  priority sorting.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Documents the approach for fixing slot ordering in the command palette:
store a ref to the outlet DOM element on each registered action node,
then pre-sort the collection output via compareDocumentPosition before
passing to flattenActions.

Covers the ref-threading chain (shared SlotRefsContext → outlet populates
→ consumer wrapper injects → node stores), the new commandPaletteSlotRefs.tsx
file needed to avoid circular imports, and all six files that need changing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pre-sort the root-level CMDK collection nodes by the DOM position of
their slot outlet element before flattening, so that task > page >
global priority is preserved regardless of React mount order.

The slot library is extended with two additions:
- SlotConsumer now provides OutletNameContext so portaled children
  see which slot they belong to (previously only the outlet provided it)
- useSlotOutletRef() hook on SlotModule returns a stable RefObject
  whose .current is always the current slot's outlet element

CommandPaletteSlot is extracted to commandPaletteSlot.tsx to avoid a
circular import between cmdk.tsx (which needs the hook) and
commandPalette.tsx (which imports the collection).

CMDKAction and CMDKGroup call CommandPaletteSlot.useSlotOutletRef() and
store the result as ref in their node data. presortBySlotRef in
commandPalette.tsx then uses compareDocumentPosition on those elements
to establish the correct order. Nodes outside any slot wrapper (ref is
null) sort after all slotted nodes and preserve their relative order.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extend CMDKActionData with a ref field that stores the slot outlet
element. CMDKAction and CMDKGroup call useSlotOutletRef and include it
in their registered node data.

commandPalette.tsx applies presortBySlotRef to collection nodes before
flattening, ordering by the outlet DOM position (task > page > global).
Callers use CommandPaletteSlot directly — no wrapper components needed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
JonasBa and others added 3 commits April 6, 2026 14:07
CMDKGroup was overwriting any enabled field returned by the resource
function with a static `!!resource` check. This meant the DSN lookup
query always fired regardless of whether the query matched a DSN pattern.

Fix CMDKGroup to respect the resource's own enabled field, then add
`enabled: DSN_PATTERN.test(query)` to the DSN lookup resource so the
API call only fires when the query looks like a DSN.

Also remove an unused GlobalCommandPaletteActions import in navigation/index.tsx.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Converts the old-style CommandPaletteAction[] fixture format into the new
* JSX registration components so tests don't need to be fully rewritten.
*/
function ActionsToJSX({actions}: {actions: CommandPaletteAction[]}) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is just the spec, I'd maybe say we just have claude update this file to just use the new JSX actions inline in the spec tests. Not a huge deal but should be easy enough then we're on the best practice moving forward

| CommandPaletteAsyncResultGroup;
| CommandPaletteActionCallback;

export type CMDKQueryOptions = UseQueryOptions<
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not touched here but should we type the output to be CommandPaletteAction | CommandPaletteAction[]? Still a tanstack newbie so that might be overkill, but I know we want the select transform to give us an action/array of actions so I don't see why we wouldn't require that unless that's unnecessary

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, this is an intermediary ttype that we can remove, I agree!

</CMDKAction>
</CMDKAction>

{user.isStaff && (
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point it might be good for us to pull these global staff actions out into their own component to keep this file simple (among potentially pulling out other global actions) but good for now

Use `CMDKAction` JSX components inside your page or feature component to register contextual actions with the global command palette. Actions are registered on mount and automatically unregistered on unmount, so they only appear in the palette while your component is rendered. This is ideal for page‑specific shortcuts.

There are a few different types of actions you can register:
Wrap your tree in `CommandPaletteProvider` and place the `CommandPalette` UI component wherever you want the dialog to render. Then declare actions anywhere inside the provider:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be worth touching on which slot a user can add to? really should only be task/page a dev could add, but probably worth calling out the difference/use case for each

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, my plan is to add a skill that will prompt the user to do this

const prefix = `/organizations/${organization.slug}`;

return (
<CommandPaletteSlot name="global">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any "default" slot if not specified - might be missing in but which slot do CMDKActions that aren't registered within an explicit slot get added to? I'd assume page by default, but again don't see anything around that

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<CommandPaletteSlot.Outlet name="global">
  {p => (
    <div {...p} style={{display: 'contents'}}>
      {props.children}
    </div>
  )}
</CommandPaletteSlot.Outlet>

This may be it with the props.children rendering, but wondering if we should move into page?

…alias

Replace the ActionsToJSX bridge component in tests with direct JSX CMDKAction
registrations. Remove the CommandPaletteAsyncResult type alias in favour of
CommandPaletteAction, which is the same shape and avoids the unnecessary split.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…es (#112650)

Move CommandPalette Slot outside of the conditionally rendered
CommandPalette which makes the action registration stable and enables
the user to toggle the modal on/off without losing state

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
The merge of master into jb/cmdk/jsx-poc accidentally reverted the fix
from 7508757 (fix(slot): Render nothing when no outlet is registered).
The conflict resolution kept the old 'render in place' fallback instead of
the correct 'return null' behavior, causing two slot tests to fail.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 006c238. Configure here.

import {addSuccessMessage} from 'sentry/actionCreators/indicator';
import {
CMDKAction,
CMDKAction,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate CMDKAction import in documentation example

Low Severity

CMDKAction is imported twice on consecutive lines in the documentation code example. This is a copy-paste error that, while not causing a runtime issue, sets a bad example for developers copying the snippet.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 006c238. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants