Review of the public props surface (types.ts + the README props tables) as of fc23b40 on v2.0-dev, looking for naming inconsistencies and other confusions. Since v2 is still in dev, all of these are actionable now. Not all points are necessarily endorsed — this is a list for consideration; refer to items by number when working them.
Naming inconsistencies in JsonEditorProps
1. allowClipboard is the odd one out in the allow* family. All five siblings — allowEdit, allowDelete, allowAdd, allowDrag, allowTypeSelection — accept boolean | FilterFunction, but allowClipboard is plain boolean (types.ts:16). Anyone pattern-matching the family will expect per-node control and be surprised. Either support a FilterFunction (arguably useful — hide copy on sensitive nodes) or rename it out of the family.
2. onChange — keep the name; the only real concern is the must-return contract. onChange is the one on* prop whose return value the editor consumes: a per-keystroke transform ((props) => ValueData, types.ts:275), not an observer like the rest of the family. An earlier framing flagged this as a naming trap — the worry that React devs would reach for onChange expecting commit semantics — but on reflection that overstated it: in React, onChange on a controlled input fires per keystroke, which is exactly when this fires, so the name sets the right expectation for when. Semantically it's the controlled-input idiom with an inverted channel — the editor owns the draft state, so "deciding what the state becomes" arrives via the return value rather than setState. Same role, inverted channel. What survives is much smaller: the must-return contract. ValueNodeWrapper.tsx:112-118 does setValue(onChange({...})) unconditionally, so a callback that forgets to return (or has a branch that doesn't) silently sets the draft to undefined. But OnChangeFunction is typed to return ValueData and inline callbacks get contextual typing, so TypeScript users get a compile error for a missing return — only plain-JS consumers can hit it. Revised position: keep the name onChange. If anything is done here, a one-line defensive tweak beats a rename: treat an undefined return as "accept the keystroke unchanged" (modifiedValue === undefined ? newValue : modifiedValue), converting the remaining footgun into middleware-style "return nothing = no transform" semantics. Counterargument: that masks a buggy callback instead of surfacing it, so doing nothing is also defensible.
3. "collapsed" vs "closed" — two words for one state. collapse, onCollapse, CollapseState.collapsed, collapseAnimationTime, collapseClickZones… and then showCollectionCount: 'when-closed' (types.ts:24). 'when-collapsed' would unify the vocabulary.
4. collapse (prop) vs editorRef.collapse() (command) — same word, opposite natures. The prop is declarative initial-display config; the handle method is an imperative command taking a completely different shape (CollapseState). This collision is new in v2 now that the imperative handle exists. Renaming the prop (defaultCollapse / collapseLevel-ish) would also fix the long-standing quirk that a verb-named prop reads like a command.
5. TextEditor vs CustomSelect — two component-replacement props, two conventions. One is bare (types.ts:53), one is Custom-prefixed (types.ts:54) — and internally the latter is threaded as plain Select. Pick one scheme. The Custom prefix is also awkward because the lowercase custom* family (customNodeDefinitions, customText, customButtons) are config objects, not components.
6. allowTypeSelection is the only non-verb in the allow* family (edit/delete/add/drag are all verbs). Minor, but allowTypeChange or similar would read more consistently.
Event-name / contract confusions
7. updateSuccessful / updateError aren't grammatically parallel (adjective vs noun, types.ts:348-349). updateSuccess/updateError or updateSucceeded/updateFailed.
8. event: 'delete' and event: 'move' appear in both discriminated unions with different powers. In UpdateFunctionProps they're vetoable; in EditEvent they're observations. Same key (event), same literal values, different contracts — easy to conflate when both callbacks are wired. Also, within EditEvent every other member follows verbNoun (startEdit, commitAdd…) while these two are bare verbs. Naming them commitDelete/commitMove in the event stream would both disambiguate and signal "instant ops jump straight to commit."
9. onError exists at two levels with different signatures. JsonEditorProps.onError takes a flat props object (types.ts:282); CustomComponentProps.onError is positional (error, errorValue) (types.ts:542). Both exported, same name, different call shape.
Custom-component surface
10. Genuine type bug: CustomNodeDefinition's U generic isn't applied to wrapperProps. The definition declares wrapperComponent?: React.FC<CustomWrapperProps<U>> but wrapperProps?: Record<string, unknown> (types.ts:578) — so the generic that exists precisely to enforce component↔props consistency enforces nothing. componentProps?: T does it correctly three lines up. Should be wrapperProps?: U.
11. setIsEditingKey: () => void is named like a state setter but takes no argument (types.ts:510) — it's really "enter key-edit mode". Meanwhile CustomComponentProps.setIsEditing is a real Dispatch<SetStateAction<boolean>>. Same name family, different shapes. startEditingKey or similar would be honest.
12. Three names for keyboard plumbing across exported types: TextEditorProps.onKeyDown, CustomComponentProps.handleKeyPress, BaseNodeProps.handleKeyboard (plus keyboardCommon). All attach to keydown, and "keyPress" is the deprecated React event name. A component author consuming two of these types at once hits the mixture immediately.
13. The word data means two different things at two levels, and a custom component gets the node value three ways. Top-level data = whole document; BaseNodeProps.data = this node's value; plus CustomComponentProps.value and nodeData.value are the same thing again. Hard to fix the inherited data now, but the data/value duplication inside CustomComponentProps deserves at least a doc note saying which one to use.
14. EnumDefinition.enum holds the enum's display name (types.ts:149) — a field literally named enum containing a label string, next to values which holds the actual options. name would say what it is, and would also match CustomNodeDefinition.name (which serves the identical "label in type selector" role). Related: Theme.displayName vs CustomNodeDefinition.name — two label fields, two conventions.
README ↔ types drift (found along the way)
- (a) The
defaultValue row says type DefaultValueFilterFunction (README:207) — that type doesn't exist; it's DefaultValueFunction (correct in the type list at README:1421).
- (b) The
customNodeDefinitions row still says the link component "is provided in the main package" (README:262) — it moved to @json-edit-react/components in v2.
- (c) The
onEditEvent row says "whenever the user starts or stops editing a node" (README:283) — stale next to the full v2 lifecycle description at README:1268 (submit/commit/settlement, delete/move).
- (d) The
insertAtTop type cell has a typo: boolean\| "object \| "array" — missing closing quote (README:237).
- (e) The
allowTypeSelection type column says DataType[] but the real TypeOptions also admits custom-type strings and EnumDefinition — the description mentions enums, so the type column contradicts its own row.
- (f)
arrayIndexStart documented as 0 | 1 but typed number — known and deliberate; listed only for completeness.
Checked and found consistent
For the record: the show* boolean family, the *Time props (all ms, all suffixed), the new* delta fields in UpdateFunctionProps, and the search* family.
If forced to pick the highest-value moves: allowClipboard filter support or rename (1) and the wrapperProps generic fix (10, which is a bug rather than a taste call) — point 2's onChange rename is withdrawn (keep the name).
Review of the public props surface (types.ts + the README props tables) as of fc23b40 on
v2.0-dev, looking for naming inconsistencies and other confusions. Since v2 is still in dev, all of these are actionable now. Not all points are necessarily endorsed — this is a list for consideration; refer to items by number when working them.Naming inconsistencies in
JsonEditorProps1.
allowClipboardis the odd one out in theallow*family. All five siblings —allowEdit,allowDelete,allowAdd,allowDrag,allowTypeSelection— acceptboolean | FilterFunction, butallowClipboardis plainboolean(types.ts:16). Anyone pattern-matching the family will expect per-node control and be surprised. Either support aFilterFunction(arguably useful — hide copy on sensitive nodes) or rename it out of the family.2.
onChange— keep the name; the only real concern is the must-return contract.onChangeis the oneon*prop whose return value the editor consumes: a per-keystroke transform ((props) => ValueData, types.ts:275), not an observer like the rest of the family. An earlier framing flagged this as a naming trap — the worry that React devs would reach foronChangeexpecting commit semantics — but on reflection that overstated it: in React,onChangeon a controlled input fires per keystroke, which is exactly when this fires, so the name sets the right expectation for when. Semantically it's the controlled-input idiom with an inverted channel — the editor owns the draft state, so "deciding what the state becomes" arrives via the return value rather thansetState. Same role, inverted channel. What survives is much smaller: the must-return contract. ValueNodeWrapper.tsx:112-118 doessetValue(onChange({...}))unconditionally, so a callback that forgets to return (or has a branch that doesn't) silently sets the draft toundefined. ButOnChangeFunctionis typed to returnValueDataand inline callbacks get contextual typing, so TypeScript users get a compile error for a missing return — only plain-JS consumers can hit it. Revised position: keep the nameonChange. If anything is done here, a one-line defensive tweak beats a rename: treat anundefinedreturn as "accept the keystroke unchanged" (modifiedValue === undefined ? newValue : modifiedValue), converting the remaining footgun into middleware-style "return nothing = no transform" semantics. Counterargument: that masks a buggy callback instead of surfacing it, so doing nothing is also defensible.3. "collapsed" vs "closed" — two words for one state.
collapse,onCollapse,CollapseState.collapsed,collapseAnimationTime,collapseClickZones… and thenshowCollectionCount: 'when-closed'(types.ts:24).'when-collapsed'would unify the vocabulary.4.
collapse(prop) vseditorRef.collapse()(command) — same word, opposite natures. The prop is declarative initial-display config; the handle method is an imperative command taking a completely different shape (CollapseState). This collision is new in v2 now that the imperative handle exists. Renaming the prop (defaultCollapse/collapseLevel-ish) would also fix the long-standing quirk that a verb-named prop reads like a command.5.
TextEditorvsCustomSelect— two component-replacement props, two conventions. One is bare (types.ts:53), one isCustom-prefixed (types.ts:54) — and internally the latter is threaded as plainSelect. Pick one scheme. TheCustomprefix is also awkward because the lowercasecustom*family (customNodeDefinitions,customText,customButtons) are config objects, not components.6.
allowTypeSelectionis the only non-verb in theallow*family (edit/delete/add/drag are all verbs). Minor, butallowTypeChangeor similar would read more consistently.Event-name / contract confusions
7.
updateSuccessful/updateErroraren't grammatically parallel (adjective vs noun, types.ts:348-349).updateSuccess/updateErrororupdateSucceeded/updateFailed.8.
event: 'delete'andevent: 'move'appear in both discriminated unions with different powers. InUpdateFunctionPropsthey're vetoable; inEditEventthey're observations. Same key (event), same literal values, different contracts — easy to conflate when both callbacks are wired. Also, withinEditEventevery other member followsverbNoun(startEdit,commitAdd…) while these two are bare verbs. Naming themcommitDelete/commitMovein the event stream would both disambiguate and signal "instant ops jump straight to commit."9.
onErrorexists at two levels with different signatures.JsonEditorProps.onErrortakes a flat props object (types.ts:282);CustomComponentProps.onErroris positional(error, errorValue)(types.ts:542). Both exported, same name, different call shape.Custom-component surface
10. Genuine type bug:
CustomNodeDefinition'sUgeneric isn't applied towrapperProps. The definition declareswrapperComponent?: React.FC<CustomWrapperProps<U>>butwrapperProps?: Record<string, unknown>(types.ts:578) — so the generic that exists precisely to enforce component↔props consistency enforces nothing.componentProps?: Tdoes it correctly three lines up. Should bewrapperProps?: U.11.
setIsEditingKey: () => voidis named like a state setter but takes no argument (types.ts:510) — it's really "enter key-edit mode". MeanwhileCustomComponentProps.setIsEditingis a realDispatch<SetStateAction<boolean>>. Same name family, different shapes.startEditingKeyor similar would be honest.12. Three names for keyboard plumbing across exported types:
TextEditorProps.onKeyDown,CustomComponentProps.handleKeyPress,BaseNodeProps.handleKeyboard(pluskeyboardCommon). All attach to keydown, and "keyPress" is the deprecated React event name. A component author consuming two of these types at once hits the mixture immediately.13. The word
datameans two different things at two levels, and a custom component gets the node value three ways. Top-leveldata= whole document;BaseNodeProps.data= this node's value; plusCustomComponentProps.valueandnodeData.valueare the same thing again. Hard to fix the inheriteddatanow, but thedata/valueduplication insideCustomComponentPropsdeserves at least a doc note saying which one to use.14.
EnumDefinition.enumholds the enum's display name (types.ts:149) — a field literally namedenumcontaining a label string, next tovalueswhich holds the actual options.namewould say what it is, and would also matchCustomNodeDefinition.name(which serves the identical "label in type selector" role). Related:Theme.displayNamevsCustomNodeDefinition.name— two label fields, two conventions.README ↔ types drift (found along the way)
defaultValuerow says typeDefaultValueFilterFunction(README:207) — that type doesn't exist; it'sDefaultValueFunction(correct in the type list at README:1421).customNodeDefinitionsrow still says the link component "is provided in the main package" (README:262) — it moved to@json-edit-react/componentsin v2.onEditEventrow says "whenever the user starts or stops editing a node" (README:283) — stale next to the full v2 lifecycle description at README:1268 (submit/commit/settlement, delete/move).insertAtToptype cell has a typo:boolean\| "object \| "array"— missing closing quote (README:237).allowTypeSelectiontype column saysDataType[]but the realTypeOptionsalso admits custom-type strings andEnumDefinition— the description mentions enums, so the type column contradicts its own row.arrayIndexStartdocumented as0 | 1but typednumber— known and deliberate; listed only for completeness.Checked and found consistent
For the record: the
show*boolean family, the*Timeprops (all ms, all suffixed), thenew*delta fields inUpdateFunctionProps, and thesearch*family.If forced to pick the highest-value moves:
allowClipboardfilter support or rename (1) and thewrapperPropsgeneric fix (10, which is a bug rather than a taste call) — point 2'sonChangerename is withdrawn (keep the name).