Skip to content

Decouple edit-session UI close from the data-commit/cleanup phase#328

Closed
Copilot wants to merge 2 commits into
v2.0-devfrom
copilot/decouple-edit-session-ui
Closed

Decouple edit-session UI close from the data-commit/cleanup phase#328
Copilot wants to merge 2 commits into
v2.0-devfrom
copilot/decouple-edit-session-ui

Conversation

Copilot AI commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

JsonEditor awaits onUpdate before committing, but closes the editor synchronously before the await — so during a deferred commit (confirm-modal hook, slow remote round-trip) value nodes flash the new typed value as if applied, and collection nodes re-derive children from the old data. The naive fix (move closeEdit into .then) breaks Tab-commit by running a stale cancelOp against the just-committed value, double-fires terminal events, and closes the wrong session.

Two-phase commit in the editing store

src/contexts/EditingProvider.tsx — splits closeEdit's coupled responsibilities:

  • beginCommit() — clears cancelOp synchronously (keeps Tab-commit safe) and registers the active path in a committingPaths set. Leaves currentlyEditingElement in place, so isEditing stays true and the editor stays visible.
  • endCommit(path) — deregisters and clears currentlyEditingElement only if it still points at path. A Tab-driven startEdit(next) during the pending window leaves the new session untouched.
  • startEdit's displacement logic skips its cancelEdit/cancelRename emit for any session in committingPaths — the node owns the terminal event from its own commit outcome, no double-fire.

Node commit handlers

  • src/ValueNodeWrapper.tsxhandleEdit uses beginCommit() synchronously and endCommit(path) in the resolve callback. The local value buffer naturally retains the user's typed value through the pending window.
  • src/CollectionNode.tsx — same shape, plus the JSON buffer (stringifiedValue) is now cleared in the resolve callback rather than synchronously, so the open textarea keeps showing the typed JSON instead of re-deriving from old data.
const handleEdit = (inputValue?: unknown) => {
  beginCommit()              // sync: clear cancelOp, mark path mid-commit
  setPreviousValue(null)
  // ...derive newValue...
  onEdit(newValue, path).then((result) => {
    handleMutationResult({ result, /* confirmEdit | revert + cancelEdit */ })
    endCommit(path)          // close session iff still ours
  })
}

Secondary edit paths (type-change, rename, add) keep the existing synchronous closeEdit() — they don't surface the optimistic-display window in practice and the buffer mechanics differ. Worth a follow-up if uniformity becomes desirable.

Tests

test/JsonEditor.test.tsx adds three regressions against a Promise resolved on demand:

  • value edit stays open while pending; resolve fires confirmEdit and closes.
  • value edit reject reverts the display and closes.
  • collection JSON buffer survives the pending window.

Existing Tab-commit / displaced-session / editorRef.cancel guards still pass — the three documented failure modes (Tab-commit revert, double terminal events, wrong-session close) are covered.

Copilot AI changed the title [WIP] Decouple edit-session UI close from data commit and cleanup Decouple edit-session UI close from the data-commit/cleanup phase Jun 5, 2026
Copilot AI requested a review from CarlosNZ June 5, 2026 13:55
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

Bundle size impact

json-edit-react

Format Base raw PR raw Δ raw Base gzip PR gzip Δ gzip
esm 54.18 KB 54.50 KB 🔺 +330 B (+0.59%) 18.94 KB 19.03 KB 🔺 +95 B (+0.49%)
cjs 55.66 KB 55.99 KB 🔺 +335 B (+0.59%) 19.01 KB 19.11 KB 🔺 +104 B (+0.53%)

Measured from build/index.{cjs,esm}.js. Gzip at level 9.

@CarlosNZ CarlosNZ marked this pull request as ready for review June 5, 2026 22:21
@CarlosNZ CarlosNZ marked this pull request as draft June 7, 2026 11:47
@CarlosNZ

CarlosNZ commented Jun 7, 2026

Copy link
Copy Markdown
Owner

Replaced by #330

@CarlosNZ CarlosNZ closed this Jun 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Decouple edit-session UI close from the data-commit/cleanup phase

2 participants