Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: do not trigger onChange on programmatic changes #410

Merged
merged 3 commits into from
Mar 1, 2024
Merged

feat: do not trigger onChange on programmatic changes #410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
264ca58
feat: do not trigger onChange on programmatic changes (#318)
josdejong Feb 29, 2024
4a49cc8
chore: cleanup unused constants
josdejong Feb 29, 2024
6f61950
docs: update docs of `onChange`
josdejong Feb 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading