Skip to content

Lift duplicated patterns from CollectionNode/ValueNodeWrapper into useCommon#312

Merged
CarlosNZ merged 2 commits into
v2.0-devfrom
copilot/refactor-reduce-duplication-again
Jun 3, 2026
Merged

Lift duplicated patterns from CollectionNode/ValueNodeWrapper into useCommon#312
CarlosNZ merged 2 commits into
v2.0-devfrom
copilot/refactor-reduce-duplication-again

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 3, 2026

CollectionNode.tsx and ValueNodeWrapper.tsx had several copy-pasted patterns (emitEditEvent, the three-branch onEdit/onAdd/onDelete result dance, keyDisplayProps, the previousValue revert), creating drift risk and bundle weight. Consolidated into useCommon.

Changes

  • useCommon — destructure onEdit, sort, arrayIndexFromOne, handleKeyboard, customNodeData from props, plus setPreviousValue/getSnapshot from the editing store. Expose:

    • emitEditEvent(event, extra?) — replaces the two inlined closures and the four hand-written shapes inside handleEditKey.
    • handleMutationResult({ result, errorCode, errorValue, cancelEvent?, confirmEvent?, confirmExtra?, onRevert?, onErrorExtra?, onConfirmExtra? }) — collapses ~8 .then(result => …) branches. Optional callbacks preserve per-site nuances: revertToData on the value-node paths, the extra closeEdit() on the enum/standard error branch in handleChangeDataType, the success-only setEnumType(null).
    • getNextOrPreviousAtPath(type) — stable wrapper used by KeyDisplay, tabForward/tabBack, and the Tab-redirect useLayoutEffect.
    • revertPreviousValue() — returns true when a type-change snapshot was applied, so ValueNodeWrapper.revertSession can fall back to revertToData() and CollectionNode.handleCancel can simply call it.
    • buildKeyDisplayProps({ handleCancel, getStyles, keyValueArray?, handleClick? }) — single source for the ~13 shared fields; collection adds keyValueArray + handleClick.
  • CollectionNode.tsxhandleEdit / handleAdd (object + array) / handleDelete now call handleMutationResult; handleCancel calls revertPreviousValue; keyDisplayProps calls buildKeyDisplayProps. Drops unused imports (EditEvent, getNextOrPrevious) and prop destructures (onEditEvent, arrayIndexFromOne, getSnapshot).

  • ValueNodeWrapper.tsx — both type-change branches plus handleEdit / handleDelete use handleMutationResult; revertSession becomes if (!revertPreviousValue()) revertToData(); both Tab callbacks and the redirect effect use getNextOrPreviousAtPath. Drops unused imports/destructures (EditEvent, getNextOrPrevious, onEditEvent, pathString, arrayIndexFromOne, sort, etc.).

Example

Before, in both files:

onEdit(value, path).then((result) => {
  if (result === false) { emitEditEvent('cancelEdit'); return }
  if (typeof result === 'string') {
    onError({ code: 'UPDATE_ERROR', message: result }, value)
    emitEditEvent('cancelEdit'); return
  }
  emitEditEvent('confirmEdit')
})

After:

onEdit(value, path).then((result) =>
  handleMutationResult({
    result,
    errorCode: 'UPDATE_ERROR',
    errorValue: value,
    cancelEvent: 'cancelEdit',
    confirmEvent: 'confirmEdit',
  })
)

No behavioural change — all 294 existing tests pass unmodified.

Copilot AI changed the title [WIP] Refactor reduce duplication between CollectionNode and ValueNodeWrapper Lift duplicated patterns from CollectionNode/ValueNodeWrapper into useCommon Jun 3, 2026
Copilot AI requested a review from CarlosNZ June 3, 2026 13:34
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

Bundle size impact

json-edit-react

Format Base raw PR raw Δ raw Base gzip PR gzip Δ gzip
esm 54.73 KB 54.70 KB 🟢 -33 B (-0.06%) 18.87 KB 18.95 KB 🔺 +86 B (+0.45%)
cjs 56.19 KB 56.18 KB 🟢 -15 B (-0.03%) 18.91 KB 18.97 KB 🔺 +61 B (+0.31%)

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

@CarlosNZ CarlosNZ marked this pull request as ready for review June 3, 2026 20:21
Base automatically changed from feat/v2.0-api-standardisation to v2.0-dev June 3, 2026 21:15
@CarlosNZ CarlosNZ merged commit bcf12be into v2.0-dev Jun 3, 2026
2 checks passed
@CarlosNZ CarlosNZ deleted the copilot/refactor-reduce-duplication-again branch June 3, 2026 21:18
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.

Refactor: reduce duplication between CollectionNode and ValueNodeWrapper

2 participants