Return something on change callback

[tomalec]

PR with suggested solution for #102
Let's return something to onChange callback.

[tomalec]

I made it simplest way possible, to reuse current jsoneditor params structure.
However I think in long term solution it would be good to use something more standarized, like
Object.observe or JSON-Patch format.

But I wonder maybe is should be separate PR, as then maybe we could also unify some internals.

[josdejong]

Thanks for your PR Tomec.

I think this solution does help a little, but is no complete solution, and will give a lot of hassle for the user to do something useful with this. I think a solution like JSON-Patch will work great, and it may not be too difficult to implement. It should be implemented two way: output change objects on change, but also accept changes as input and adjust the document accordingly. Then integration with for example sharejs will be trivial.

I rather want to go for a "real", long term solution rather than having a "half" solution causing a lot of questions and frustration.

Few practical things:

• this.options.change() occurs three times in the code, so should be adjusted three times.
• please do the changes in the /src/ folder, the jsoneditor.js file is auto generated from these source files.

[tomalec]

Sorry for such messy PR. I have just needed quick workaround for yesterdays conference. I will upgrade it soon (at least those two things you mentioned).

I use it inside Polymer Custom Element, and this feature would be useful to preserve Polymer's two-way (HTML-JS) data binding.

I do not have experience with sharejs, but I also integrate it with <puppet-js> (PuppetJs ) For three-way databinding with any server (HTML-JS-Server).

That is great that you also like going for "real" :) but do you think it should also use JSON-Patch for internal history manipulations (-> use already made implementation), or just expose/consume changes in this format?

[josdejong]

I think JSONEditor should use its own protocol internally, this can be similar to JSON-Patch, but for example we can gain a lot of performance by keeping detached DOM elements in memory and reusing them (for example when moving a subtree from one place to another).

[tomalec]

@josdejong,

• tomalec@e6321ca is quite stable, but returns only what is in action, params
• starting from tomalec@0ce73ab change callback is called with JSON-Patch, but I faced few problems:
  1. I'm not sure if I fit your code style ;)
  2. Isn't such mapping too big overhead, and maybe I should integrate JSON-Patch deeper?
  3. Problems with mapping particular ops:
     1. duplicateNode duplicates it with same name, so any operation on real JSON object will overwrite original value, (not only JSON-Patch issue, but also problem for anybody that tries to observe it and map to real JSON),
     2. changeType When I change type from Number to String, params.node.value changes from 123 to "123" - it's obvious to return: {op: "replace", path: "/number", value: "123"}. However if I change it to Object or Array, ..value is "", shouldn't it be {}, or []?
     3. insertBefore,appendNode when node is inserted to an array, I cannot find any trace of old index where it was inserted or pushed, to add correct index or - to patch.
     4. moveNode again there is a problem with array items, and "" as key. First moved item loses it's index and kay is changed to "", so I cannot guess should I use "", and replace anything that was there before under "" key, or try to use original index? When I move second one, it also uses "" which leads to duplicated keys conflict as described above.

[josdejong]

Nice job so far! Your code looks well structured and well documented. I might try to come up with a shorter name for the function translateChangeToJSONPatch though ;).

I don't see any shockingly heavy operations, is there some reason you think there will be performance issues?

a. For the duplicateNode issue we could give the copied node a field like field (1), similar to copying a file on your computer. Or could we use some hidden, unique identifier somehow?
b. Does it make sense to make it null? In case of an Array and Object we use Node.childs instead of Node.value
c. So far there was no need to store the index, I could just do .insertBefore(node, params.beforeNode). I think you can figure out the index using parent.childs.indexOf(beforeNode) or something like that.
d. In case of Arrays, the childs do not have a key but an index, can you use the index in that case?

[MiroHibler]

Hey guys, is this going to happen? I'd like to see it in master ;)

[timelyportfolio]

👍 Are there plans to accept this or to provide similar functionality? Thanks so much.

[josdejong]

Time to pick this up again. @tomalec are you still interested in finishing this feature?

[timelyportfolio]

I'm very interested as I am wrapping jsoneditor as an R htmlwidget called listviewer. The change callback will be very important for dynamic/reactive environments in Shiny.

[josdejong]

Thanks @timelyportfolio, would be great if you could pick this up!

[mmacfadden]

I actually came across this issue because I actually started implementing it myself. I would be interested in helping out.

[josdejong]

Thanks for your offer @mmacfadden would be great if you could give this a go.

[tomalec]

Sorry for leaving it for so long. I'm still interesting in make it done.
@mmacfadden do you have your fork somewhere, it would be easier to get merged , as probably I might be very outdated :/

[djmattyg007]

Any more progress on this?

[josdejong]

@djmattyg007 yep, I'm working on a completely new version of the JSONEditor which works with JSON-Patch internally. So the onChange events give you a patch with the changes, and there is a new .patch() method to apply patches.

I've opened an issue describing the progress of this new version, see: #337. It will still take a couple of months to finish.

[djmattyg007]

Awesome!