Feat/v2.0 api standardisation#313
Merged
Merged
Conversation
Adds the §17 three-category model as TypeScript types under their final names, in a new staging file that nothing imports yet. No behaviour change, no public-API change (not re-exported from index.ts), no demo change — each type is wired into the implementation (replacing its legacy counterpart in types.ts) by a later phase. - src/apiTypes.ts: JsonEditorError + JsonEditorErrorCode (Group A/B), the async-capable FilterFunction, the discriminated InterceptableEvent / UpdateFunctionProps / EditEvent unions, UpdateResult, the Cat-3 observers, and the CommandResult / JsonEditorHandle imperative surface. - test/apiTypes.test.ts: compile-check + sample usage. Because apiTypes.ts is outside the `files: ["src/index.ts"]` graph, tsc never reaches it; ts-jest type-checks this test, so a malformed union (e.g. an event branch that stops narrowing) fails `pnpm test`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Splits the dual-purpose `enableClipboard?: boolean | CopyFunction` into a
single-purpose Cat-1 gate `allowClipboard?: boolean` (default true) plus a
Cat-3 observer `onCopy?: OnCopyFunction`. Retires `CopyFunction`.
- onCopy receives the flat NodeData + { success, stringValue, type, error? };
a clipboard-write failure is reported as `error: { message }` (not a coded
error — clipboard failures aren't part of the §17 error taxonomy).
- OnCopyFunction graduates from apiTypes.ts staging into the public types.ts +
index.ts; InterceptableEvent/EventInterceptFunction stay in staging.
This is carved out of the original Phase 1 work so the §17 phases can proceed
without the `onEventIntercept` mechanism, which is parked for a design rethink
(complexity / bundle cost / the confirm-gate-vs-Tab question) — see PR #301.
Tests: test/clipboard.test.tsx (allowClipboard toggle + onCopy payload). Docs:
README props row + Copy Function section + type list; migration-guide §8; demo
updated to allowClipboard + onCopy.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collapse the update API onto the three-category model's result producers:
- Remove discrete onEdit/onAdd/onDelete — onUpdate is the sole result
producer, taking a discriminated `event` ('edit'|'add'|'delete'|'rename'|
'move') on a flat NodeData payload. rename + move are now first-class
events (previously disguised as edits).
- Unify the return shape into UpdateResult: true|void|false|null|{value?|error?}.
Drops the legacy ['value'|'error', x] tuple and bare-string returns. `null`
is new — a silent abort (no commit, no error).
- onChange aligned to (NodeData & { newValue }) => ValueData (clean break:
currentData→fullData, currentValue→value, name→key).
- Graduate the canonical types from apiTypes.ts: JsonEditorError /
JsonEditorErrorCode (retiring JerError), UpdateResult, UpdateFunctionProps,
UpdateFunction, OnChangeFunction.
- ValueNodeWrapper: revert the display explicitly on reject AND cancel via a
shared revertToData(), dropping the `error` dependency from the resync
effect — matching CollectionNode's already-explicit buffer model.
Demo + README + migration-guide (§9) updated; tests rewritten to the new
contract plus new cases for null-cancel, reject-revert, and each event.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#298 Reshape the observer callbacks onto the three-category model: - onEditEvent: from `(path, isKey)` to the full discriminated lifecycle stream (start*/confirm*/cancel* for edit/rename/add + instant delete/move) on flat NodeData. A session ends in confirm* (committed) or cancel* (closed without a commit — explicit cancel, no-op confirm, or a rejected/aborted change). Absorbs onRenameProperty (§12). - onError / onCollapse: flat `NodeData & { ... }` payloads (CollapseState stays as the editorRef.collapse command input). - onCopy: error aligned to JsonEditorError (new CLIPBOARD_ERROR code). Mechanics: - Graduate EditEvent/OnEditEventFunction + flat OnErrorFunction/OnCollapseFunction from apiTypes.ts to types.ts; export EditEvent. - Editing store: silent `closeEdit` (clears cancelOp — avoids a Tab-commit reverting via the stale cancelOp) used by commit/confirm flows; `cancelEdit` fires cancel* (snapshot-before-null) for true/external cancels (Esc, node switch, editorRef.cancelEdit, search). start* fires from startEdit. - Terminal confirm*/cancel*/delete/add fire from the node handlers (which hold both NodeData and the commit outcome); `move` fires from the internal onMove (it owns the source NodeData). onEdit's no-op now returns the `false` sentinel so the node reports it as a cancel. - Shared `buildNodeDataFromPath` ref bridges live NodeData from the inner Editor into the editing/collapse providers (its ancestors) for path-only events. Demo + README "Event callbacks" + migration-guide §10 + roadmap updated; tests rewritten (renderScope onError → fullData, collapseBroadcasts → flat payload) plus a new onEditEvent lifecycle suite and an onCopy CLIPBOARD_ERROR case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…) — #299 Replace the §10 editorRef handle with the canonical §17 Category 4 handle. The handle is UI-interactions only: session openers (startEdit / startRename / startAdd), shared confirm()/cancel(), and collapse. The direct mutators (delete/edit/add/rename/move) sketched in the roadmap were dropped — a consumer owns data/setData, so a data-mutating command is a redundant second path. - Editing store gains a `sessionCommit` slot + registerSessionCommit / confirmSession (mirroring the cancelOp closure). Each session-owning node registers a committer via a stable commitRef indirection so confirm() always sees the live buffer; commit handlers now return the canonical UpdateResult outcome (void | false | JsonEditorError). - Add session lifted into the store as `mode: 'add'` so confirm/cancel and the one-session-at-a-time invariant are shared with edit/rename; the options payload stays local, synced off the store flag. - Session openers (sync) validate path existence (PATH_NOT_FOUND, dev-warn) and the restrict* filter (RESTRICTED) at the root against the live tree; overrideRestrictions skips ONLY the filter, never onUpdate. confirm() (async) commits the open session through onUpdate and maps accept/reject to CommandResult. - Graduate JsonEditorHandle / CommandResult into types.ts (trim the dropped mutators from the apiTypes staging + compile-check); export CommandResult. - Decouples Phase 4 from #117 (resume is the consumer's own setData, not editorRef.delete) and moots the direct-mutation half of #286. Tests: rewrite test/imperativeHandle.test.tsx (openers + confirm across value/key/add, PATH_NOT_FOUND/RESTRICTED, override-but-onUpdate-vetoes); update apiTypes compile-check. Docs: README handle section, migration-guide §7, demo External Control panel, V2-roadmap §17 mutators-dropped note, and the editor-ref changeset. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Tier 1 removed editorRef.confirm()'s session-commit registry, which was the only consumer of the canonical outcome the node commit handlers returned. Those returns were dead afterward — every caller ignores them, and KeyDisplay's prop type already declared handleEditKey as `=> void`. Strip them: handleEdit (ValueNodeWrapper + CollectionNode), handleAdd (CollectionNode), handleEditKey (useCommon) and commitAdd (ButtonPanels) are now plain void side-effecting handlers (their v2.0-dev shape) plus the §17 onEditEvent firing — no return-type gymnastics in the .then callbacks, error objects inlined into onError, the Promise.resolve wrappers and the mapAdd promise-mapper gone. Removes the now-unused JsonEditorError imports (kept in useCommon, used by its onError param) and simplifies EditButtonProps.handleAdd to `=> void`. No public-API or behaviour change: UpdateFunction/onUpdate keep their UpdateResult return shape, the internal onEdit/onDelete/onAdd/onRename pipeline is untouched, and the same commits/errors/events fire. All 284 tests pass unchanged. Also evaluated removing the buildNodeDataFromPath provider bridge and KEPT it (load-bearing for provider-fired onEditEvent start/cancel + once-per-command onCollapse broadcasts, per the §16 store-in-provider design) — documented why at its definition so it reads as intentional, not a leak. Core ESM gzip 19,124 -> 19,096 B (-28 B); §17 batch now 19,673 -> 19,096 (-577). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the §17 three-category model as TypeScript types under their final names, in a new staging file that nothing imports yet. No behaviour change, no public-API change (not re-exported from index.ts), no demo change — each type is wired into the implementation (replacing its legacy counterpart in types.ts) by a later phase. - src/apiTypes.ts: JsonEditorError + JsonEditorErrorCode (Group A/B), the async-capable FilterFunction, the discriminated InterceptableEvent / UpdateFunctionProps / EditEvent unions, UpdateResult, the Cat-3 observers, and the CommandResult / JsonEditorHandle imperative surface. - test/apiTypes.test.ts: compile-check + sample usage. Because apiTypes.ts is outside the `files: ["src/index.ts"]` graph, tsc never reaches it; ts-jest type-checks this test, so a malformed union (e.g. an event branch that stops narrowing) fails `pnpm test`. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* §17: clipboard split — enableClipboard → allowClipboard + onCopy (#296) Splits the dual-purpose `enableClipboard?: boolean | CopyFunction` into a single-purpose Cat-1 gate `allowClipboard?: boolean` (default true) plus a Cat-3 observer `onCopy?: OnCopyFunction`. Retires `CopyFunction`. - onCopy receives the flat NodeData + { success, stringValue, type, error? }; a clipboard-write failure is reported as `error: { message }` (not a coded error — clipboard failures aren't part of the §17 error taxonomy). - OnCopyFunction graduates from apiTypes.ts staging into the public types.ts + index.ts; InterceptableEvent/EventInterceptFunction stay in staging. This is carved out of the original Phase 1 work so the §17 phases can proceed without the `onEventIntercept` mechanism, which is parked for a design rethink (complexity / bundle cost / the confirm-gate-vs-Tab question) — see PR #301. Tests: test/clipboard.test.tsx (allowClipboard toggle + onCopy payload). Docs: README props row + Copy Function section + type list; migration-guide §8; demo updated to allowClipboard + onCopy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Generic rejection (onUpdate returning `false`) now picks an event-specific localisation key — ERROR_ADD / ERROR_DELETE / ERROR_UPDATE — so the surfaced message matches the ADD_ERROR / DELETE_ERROR code the node routes to onError, instead of always showing "Update unsuccessful". rename/move fall back to ERROR_UPDATE (their code is UPDATE_ERROR); dedicated keys tracked in #308. Regression tests cover the delete- and add-specific reject messages. Demo's onUpdate wrapper now passes `false` (reject) and `null` (silent cancel) straight through, so neither falsy result wrongly runs the post-commit editTheme side effect under the new UpdateResult contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
revertToData() now also resets enumType (to matchEnumType of the reverted
data). A rejected/cancelled to-enum type change sets enumType synchronously
before the async commit resolves, so reverting only value/dataType left the
type selector stuck on the enum (with a possibly-invalid option) on re-open.
Regression test drives a rejected to-enum change and asserts the selector
reverts.
Demo's onUpdate wrapper: `true` (a valid commit per UpdateResult) was treated
as a truthy "special" result and skipped the post-commit editTheme side
effect. It now falls through to the commit branch like void/undefined; only
object results (error / { value } override) short-circuit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
UpdateFunctionProps' `rename` event declared `newKey: CollectionKey` (string | number), but renaming only applies to object keys — the whole internal pipeline (InternalRenameFunction, handleEditKey) is string-only, and numeric array indices can't be renamed. Narrow to `string` so the public type doesn't imply numeric keys are possible. Output-type tightening; no breakage for consumers reading `props.newKey`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The shipped JsonEditorHandle is collapse + startEdit + confirm + cancel; startRename/startAdd were deferred to 2.x in the Tier 1 trim. Three doc spots still described them as shipped/delivered: - migration-guide quick-overview: drop startRename/startAdd from the editorRef method list (consumer-facing — must match the handle). - V2-roadmap §10 follow-up: "Delivered ... startRename/startAdd" → implemented then deferred, with the shipped shape stated inline. - V2-roadmap Category 4 blockquote: scope the startRename/startAdd handle to the Phase 4 implementation snapshot (since trimmed), not the shipped handle. The onEditEvent lifecycle event strings startRename/startAdd are unchanged — those still fire for UI-driven sessions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `add` event payload spread buildNodeData(newData, …) (post-add tree) and overrode only value/fullData, leaving parentData + size describing the post-add state while value (undefined) and fullData (pre-add) described another. Now parentData/size are pre-add too (parent without the new node; size null — no committed value yet), keeping only key/path/level/index from the post-add tree (the slot only exists there). Payload is internally consistent: pre-add state + insertion position. Regression test asserts the add payload's parentData/fullData are the pre-add document (not including the new key) with value undefined + size null. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- ValueNodeWrapper: custom-node type-change branch now mirrors the enum branch's three-way onEdit result split, so result===false reports as cancelEdit (was confirmEdit) and a non-empty error string fires onError (was silently swallowed). - EditingProvider.startEdit: fires cancel*/cancelRename for the displaced session on node-switch, skipping the fire only when the cancelOp itself routed through cancelEdit. - EditingProvider.cancelEdit: now invokes the registered cancelOp (with a re-entrancy guard) so external cancels — search-text change, editorRef.cancelEdit — actually reset per-node UI buffers (e.g. CollectionNode's stringifiedValue), not just clear store state. - ValueNodeWrapper: extract revertSession from handleCancel and register it as cancelOp on every value-edit entry point; handleCancel still runs the revert directly to cover entry paths (Tab-arrival, filter redirect) that don't register a cancelOp. - Tests: regression cases for node-switch cancelEdit emission and for editorRef.cancelEdit clearing the local edit buffer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Brings phase-2's review fixes (event-specific reject messages, enum-revert, rename newKey:string, coherent add-event NodeData) and phase-3's #304 observer-lifecycle work into phase-4. Conflict resolution — src/contexts/EditingProvider.tsx `cancelEdit`: took phase-3's robust body (the `cancelling` reentrancy guard managed in try/finally + running the outgoing session's `cancelOp` UI cleanup + the commit guard) combined with phase-4's `eventForMode(prev.mode, 'cancel')` helper. eventForMode is the better choice — it also maps `add` mode to `cancelAdd`, which phase-3's inline `key ? cancelRename : cancelEdit` missed. Test reconciliation — test/JsonEditor.test.tsx: phase-3 added handle tests using the pre-rename method names (`cancelEdit`/`confirmEdit`); phase-4 renamed the handle methods to `cancel`/`confirm` (the lifecycle event names are unchanged). Updated the calls + the JsonViewer absence-assertions to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…esult doc
editorRef.confirm() called cancelEdit() unconditionally, even when there was
no live value-edit confirm control to click. On an unrelated open session (e.g.
a UI-driven key rename) that trailing cancel tore the session down — confirm()
silently cancelled it. Now confirm() is a no-op when editConfirmRef.current is
null. Regression test: confirm() during a key rename no longer fires
cancelRename and leaves the rename input open (fails without the guard).
StartEditResult JSDoc: "What editorRef.startEdit did" → present tense ("The
result of editorRef.startEdit") — it documents the return type, not a past act.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ion path onMove's `move` payload set `newPath: destPath` (the drop-target node's path), but the moved node's actual landing spot is `[...targetPath, targetKey]` — the path it's inserted at in `addedData`. They diverge whenever `targetKey` differs from the drop target's key: the array above/below offset (+1, then -1 for same-array forward moves), the kept object key on object→object moves, or the generated `arr_N` key on array→object moves. `newPath` now reports the true destination. (No regression test — the `move`/DnD path is the deferred #270 test gap, where the `event:"move"` case is already `test.todo`. Fix verified by reasoning through the array/object move cases.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ty-error)
Two related fixes from the review:
Theme A — handleEdit's reject paths (generic `false` and `{ error }`) no longer
call setData(input.fullData). setData only ever runs AFTER onUpdate resolves
(no optimistic commit), so a reject has nothing to undo: the write was a no-op
in the sync case and, with a slow async onUpdate, could clobber a newer commit
that landed while the rejection was in flight. The initiating node already
reverts its own display buffer on reject.
Theme B — result handlers treated only truthy strings as errors, so an
empty-string error (`{ error: '' }`) was silently read as success (no revert,
no onError). Branch on `typeof === 'string'` instead (InternalResult is
`string | void | false`). Applied at all sites: ValueNodeWrapper (3 type/edit
+ delete), useCommon (rename), and — same latent bug, not in the flagged diff
but fixed for consistency — CollectionNode (json-edit + add×2 + delete).
Tests: false-reject (sync + async) now assert setData is NOT called and the
display still reverts; new `{ error: '' }` case asserts revert + onError fires
with the empty message. All three fail without the fixes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Brings phase-2's reject-hygiene fix (f94117d) up the stack. Conflict resolution — the Theme B `typeof === 'string'` error checks (phase-2) landed in the same handlers phase-3's #304 work rewrote to emit lifecycle events. Resolved by keeping phase-3's richer branches (cancelEdit/confirmEdit/ confirmAdd/delete lifecycle events) AND applying the empty-string-error fix on top — so e.g. a rename/edit/add/delete rejected with `{ error: '' }` now surfaces onError and reports the session as cancelled, instead of being read as a confirm. Files: CollectionNode (json-edit + add×2 + delete), ValueNodeWrapper (enum + type-change + handleEdit + delete), useCommon (rename). The reject-no-setData change merged cleanly into handleEdit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Completes the cascade: phase-4 now integrates all of phase-2 + phase-3, including the reject-hygiene + empty-string-error (Theme A/B) fixes. Conflict resolution — same handlers, where phase-4 had refactored to the early-return style (handleEdit/onRename) and a shared `onAddResult` helper + parse-in-try/commit-after for JSON edits, while phase-3 carried the Theme B `typeof === 'string'` fix. Kept phase-4's (cleaner) structure and folded the empty-string-error fix into it across CollectionNode (json-edit commit + onAddResult), ValueNodeWrapper (handleEdit), and useCommon (rename). The enum/type-change/delete handlers auto-merged to phase-3's typeof version (phase-4 hadn't touched those lines since the base). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Post-consolidation integration review surfaced two result-handlers still using
the truthy `if (result)` check instead of `typeof result === 'string'`. Both
sit in lifecycle/drag code that didn't exist on phase-2 (where the Theme B fix
landed), so the cascade never touched them:
- ValueNodeWrapper handleChangeDataType, custom-node branch: an empty-string
rejection (`{ error: '' }`) fell through to emitEditEvent('confirmEdit'),
skipping revertToData() + onError — leaving the UI on the rejected custom
type and firing a bogus confirm. (User-visible.)
- useDragNDrop move drop handler: an empty-string move rejection was swallowed
(no onError). Data stays correct (handleEdit no longer setData's on reject),
but the error notification was lost.
All result/error handlers across src/ now consistently branch on
`typeof === 'string'`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-key handleAdd's object path opened a startAdd session (key entry) but, on a commit-time rejection, error, or KEY_EXISTS, fired no terminal event — leaving an orphaned startAdd, unlike value-edit/rename which fire cancel* on the equivalent paths. Now the object-add path emits cancelAdd on all three non-commit outcomes (and confirmAdd on commit), mirroring the value-edit confirm flow. Array adds are unchanged — they're instant (no startAdd session), so only confirmAdd fires on success. Also extends the emitEditEvent Extract type to include 'cancelAdd' (it was structurally unreachable before). Regression test: a rejected object add now yields the [startAdd, cancelAdd] sequence. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eanup Cross-PR review audit (against the integrated phase-4): Stale-fullData in observer payloads (PR #304 ×5): emitEditEvent (both nodes), the click onCollapse, and the useCommon rename events all spread `nodeData`, whose `fullData` a memo-bailed node keeps stale (areNodePropsEqual ignores its identity). Now they override `fullData: getLatestData()` — matching what onError/onChange already do (the PERF-ARCHITECTURE "read live in event handlers, never from a memoizable prop" rule). Regression test: a confirmEdit fired from a bailed subtree now carries the live document, not the snapshot. onChangeArgsRef (PR #303): dropped the unused `name`/`path` fields — only `value` needs the ref-to-latest; name/path come from nodeDataRef. BuildNodeDataFromPathRef (PR #304): kept as React.RefObject — the comment suggested MutableRefObject, but under the pinned @types/react 19 RefObject.current is mutable (the write compiles) and MutableRefObject is deprecated, so RefObject is the correct, non-deprecated type. Added a note. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
§17 API standardisation — onUpdate + observers + editorRef (consolidated, #289)
5 tasks
Bundle size impact
|
| Format | Base raw | PR raw | Δ raw | Base gzip | PR gzip | Δ gzip |
|---|---|---|---|---|---|---|
| esm | 52.16 KB | 54.73 KB | 🔺 +2.58 KB (+4.94%) | 18.18 KB | 18.87 KB | 🔺 +701 B (+3.76%) |
| cjs | 53.58 KB | 56.19 KB | 🔺 +2.61 KB (+4.87%) | 18.23 KB | 18.91 KB | 🔺 +705 B (+3.78%) |
Measured from build/index.{cjs,esm}.js. Gzip at level 9.
This was referenced Jun 3, 2026
8 tasks
This was referenced Jun 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #294
Consolidated branch of Phase 0-4 of that stack.