Skip to content

Commit

Permalink
feat: do not trigger onChange on programmatic changes (#410)
Browse files Browse the repository at this point in the history 
BREAKING CHANGE:

The `onChange` callback is no longer triggered on programmatic changes via a
two-way binded `content` or via methods `.update()`, `.set()`, and `.patch()`.
  • Loading branch information
@josdejong
josdejong authored Mar 1, 2024
1 parent 4e188fe commit 201f602
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 76 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ Callback fired when an error occurs. Default implementation is to log an error i
onChange(content: Content, previousContent: Content, changeStatus: { contentErrors: ContentErrors | null, patchResult: JSONPatchResult | null })
```

The callback which is invoked on every change of the contents, both changes made by a user and programmatic changes made via methods like `.set()`, `.update()`, or `.patch()`.
The callback which is invoked on every change of the contents made by the user from within the editor. It will not trigger on changes that are applied programmatically via methods like `.set()`, `.update()`, or `.patch()`.

The returned `content` is sometimes of type `{ json }`, and sometimes of type `{ text }`. Which of the two is returned depends on the mode of the editor, the change that is applied, and the state of the document (valid, invalid, empty). Please be aware that `{ text }` can contain invalid JSON: whilst typing in `text` mode, a JSON document will be temporarily invalid, like when the user is typing a new string. The parameter `patchResult` is only returned on changes that can be represented as a JSON Patch document, and for example not when freely typing in `text` mode.
Expand Down
28 changes: 7 additions & 21 deletions src/lib/components/modes/tablemode/TableMode.svelte
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,6 @@
return
}
const previousContent = { json, text }
const previousJson = json
const previousState = documentState
const previousText = text
Expand Down Expand Up @@ -419,13 +418,6 @@
previousText,
previousTextIsRepaired
})
// we could work out a patchResult, or use patch(), but only when the previous and new
// contents are both json and not text. We go for simplicity and consistency here and
// let the function applyExternalContent _not_ return a patchResult ever.
const patchResult = null
emitOnChange(previousContent, patchResult)
}
function applyExternalSelection(externalSelection: JSONEditorSelection | null) {
Expand Down Expand Up @@ -580,7 +572,6 @@
throw new Error('Cannot apply patch: no JSON')
}
const previousContent: Content = { json }
const previousJson = json
const previousState = documentState
const previousTextIsRepaired = textIsRepaired
Expand Down Expand Up @@ -638,26 +629,21 @@
redo: operations
}
emitOnChange(previousContent, patchResult)
return patchResult
}
function handlePatch(
operations: JSONPatchDocument,
afterPatch?: AfterPatchCallback
): JSONPatchResult {
if (readOnly) {
// this should never happen in practice
return {
json,
previousJson: json,
redo: [],
undo: []
}
}
debug('handlePatch', operations, afterPatch)
return patch(operations, afterPatch)
const previousContent = { json, text }
const patchResult = patch(operations, afterPatch)
emitOnChange(previousContent, patchResult)
return patchResult
}
function emitOnChange(previousContent: Content, patchResult: JSONPatchResult | null) {
Expand Down
56 changes: 34 additions & 22 deletions src/lib/components/modes/textmode/TextMode.svelte
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@
escapeUnicodeCharacters
})
$: setCodeMirrorContent(externalContent)
$: setCodeMirrorContent(externalContent, false, false)
$: applyExternalSelection(externalSelection)
$: updateLinter(validator)
$: updateIndentation(indentation)
Expand Down Expand Up @@ -250,14 +250,20 @@
})
export function patch(operations: JSONPatchDocument): JSONPatchResult {
debug('patch', operations)
return handlePatch(operations, false)
}
export function handlePatch(operations: JSONPatchDocument, emitChange: boolean): JSONPatchResult {
debug('handlePatch', operations, emitChange)
const previousJson = parser.parse(text)
const updatedJson = immutableJSONPatch(previousJson, operations)
const undo = revertJSONPatch(previousJson, operations)
setCodeMirrorContent({
const updatedContent = {
text: parser.stringify(updatedJson, null, indentation) as string
})
}
setCodeMirrorContent(updatedContent, emitChange, false)
return {
json: updatedJson,
Expand All @@ -275,11 +281,12 @@
}
try {
const json = parser.parse(text)
setCodeMirrorContent({
text: parser.stringify(json, null, indentation) as string
})
askToFormat = true
const updatedJson = parser.parse(text)
const updatedContent = {
text: parser.stringify(updatedJson, null, indentation) as string
}
setCodeMirrorContent(updatedContent, true, false)
return true
} catch (err) {
Expand All @@ -297,11 +304,12 @@
}
try {
const json = parser.parse(text)
setCodeMirrorContent({
text: parser.stringify(json) as string
})
askToFormat = false
const updatedJson = parser.parse(text)
const updatedContent = {
text: parser.stringify(updatedJson) as string
}
setCodeMirrorContent(updatedContent, true, false)
return true
} catch (err) {
Expand All @@ -319,9 +327,12 @@
}
try {
setCodeMirrorContent({
const updatedContent = {
text: jsonrepair(text)
})
}
setCodeMirrorContent(updatedContent, true, false)
jsonStatus = JSON_STATUS_VALID
jsonParseError = null
} catch (err) {
Expand All @@ -345,7 +356,7 @@
rootPath: [],
onSort: async ({ operations }) => {
debug('onSort', operations)
patch(operations)
handlePatch(operations, true)
},
onClose: () => {
modalOpen = false
Expand Down Expand Up @@ -384,7 +395,7 @@
})
} else {
debug('onTransform', operations)
patch(operations)
handlePatch(operations, true)
}
},
onClose: () => {
Expand Down Expand Up @@ -445,7 +456,7 @@
function handleAcceptTooLarge() {
acceptTooLarge = true
setCodeMirrorContent(externalContent, true)
setCodeMirrorContent(externalContent, true, true)
}
function handleSwitchToTreeMode() {
Expand Down Expand Up @@ -671,12 +682,12 @@
}
}
function setCodeMirrorContent(newContent: Content, forceUpdate = false) {
function setCodeMirrorContent(newContent: Content, emitChange: boolean, forceUpdate: boolean) {
const newText = getText(newContent, indentation, parser)
const isChanged = !isEqual(newContent, content)
const previousContent = content
debug('setCodeMirrorContent', { isChanged, forceUpdate })
debug('setCodeMirrorContent', { isChanged, emitChange, forceUpdate })
if (!codeMirrorView || (!isChanged && !forceUpdate)) {
return
Expand All @@ -698,7 +709,8 @@
}
updateCanUndoRedo()
if (isChanged) {
if (isChanged && emitChange) {
emitOnChange(content, previousContent)
}
}
Expand Down
38 changes: 6 additions & 32 deletions src/lib/components/modes/treemode/TreeMode.svelte
View file
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,6 @@
return
}
const previousContent: Content = { json, text }
const previousState = documentState
const previousJson = json
const previousText = text
Expand All @@ -533,14 +532,6 @@
previousText,
previousTextIsRepaired
})
// we could work out a patchResult, or use patch(), but only when the previous and new
// contents are both json and not text. We go for simplicity and consistency here and
// let the functions applyExternalJson and applyExternalText _not_ return
// a patchResult ever.
const patchResult = null
emitOnChange(previousContent, patchResult)
}
function applyExternalText(updatedText: string | undefined) {
Expand All @@ -557,7 +548,6 @@
return
}
const previousContent: Content = { json, text }
const previousJson = json
const previousState = documentState
const previousText = text
Expand Down Expand Up @@ -597,14 +587,6 @@
previousText,
previousTextIsRepaired
})
// we could work out a patchResult, or use patch(), but only when the previous and new
// contents are both json and not text. We go for simplicity and consistency here and
// let the functions applyExternalJson and applyExternalText _not_ return
// a patchResult ever.
const patchResult = null
emitOnChange(previousContent, patchResult)
}
function applyExternalSelection(externalSelection: JSONEditorSelection | null) {
Expand Down Expand Up @@ -739,7 +721,6 @@
throw new Error('Cannot apply patch: no JSON')
}
const previousContent = { json, text }
const previousJson = json
const previousState = documentState
const previousText = text
Expand Down Expand Up @@ -800,8 +781,6 @@
redo: operations
}
emitOnChange(previousContent, patchResult)
return patchResult
}
Expand Down Expand Up @@ -1451,19 +1430,14 @@
operations: JSONPatchDocument,
afterPatch?: AfterPatchCallback
): JSONPatchResult {
if (readOnly) {
// this should never happen in practice
return {
json,
previousJson: json,
undo: [],
redo: []
}
}
debug('handlePatch', operations, afterPatch)
return patch(operations, afterPatch)
const previousContent = { json, text }
const patchResult = patch(operations, afterPatch)
emitOnChange(previousContent, patchResult)
return patchResult
}
function handleReplaceJson(updatedJson: unknown, afterPatch?: AfterPatchCallback) {
Expand Down

0 comments on commit 201f602

Please sign in to comment.