From 264ca58c46f6a660e190b537f63dacfc3fc8d513 Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Thu, 29 Feb 2024 19:52:20 +0100 Subject: [PATCH 1/3] feat: do not trigger onChange on programmatic changes (#318) --- .../modes/tablemode/TableMode.svelte | 27 +++------ .../components/modes/textmode/TextMode.svelte | 56 +++++++++++-------- .../components/modes/treemode/TreeMode.svelte | 36 ++---------- 3 files changed, 47 insertions(+), 72 deletions(-) diff --git a/src/lib/components/modes/tablemode/TableMode.svelte b/src/lib/components/modes/tablemode/TableMode.svelte index 1ee192a6..ca7fbe60 100644 --- a/src/lib/components/modes/tablemode/TableMode.svelte +++ b/src/lib/components/modes/tablemode/TableMode.svelte @@ -419,13 +419,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) { @@ -580,7 +573,6 @@ throw new Error('Cannot apply patch: no JSON') } - const previousContent: Content = { json } const previousJson = json const previousState = documentState const previousTextIsRepaired = textIsRepaired @@ -638,8 +630,6 @@ redo: operations } - emitOnChange(previousContent, patchResult) - return patchResult } @@ -647,17 +637,14 @@ operations: JSONPatchDocument, afterPatch?: AfterPatchCallback ): JSONPatchResult { - if (readOnly) { - // this should never happen in practice - return { - json, - previousJson: json, - redo: [], - undo: [] - } - } + debug('handlePatch', operations, afterPatch) + + const previousContent = { json, text } + const patchResult = patch(operations, afterPatch) + + emitOnChange(previousContent, patchResult) - return patch(operations, afterPatch) + return patchResult } function emitOnChange(previousContent: Content, patchResult: JSONPatchResult | null) { diff --git a/src/lib/components/modes/textmode/TextMode.svelte b/src/lib/components/modes/textmode/TextMode.svelte index 3ebad394..0938289c 100644 --- a/src/lib/components/modes/textmode/TextMode.svelte +++ b/src/lib/components/modes/textmode/TextMode.svelte @@ -173,7 +173,7 @@ escapeUnicodeCharacters }) - $: setCodeMirrorContent(externalContent) + $: setCodeMirrorContent(externalContent, false, false) $: applyExternalSelection(externalSelection) $: updateLinter(validator) $: updateIndentation(indentation) @@ -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, @@ -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) { @@ -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) { @@ -319,9 +327,12 @@ } try { - setCodeMirrorContent({ + const updatedContent = { text: jsonrepair(text) - }) + } + + setCodeMirrorContent(updatedContent, true, false) + jsonStatus = JSON_STATUS_VALID jsonParseError = null } catch (err) { @@ -345,7 +356,7 @@ rootPath: [], onSort: async ({ operations }) => { debug('onSort', operations) - patch(operations) + handlePatch(operations, true) }, onClose: () => { modalOpen = false @@ -384,7 +395,7 @@ }) } else { debug('onTransform', operations) - patch(operations) + handlePatch(operations, true) } }, onClose: () => { @@ -445,7 +456,7 @@ function handleAcceptTooLarge() { acceptTooLarge = true - setCodeMirrorContent(externalContent, true) + setCodeMirrorContent(externalContent, true, true) } function handleSwitchToTreeMode() { @@ -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 @@ -698,7 +709,8 @@ } updateCanUndoRedo() - if (isChanged) { + + if (isChanged && emitChange) { emitOnChange(content, previousContent) } } diff --git a/src/lib/components/modes/treemode/TreeMode.svelte b/src/lib/components/modes/treemode/TreeMode.svelte index ab42838d..d2314f3d 100644 --- a/src/lib/components/modes/treemode/TreeMode.svelte +++ b/src/lib/components/modes/treemode/TreeMode.svelte @@ -533,14 +533,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) { @@ -597,14 +589,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) { @@ -739,7 +723,6 @@ throw new Error('Cannot apply patch: no JSON') } - const previousContent = { json, text } const previousJson = json const previousState = documentState const previousText = text @@ -800,8 +783,6 @@ redo: operations } - emitOnChange(previousContent, patchResult) - return patchResult } @@ -1451,19 +1432,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) { From 4a49cc84f0a227335ebeb30358eb4b36b185cb92 Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Thu, 29 Feb 2024 20:04:32 +0100 Subject: [PATCH 2/3] chore: cleanup unused constants --- src/lib/components/modes/tablemode/TableMode.svelte | 1 - src/lib/components/modes/treemode/TreeMode.svelte | 2 -- 2 files changed, 3 deletions(-) diff --git a/src/lib/components/modes/tablemode/TableMode.svelte b/src/lib/components/modes/tablemode/TableMode.svelte index ca7fbe60..823328b3 100644 --- a/src/lib/components/modes/tablemode/TableMode.svelte +++ b/src/lib/components/modes/tablemode/TableMode.svelte @@ -371,7 +371,6 @@ return } - const previousContent = { json, text } const previousJson = json const previousState = documentState const previousText = text diff --git a/src/lib/components/modes/treemode/TreeMode.svelte b/src/lib/components/modes/treemode/TreeMode.svelte index d2314f3d..e1f551de 100644 --- a/src/lib/components/modes/treemode/TreeMode.svelte +++ b/src/lib/components/modes/treemode/TreeMode.svelte @@ -514,7 +514,6 @@ return } - const previousContent: Content = { json, text } const previousState = documentState const previousJson = json const previousText = text @@ -549,7 +548,6 @@ return } - const previousContent: Content = { json, text } const previousJson = json const previousState = documentState const previousText = text From 6f61950a9b620a53e9e20d5e3f22a6266f6e100b Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Thu, 29 Feb 2024 20:09:15 +0100 Subject: [PATCH 3/3] docs: update docs of `onChange` --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 320cb405..9888b1d8 100644 --- a/README.md +++ b/README.md @@ -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.