onChange not called when .patch() applies JSON patches

[RyKilleen]

Hi there! First off, thanks for two excellent libraries that have been incredibly useful!

It was my expectation that calling the .patch method would result in a call to the onChange callback, but that doesn't seem to be the case in my setup. Is that the expected result? It's worth noting that it does trigger validation.

Hoping to narrow down whether that's working as intended or due to my local setup.

(I'll do some work for a minimal repro)

[RyKilleen]

A reproduction is available here:

https://codesandbox.io/s/svelte-jsoneditor-react-forked-u6r5w1?file=/src/SvelteJSONEditor.js

[josdejong]

Thanks Ryan!

Sorry for the confusion. The idea is indeed that the onChange callback is indeed only triggered by user action, and not by programmatic changes via .patch(), .set(), .update().

Reason is that if onChange would always fire, you can easily get infinite loops (at least, I myself had troubles with this in the past in various occasions): the user makes a change, onChange is fired, the updated document is put in some React useState variable, changing this state triggers a rerender, which invokes .update() to bring the editor up to date, which in turn triggers onChange, etc. Thinking about it now, it may be that this is not an issue anymore since I've put some measures in place in .update() and .set() to check whether the updated document is actually changed, and if not, do nothing. 🤔

I'll at least clarify this in the docs with the current behavior.

[RyKilleen]

I appreciate the clarification! It makes sense and I can manually handle the onChange and and patch in parallel myself, thanks!

[RyKilleen]

A quick follow-up to this question, would there be any interest in exposing an onPatch or afterPatch callback publicly? That'd enable me to structure my code in the way I'm hoping to, reusing my handleChange in two places instead of also calling it somewhere else with the same OnChange type.

edit:
In reality, the desire is for an onValidated hook. My particular need doesn't care about the source of the change, user or programmatic, only that changes happened and the JSON is validated. Unfamiliar with svelte but happy to get familiar and PR if wanted/needed!

A more detailed background of my use case:

I have a large JSON configuration and a form that submits this configuration if there are changes. I have a set of different types of patches that can be made to the object, initiated by the user (previously done manually.) It's an awesome feature to be able to patch, but now not possible for me to react to diffs from the from through onChange.

[josdejong]

Yes, good questions.

There is an onPatch but it is called onChange 😁. The patch operations (both undo and redo operations) are passed via the object in the third argument of onChange, it is called patchResult . The validation results are also passed there, this is named contentErrors. You can also call the .validate() method yourself at any moment (I see that method is not described in the docs, I'll add that). Is that what you mean?

function handleChange(updatedContent, previousContent, { contentErrors, patchResult }) {
  console.log('onChange: ', { updatedContent, previousContent, contentErrors, patchResult })
}

(on a side note: the exact function signature of the onChange callback may change before the library reaches v1.0.0)

[RyKilleen]

What I'm hoping to do is have my external UI care only about the output of a single callback, to determine whether valid changes have been made in the editor, whether by a user or programmatically via patch. If a user makes a change and it's valid, I'd like to know about that. If a patch is applied and results in a valid change, I'd like to know about that.

So far I can't find a way forward with the currently exposed API. The onChange callback isn't called when a patch is applied. I can't reuse the same callback, since .patch() doesn't have the same signature as onChange or return the newly transformed JSON.

[josdejong]

I guess you can create a callback somethingChanged(...), and invoke this function both in the onChange callback and right after calling .patch()?

[RyKilleen]

In my testing, .patch() returns void immediately and there's no way for me to determine the updated config like provided from onChange. editor.get() at that point in time returns the previous config (it's a massive JSON object, for what it's worth.)

The goal
Update external state when JSONEditor has valid changes, through UI or patch

My efforts so far have been:

1. Use onChange callback
   • we can't expect that to be called after .patch() (totally reasonable for the reasons you stated!)
2. Call patch, get new content with editor.get(), then manually pass previous and current content to my handleOnChange
   • .patch() returns void immediately, and I can't easily determine when patch is applied to get new content
3. .validate() lets me manually validate the editor, but again doing so immediately after .patch() validates the previous state, not the incoming latest one.

[josdejong]

Thanks for your inputs. Two thoughts:

1. I think it makes sense if we let .patch() return the same data as you get via onChange
2. I want to try out if I can change onChange to always fire (like you originally expected). It must not lead to infinite loops though and should not degrade performance if used in for example a typical React app, so I'll have to do some testing there.

Please think along if you have more ideas 😄

[RyKilleen]

I love both of those approaches! My hunch is that #2 should be possible and is preferred for a few reasons:

1. I'd love to leverage the JSON patch history returned from onChange and send it server-side (in the past we used deep diff to send diff patches, this would let us delete a lot of code!)
2. If we think of JSON Editor as an Input, and I'm using onChange and controlling state in React, it's on me as the implementer to do that responsibly

Another alternative comes to mind: consider a library like React Hook Form (RHF) as an interesting analogy. handleSubmit takes two callbacks, one for successful validation and one for errors. RHF takes user or programmatic changes, validates the data, and then provides the data to you on successful validation only, letting us react to the tail end of the change -> validate -> success/error cycle.

Mostly my external code wants to know the results of all the hard work JSONEditor has done, and react to those results without meddling too much in the internals.

[RyKilleen]

(And of course thanks for your time and collaboration!)

[josdejong]

Yes agree, it will make life easier if onChange always fires. I'll keep your handleSubmit example in mind and do some tinkering with the API 👍

(And of course thanks for your time and collaboration!)

You're welcome :)

[josdejong]

OK I've changed the behavior to always fire onChange, and return results from patch() (changes not yet published)

It was quite funny. I realized that this "round-trip" that I was concerned about is already happening all the time: every time a user does make a change 😂. And well, there is no performance problem with that at all and no loops. So, no problem to add this "round-trip" too when making a programmatic change. This change actually simplified the code for text mode, which is a nice side-effect.

[josdejong]

Changes are now published in v0.7.0

[RyKilleen]

That's great to hear! Can't wait to pull em down and try them out. Thanks for taking the time and glad there were some surprise benefits!

[josdejong]

I've just published a breaking v0.22.0 which reverts the changes made for this issue: onChange no longer triggers on programmatic changes made via .patch, .set, or .update. See #318 for details.