Skip to content

feat(superdoc/ui): ui.commands.get(id) for dynamic toolbar lookup (SD-2814)#3013

Merged
caio-pizzol merged 2 commits into
caio/sd-2812-selection-target-alignmentfrom
caio/sd-2814-ui-commands-get
Apr 29, 2026
Merged

feat(superdoc/ui): ui.commands.get(id) for dynamic toolbar lookup (SD-2814)#3013
caio-pizzol merged 2 commits into
caio/sd-2812-selection-target-alignmentfrom
caio/sd-2814-ui-commands-get

Conversation

@caio-pizzol
Copy link
Copy Markdown
Contributor

Adds a typed string-indexed lookup on the commands surface so consumers iterating over command IDs from a config array can resolve handles without unsafe casts.

The Proxy-driven ui.commands mixes per-command handles, the register() method, and custom IDs, which makes string-indexed lookup type-error today: consumers fall back to as unknown as casts at every dispatch site. The fix is a sibling method, not a shape change to the existing surface, so per-id typing on ui.commands.bold is unaffected.

  • ui.commands.get(id) returns a unified DynamicCommandHandle for built-in or custom IDs and undefined for unknown IDs.
  • Custom takes priority on every lookup so register({ id: 'bold', override: true }) is honored without stale built-in handles.
  • The emitted state carries the source discriminator so a single render path can drive both built-ins and customs without branching.
  • Built-in dynamic handles are cached for stable identity across useMemo deps; custom handles rebuild per call to follow unregister/re-register lifecycles.

Stacked on #3011 (SD-2813). Once #3011 merges, rebase against main.

Resolves SD-2814.

Verified: pnpm --filter @superdoc/super-editor test src/ui/ -> 123 passed; pnpm --filter superdoc build:es -> bundle audit + ensure-types clean.

@caio-pizzol caio-pizzol requested a review from a team as a code owner April 29, 2026 20:04
@linear
Copy link
Copy Markdown

linear Bot commented Apr 29, 2026

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 08e11e5f96

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread packages/super-editor/src/ui/create-super-doc-ui.ts Outdated
caio-pizzol added a commit that referenced this pull request Apr 29, 2026
…ride (SD-2814)

Addresses PR #3013 review (P1): a `DynamicCommandHandle` returned by
`ui.commands.get('bold')` before a later
`register({ id: 'bold', override: true })` kept routing execute
through `toolbarController.execute('bold', ...)` even though the same
handle's observe stream now emits the merged custom state. Config
driven toolbars that memoize handles once would render the override
visually while clicks ran the original built-in.

The built-in dynamic handle now re-resolves at dispatch time:
`customCommandsRegistry.has(id)` is checked before falling back to
the toolbar controller, so override semantics hold for long-lived
handles. Two regression tests cover the override path and the
revert-after-unregister path.
Base automatically changed from caio/sd-2813-react-provider-hooks to caio/sd-2812-selection-target-alignment April 29, 2026 20:23
…-2814)

Adds a typed string-indexed lookup on the commands surface so consumers
iterating over command IDs from a config array can resolve handles
without unsafe casts.

The Proxy-driven `ui.commands` mixes per-command handles, the
`register()` method, and custom IDs, which makes string-indexed
lookup type-error today: consumers fall back to `as unknown as`
casts at every dispatch site.

`ui.commands.get(id)` returns a unified `DynamicCommandHandle` for
built-in or custom IDs and `undefined` for unknown IDs. Custom
takes priority so `register({ override: true })` is honored. The
emitted state carries the `source` discriminator so a single
render path can drive both built-ins and customs without branching.
…ride (SD-2814)

Addresses PR #3013 review (P1): a `DynamicCommandHandle` returned by
`ui.commands.get('bold')` before a later
`register({ id: 'bold', override: true })` kept routing execute
through `toolbarController.execute('bold', ...)` even though the same
handle's observe stream now emits the merged custom state. Config
driven toolbars that memoize handles once would render the override
visually while clicks ran the original built-in.

