Skip to content

Commit 201f602

Browse files
authoredMar 1, 2024··
feat: do not trigger onChange on programmatic changes (#410)
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()`.
1 parent 4e188fe commit 201f602

File tree

4 files changed

+48
-76
lines changed

4 files changed

+48
-76
lines changed
 

‎README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ Callback fired when an error occurs. Default implementation is to log an error i
387387
onChange(content: Content, previousContent: Content, changeStatus: { contentErrors: ContentErrors | null, patchResult: JSONPatchResult | null })
388388
```
389389

390-
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()`.
390+
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()`.
391391

392392
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.
393393

‎src/lib/components/modes/tablemode/TableMode.svelte

+7-21
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,6 @@
371371
return
372372
}
373373
374-
const previousContent = { json, text }
375374
const previousJson = json
376375
const previousState = documentState
377376
const previousText = text
@@ -419,13 +418,6 @@
419418
previousText,
420419
previousTextIsRepaired
421420
})
422-
423-
// we could work out a patchResult, or use patch(), but only when the previous and new
424-
// contents are both json and not text. We go for simplicity and consistency here and
425-
// let the function applyExternalContent _not_ return a patchResult ever.
426-
const patchResult = null
427-
428-
emitOnChange(previousContent, patchResult)
429421
}
430422
431423
function applyExternalSelection(externalSelection: JSONEditorSelection | null) {
@@ -580,7 +572,6 @@
580572
throw new Error('Cannot apply patch: no JSON')
581573
}
582574
583-
const previousContent: Content = { json }
584575
const previousJson = json
585576
const previousState = documentState
586577
const previousTextIsRepaired = textIsRepaired
@@ -638,26 +629,21 @@
638629
redo: operations
639630
}
640631
641-
emitOnChange(previousContent, patchResult)
642-
643632
return patchResult
644633
}
645634
646635
function handlePatch(
647636
operations: JSONPatchDocument,
648637
afterPatch?: AfterPatchCallback
649638
): JSONPatchResult {
650-
if (readOnly) {
651-
// this should never happen in practice
652-
return {
653-
json,
654-
previousJson: json,
655-
redo: [],
656-
undo: []
657-
}
658-
}
639+
debug('handlePatch', operations, afterPatch)
659640
660-
return patch(operations, afterPatch)
641+
const previousContent = { json, text }
642+
const patchResult = patch(operations, afterPatch)
643+
644+
emitOnChange(previousContent, patchResult)
645+
646+
return patchResult
661647
}
662648
663649
function emitOnChange(previousContent: Content, patchResult: JSONPatchResult | null) {

‎src/lib/components/modes/textmode/TextMode.svelte

+34-22
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@
173173
escapeUnicodeCharacters
174174
})
175175
176-
$: setCodeMirrorContent(externalContent)
176+
$: setCodeMirrorContent(externalContent, false, false)
177177
$: applyExternalSelection(externalSelection)
178178
$: updateLinter(validator)
179179
$: updateIndentation(indentation)
@@ -250,14 +250,20 @@
250250
})
251251
252252
export function patch(operations: JSONPatchDocument): JSONPatchResult {
253-
debug('patch', operations)
253+
return handlePatch(operations, false)
254+
}
255+
256+
export function handlePatch(operations: JSONPatchDocument, emitChange: boolean): JSONPatchResult {
257+
debug('handlePatch', operations, emitChange)
254258
255259
const previousJson = parser.parse(text)
256260
const updatedJson = immutableJSONPatch(previousJson, operations)
257261
const undo = revertJSONPatch(previousJson, operations)
258-
setCodeMirrorContent({
262+
const updatedContent = {
259263
text: parser.stringify(updatedJson, null, indentation) as string
260-
})
264+
}
265+
266+
setCodeMirrorContent(updatedContent, emitChange, false)
261267
262268
return {
263269
json: updatedJson,
@@ -275,11 +281,12 @@
275281
}
276282
277283
try {
278-
const json = parser.parse(text)
279-
setCodeMirrorContent({
280-
text: parser.stringify(json, null, indentation) as string
281-
})
282-
askToFormat = true
284+
const updatedJson = parser.parse(text)
285+
const updatedContent = {
286+
text: parser.stringify(updatedJson, null, indentation) as string
287+
}
288+
289+
setCodeMirrorContent(updatedContent, true, false)
283290
284291
return true
285292
} catch (err) {
@@ -297,11 +304,12 @@
297304
}
298305
299306
try {
300-
const json = parser.parse(text)
301-
setCodeMirrorContent({
302-
text: parser.stringify(json) as string
303-
})
304-
askToFormat = false
307+
const updatedJson = parser.parse(text)
308+
const updatedContent = {
309+
text: parser.stringify(updatedJson) as string
310+
}
311+
312+
setCodeMirrorContent(updatedContent, true, false)
305313
306314
return true
307315
} catch (err) {
@@ -319,9 +327,12 @@
319327
}
320328
321329
try {
322-
setCodeMirrorContent({
330+
const updatedContent = {
323331
text: jsonrepair(text)
324-
})
332+
}
333+
334+
setCodeMirrorContent(updatedContent, true, false)
335+
325336
jsonStatus = JSON_STATUS_VALID
326337
jsonParseError = null
327338
} catch (err) {
@@ -345,7 +356,7 @@
345356
rootPath: [],
346357
onSort: async ({ operations }) => {
347358
debug('onSort', operations)
348-
patch(operations)
359+
handlePatch(operations, true)
349360
},
350361
onClose: () => {
351362
modalOpen = false
@@ -384,7 +395,7 @@
384395
})
385396
} else {
386397
debug('onTransform', operations)
387-
patch(operations)
398+
handlePatch(operations, true)
388399
}
389400
},
390401
onClose: () => {
@@ -445,7 +456,7 @@
445456
446457
function handleAcceptTooLarge() {
447458
acceptTooLarge = true
448-
setCodeMirrorContent(externalContent, true)
459+
setCodeMirrorContent(externalContent, true, true)
449460
}
450461
451462
function handleSwitchToTreeMode() {
@@ -671,12 +682,12 @@
671682
}
672683
}
673684
674-
function setCodeMirrorContent(newContent: Content, forceUpdate = false) {
685+
function setCodeMirrorContent(newContent: Content, emitChange: boolean, forceUpdate: boolean) {
675686
const newText = getText(newContent, indentation, parser)
676687
const isChanged = !isEqual(newContent, content)
677688
const previousContent = content
678689
679-
debug('setCodeMirrorContent', { isChanged, forceUpdate })
690+
debug('setCodeMirrorContent', { isChanged, emitChange, forceUpdate })
680691
681692
if (!codeMirrorView || (!isChanged && !forceUpdate)) {
682693
return
@@ -698,7 +709,8 @@
698709
}
699710
700711
updateCanUndoRedo()
701-
if (isChanged) {
712+
713+
if (isChanged && emitChange) {
702714
emitOnChange(content, previousContent)
703715
}
704716
}

‎src/lib/components/modes/treemode/TreeMode.svelte

+6-32
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,6 @@
514514
return
515515
}
516516
517-
const previousContent: Content = { json, text }
518517
const previousState = documentState
519518
const previousJson = json
520519
const previousText = text
@@ -533,14 +532,6 @@
533532
previousText,
534533
previousTextIsRepaired
535534
})
536-
537-
// we could work out a patchResult, or use patch(), but only when the previous and new
538-
// contents are both json and not text. We go for simplicity and consistency here and
539-
// let the functions applyExternalJson and applyExternalText _not_ return
540-
// a patchResult ever.
541-
const patchResult = null
542-
543-
emitOnChange(previousContent, patchResult)
544535
}
545536
546537
function applyExternalText(updatedText: string | undefined) {
@@ -557,7 +548,6 @@
557548
return
558549
}
559550
560-
const previousContent: Content = { json, text }
561551
const previousJson = json
562552
const previousState = documentState
563553
const previousText = text
@@ -597,14 +587,6 @@
597587
previousText,
598588
previousTextIsRepaired
599589
})
600-
601-
// we could work out a patchResult, or use patch(), but only when the previous and new
602-
// contents are both json and not text. We go for simplicity and consistency here and
603-
// let the functions applyExternalJson and applyExternalText _not_ return
604-
// a patchResult ever.
605-
const patchResult = null
606-
607-
emitOnChange(previousContent, patchResult)
608590
}
609591
610592
function applyExternalSelection(externalSelection: JSONEditorSelection | null) {
@@ -739,7 +721,6 @@
739721
throw new Error('Cannot apply patch: no JSON')
740722
}
741723
742-
const previousContent = { json, text }
743724
const previousJson = json
744725
const previousState = documentState
745726
const previousText = text
@@ -800,8 +781,6 @@
800781
redo: operations
801782
}
802783
803-
emitOnChange(previousContent, patchResult)
804-
805784
return patchResult
806785
}
807786
@@ -1451,19 +1430,14 @@
14511430
operations: JSONPatchDocument,
14521431
afterPatch?: AfterPatchCallback
14531432
): JSONPatchResult {
1454-
if (readOnly) {
1455-
// this should never happen in practice
1456-
return {
1457-
json,
1458-
previousJson: json,
1459-
undo: [],
1460-
redo: []
1461-
}
1462-
}
1463-
14641433
debug('handlePatch', operations, afterPatch)
14651434
1466-
return patch(operations, afterPatch)
1435+
const previousContent = { json, text }
1436+
const patchResult = patch(operations, afterPatch)
1437+
1438+
emitOnChange(previousContent, patchResult)
1439+
1440+
return patchResult
14671441
}
14681442
14691443
function handleReplaceJson(updatedJson: unknown, afterPatch?: AfterPatchCallback) {

0 commit comments

Comments
 (0)
Please sign in to comment.