The built-in dynamic handle now re-resolves at dispatch time:
`customCommandsRegistry.has(id)` is checked before falling back to
the toolbar controller, so override semantics hold for long-lived
handles. Two regression tests cover the override path and the
revert-after-unregister path.
@caio-pizzol caio-pizzol force-pushed the caio/sd-2814-ui-commands-get branch from 0179539 to 1b74608 Compare April 29, 2026 20:27
@caio-pizzol caio-pizzol merged commit 2abb0a9 into caio/sd-2812-selection-target-alignment Apr 29, 2026
41 checks passed
@caio-pizzol caio-pizzol deleted the caio/sd-2814-ui-commands-get branch April 29, 2026 20:31
caio-pizzol added a commit that referenced this pull request Apr 29, 2026
…TextTarget (SD-2812) (#3010)

* feat(superdoc/ui): selection slice exposes SelectionTarget alongside TextTarget (SD-2812)

A consumer reading the current cursor through `ui.selection` got a
TextTarget — the right shape for `editor.doc.comments.create` and other
range mutations. The same cursor in SelectionTarget shape (kind:
'selection' with explicit start/end SelectionPoints) is what
`editor.doc.insert`, `editor.doc.text.replace`, and similar
point/range operations expect. Two shapes for the same cursor, no
public conversion, so every consumer wrote the lift manually:

  const seg = ui.selection.getSnapshot().target?.segments[0];
  const target = {
    kind: 'selection',
    start: { kind: 'text', blockId: seg.blockId, offset: seg.range.start },
    end: { kind: 'text', blockId: seg.blockId, offset: seg.range.end },
  };
  editor.doc.insert({ value, type: 'text', target });

This was real friction: the build-your-own-ui example app had to do
exactly this conversion in its custom Insert Clause command. Filed as
SD-2812.

Fix: add `selectionTarget` to `SelectionSlice`, computed from `target`
in the same memo path so identity stays stable across no-op
recomputes. Single-segment selection produces start/end on the same
blockId; multi-block uses the first segment's start and the last
segment's end (the doc-api adapter resolves inner blocks via the same
walk it already does internally).

  ui.selection.getSnapshot().target          // TextTarget — for comments / format.apply
  ui.selection.getSnapshot().selectionTarget // SelectionTarget — for insert / replace

Also re-exports `SelectionTarget`, `SelectionPoint`, `TextTarget`,
`TextSegment`, `TextAddress`, and `EntityAddress` from `superdoc/ui`
and the public `superdoc/ui` sub-entry. Consumers no longer need to
know `@superdoc/document-api` exists. Partial close on SD-2815 (the
broader type re-export pass).

Tests: 2 new (multi-block lift, null pass-through) + 2 updated for
the wider slice shape. 104 ui tests pass, tsc -b clean.

* fix(superdoc/ui): preserve story field when lifting selection target (PR #3010 review)

The TextTarget→SelectionTarget converter dropped the optional `story`
field. Mutation operations route from `target.story`; without it,
inserts and replaces against `ui.selection.selectionTarget` for a
header / footer / footnote / endnote selection would silently route
into the body and either fail to resolve the block or edit the wrong
story.

Fix: copy `story` onto every SelectionPoint and onto the
SelectionTarget root. Also fold `story` into the selection memo key
so a cursor move from one story to another (same blockId/offset by
coincidence) busts the cache and re-derives the slice.

Adds a regression test for the header-selection case.

* feat(superdoc/ui): official React provider and hooks (SD-2813) (#3011)

* feat(superdoc/ui): official React provider and hooks (SD-2813)

The build-your-own-ui example app had to write four pieces of glue
from scratch: a context provider that creates one controller per app,
a hook to read it, a hook that subscribes a component to a slice of
controller state, and the unmount lifecycle that destroys the
controller. Two of those are easy to get wrong — the unmount in
particular has a stale-closure bug (`useEffect(() => () =>
ui?.destroy(), [])` captures the initial null, leaks subscriptions on
unmount).

Ship the bindings officially so consumers don't reinvent them.

  import {
    SuperDocUIProvider,
    useSuperDocUI,
    useSuperDocHost,
    useSetSuperDoc,
    useSuperDocSlice,
    useSuperDocSelection,
    useSuperDocComments,
    useSuperDocReview,
    useSuperDocToolbar,
    useSuperDocCommand,
  } from 'superdoc/ui/react';

Plumbing
  - New `./ui/react` exports entry on @superdoc/super-editor.
  - New Vite build input emitting `dist/ui-react.es.js`.
  - Public sub-entry on the superdoc package: `superdoc/ui/react`.
  - Vite alias ordered before `/ui` so the longer path matches first.
  - `react` and `react/jsx-runtime` externalized in the rollup config
    so the built artifact stays peer-dep-friendly.
  - tsconfig grows `"jsx": "react-jsx"` so `.tsx` files compile.
  - @testing-library/react + react-dom + @types/react-dom added as
    devDeps.

Verified
  - 10 new tests (6 provider, 4 domain hooks); 114 ui tests pass total
  - tsc -b clean
  - superdoc build emits dist/ui-react.es.js with react +
    react/jsx-runtime as external imports
  - bundle audit still clean

Open follow-ups: useSuperDocCustomCommand once `ui.commands.get(id)`
lands (SD-2814). Vue equivalent deferred until the React surface is
exercised against real consumer apps.

* fix(superdoc/ui): address PR #3011 review (StrictMode + id resubscribe + types) (SD-2813)

Four review fixes from the React-bindings PR:

1. SuperDocUIProvider.setSuperDoc no longer constructs the controller
   inside a setUI((prev) => ...) updater. Under React StrictMode, React
   double-invokes state-updater functions for purity-checking, which
   would call createSuperDocUI() twice and leak one controller's
   editor / SuperDoc subscriptions per call. Construction now lives
   in the callback body and the prior controller is torn down via the
   uiRef. A regression test reproduces the leak (24 editor.on calls
   under StrictMode without the fix vs 12 with).

2. useSuperDocCommand(id) bypasses useSuperDocSlice and subscribes
   with [ui, id] effect deps. The selector closes over id, but
   useSuperDocSlice's effect only re-runs when ui changes, so a
   toolbar that reuses one component slot with different command ids
   would observe the prior command forever. A regression test
   reproduces the stale read.

3. typesVersions adds the ui/react entry so TypeScript projects on
   the legacy moduleResolution: "node" can resolve declarations.
   exports already had it, but typesVersions is what matters for
   classic resolution (the project follows this pattern for every
   other public subpath).

4. super-editor's vite build externalizes react/jsx-runtime defensively.
   The published consumer path (superdoc/ui/react) already externalized
   correctly via aliases, but this keeps the intermediate
   @superdoc/super-editor dist compatible with React 17/18 hosts in
   case any pnpm-link / examples consumer reaches it directly.

* feat(superdoc/ui): ui.commands.get(id) for dynamic toolbar lookup (SD-2814) (#3013)

* feat(superdoc/ui): ui.commands.get(id) for dynamic toolbar lookup (SD-2814)

Adds a typed string-indexed lookup on the commands surface so consumers
iterating over command IDs from a config array can resolve handles
without unsafe casts.

The Proxy-driven `ui.commands` mixes per-command handles, the
`register()` method, and custom IDs, which makes string-indexed
lookup type-error today: consumers fall back to `as unknown as`
casts at every dispatch site.

`ui.commands.get(id)` returns a unified `DynamicCommandHandle` for
built-in or custom IDs and `undefined` for unknown IDs. Custom
takes priority so `register({ override: true })` is honored. The
emitted state carries the `source` discriminator so a single
render path can drive both built-ins and customs without branching.

* fix(superdoc/ui): cached dynamic handle dispatches through later override (SD-2814)

Addresses PR #3013 review (P1): a `DynamicCommandHandle` returned by
`ui.commands.get('bold')` before a later
`register({ id: 'bold', override: true })` kept routing execute
through `toolbarController.execute('bold', ...)` even though the same
handle's observe stream now emits the merged custom state. Config
driven toolbars that memoize handles once would render the override
visually while clicks ran the original built-in.

The built-in dynamic handle now re-resolves at dispatch time:
`customCommandsRegistry.has(id)` is checked before falling back to
the toolbar controller, so override semantics hold for long-lived
handles. Two regression tests cover the override path and the
revert-after-unregister path.

* feat(superdoc/ui): re-export public document types (SD-2815) (#3014)

* feat(superdoc/ui): re-export public document types (SD-2815)

The browser UI controller surfaces document-side shapes everywhere:
state.comments.items returns CommentInfo records, action methods
return Receipt, ui.viewport.scrollIntoView accepts ScrollIntoViewInput,
state.review.items references TrackChangeInfo, etc. Consumers typing
their components had to reach into @superdoc/document-api directly,
which isn't on the recommended import path.

Re-exports the controller-surfaced doc-api types from superdoc/ui
and packages/superdoc/src/ui.d.ts so consumers can write the
custom-toolbar / sidebar example types entirely from one entrypoint.
The types resolve to the same shapes reached through the root
superdoc import - parity asserted in customer-scenario.ts via
distribution-equivalence checks.

Adds:
- CommentInfo / CommentsListQuery / CommentsListResult
- TrackChangeInfo / TrackChangesListResult
- Receipt
- ScrollIntoViewInput / ScrollIntoViewOutput
- SelectionInfo
- CommentAddress / TrackedChangeAddress

* fix(superdoc/ui): ship real doc-api types for packed consumers (PR #3014 review)

The PR #3014 (SD-2815) re-exports added on superdoc/ui resolved
through @superdoc/document-api, which is private to the workspace
and not published. The ensure-types.cjs post-build step generated an
ambient `declare module '@superdoc/document-api' { ... = any }` shim
in dist/_internal-shims.d.ts so consumer compiles wouldn't error,
but every doc-api type re-exported through superdoc/ui (CommentInfo,
Receipt, SelectionInfo, TextTarget, etc.) collapsed to `any` for
packed consumers. Components typed against these shapes had no
checking, defeating the purpose of the new public surface.

Three coordinated changes:

1. vite.config.js dts plugin now includes ../document-api/src/**/*
   so the document-api types emit into dist/document-api/. tsconfig
   include matches.
2. ensure-types.cjs rewrites every bare @superdoc/document-api
   specifier in emitted .d.ts files to a relative path into
   dist/document-api/, and skips the package when generating the
   _internal-shims.d.ts ambient declarations (parity with how
   @superdoc/super-editor was already handled).
3. tests/consumer-typecheck/customer-scenario.ts adds an
   `IsNotAny<T>` distribution check on every newly re-exported
   doc-api type. If a future change drops the document-api dist or
   loses the import-rewrite, the consumer-typecheck fails (assigning
   `boolean` to `true`).

Verified end-to-end by packing superdoc, installing into
consumer-typecheck, running tsc --noEmit. The IsNotAny guards pass;
_internal-shims.d.ts no longer carries the doc-api types.

* feat(superdoc/ui): ui.selection.capture for sidebar / floating-menu composers (SD-2821) (#3016)

* feat(superdoc/ui): ui.selection.capture for sidebar / floating-menu composers (SD-2821)

A sidebar comment composer or floating menu takes focus into its
own input element when it opens; the editor's selection visually
clears and `state.selection.target` becomes null. A consumer that
calls `editor.doc.comments.create({ target })` on submit then has
no anchor and the create rejects.

Adds `ui.selection.capture(): SelectionCapture | null` so consumers
freeze the addressable selection at the moment the composer opens
(or the menu mounts) and pass `captured.target` /
`captured.selectionTarget` straight into `editor.doc.*` actions
when the composer submits. The captured handle is `Object.freeze`d
so a stored reference can't be accidentally mutated across renders.

Visual restore (re-focus the editor + re-highlight the captured
range) is intentionally NOT on this surface yet. The public
Document API has no `selection.set` primitive today, and routing
through `editor.commands.*` would skip the contract this controller
is explicitly built on. A `restore(capture)` method lands once the
doc-api primitive does.

Returns null on non-text selections, no-editor state, or pre-ready
snapshots so the consumer's null-guard is the explicit "capture
isn't applicable here" signal instead of silent failure later in
the flow.

3 new tests: addressable-selection capture, null-on-no-anchor,
captured value survives a later live-selection clear (the
documented sidebar-composer scenario). 131 ui tests pass; bundle
audit clean.

* fix(superdoc/ui): deep-freeze captured selection (PR #3016 review)

The shallow `Object.freeze({ ...slice })` in `ui.selection.capture()`
left nested fields (target, target.segments, activeMarks array)
mutable AND sharing references with the controller's memoized
selection slice. A consumer doing
`captured.target.segments[0].range.start = 99` or
`captured.activeMarks.push('foo')` would corrupt the shared
snapshot every other subscriber sees and feed bad targets into
later editor.doc.* calls, despite the API promising a frozen
captured handle.

Capture now deep-clones the slice before deep-freezing the clone.
The clone breaks the reference share with the memo so a frozen
captured handle is genuinely independent. JSON-style structural
clone is sufficient: the selection slice is plain data with no
functions, Dates, Maps, or cycles. Recursive freeze short-circuits
on already-frozen values so EMPTY_ACTIVE_IDS doesn't loop.

Regression test asserts every nested field is frozen and that
strict-mode mutation attempts on captured.target.segments[0].range
and captured.activeMarks throw, leaving the live snapshot from
ui.selection.getSnapshot() unaffected.

* fix(superdoc/ui): unify command dispatch + handle identity + memo key + polish (PR #3010 review)

Six review findings rolled into one commit. Each verified and tested.

1. Memo key for selection slice was using made-up StoryLocator
   field names (`story.type` / `story.id`) instead of the real
   discriminated-union shape (`storyType` / `refId` / `noteId` /
   `section` / `headerFooterKind` / `variant`). Two selections in
   different stories collapsed to the same key, defeating the memo
   bust the comment claimed. Fixed to walk every discriminating
   field; once the doc-api resolver starts stamping `target.story`
   for non-body surfaces (separate ticket), cross-story navigation
   correctly invalidates the slice.

2. Override-routing inconsistency. `ui.commands.get(id)?.execute()`
   re-resolved through the custom registry on every call, but
   `ui.commands.bold.execute()` and `ui.toolbar.execute('bold')`
   went straight to the headless-toolbar built-in. After
   `register({ id: 'bold', override: true })`, `state.toolbar.commands.bold`
   showed `source: 'custom'` while clicks via the per-id /
   aggregate surfaces ran the original built-in. Centralized as
   `dispatchCommand(id, payload)`; all three paths use it. Two
   regression tests cover the per-id and aggregate surfaces.

3. Stale custom-command execute / observe through replacement.
   `regA.handle.execute()` after a custom-vs-custom replace ran
   B's executor because `buildHandle` closed only over `id` and
   `registry.execute(id)` was identity-blind. Bound the handle to
   its own `InternalCustomEntry` and added an identity check on
   every execute and observe emit: stale handles return `false`
   from execute and detach from observe.

4. Stale JSDoc on `ui.comments.reopen`: said the doc-api
   "currently throws INVALID_INPUT". SD-2789 shipped the lifecycle
   inverse; updated the prose to match.

5. Stale "skeleton" framing in the top-level types.ts JSDoc:
   the surface is no longer a skeleton. Reframed to describe the
   substrate vs. domain handles split.

6. `SelectionCapture` was typed as `SelectionSlice` even though
   the runtime calls `deepFreeze`. Changed to `DeepReadonly<SelectionSlice>`
   so the static type matches reality; consumer mutations on
   nested fields are TypeScript errors at compile time.

Plus: added a JSDoc paragraph on `SelectionSlice.selectionTarget`
documenting the known gap that the doc-api resolver doesn't yet
stamp `target.story` for non-body surfaces (header / footer /
footnote / endnote). Story preservation in the lift is honored
once the resolver starts stamping; tracked as a separate doc-api
ticket.

Plus: added `src/ui-react.js` to the superdoc package's coverage
exclude list. The file is a pure re-export barrel like the other
already-excluded `superdoc/headless-toolbar*` and `superdoc/ui`
barrels; missing it from the list caused the codecov patch report
to flag 8 missing lines for SD-2813.

Verified: 136 ui tests pass (4 new regressions); bundle audit clean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant