search

Palindrom / JSONPatcherProxy

ES6 proxy powered JSON Object observer that emits JSON patches when changes occur to your object tree.
94 stars 14 forks source link

Revert Patches #33

Closed jonahbron closed 5 years ago

jonahbron commented 5 years ago

Is there any interest in adding undo capability to this utility? In my particular case, it would be super useful if I could get not only the patch that represents the change that was made, but also a patch that has the ability to revert that patch. This would allow me to build undo/redo stacks on a whole document tree.

jonahbron commented 5 years ago

@alshakero I'm willing to implement this feature if you think you'd merge it in.

alshakero commented 5 years ago

The only requirement of this library, other than that it works, is performance. We use it in a performance-critical way. And adding features that slow it down, that don't comply with the RFC, is not part of our strategy.

jonahbron commented 5 years ago

Is that a definite "no"? In case you're undecided, I don't think it would introduce a substantial impact on performance, especially if it's made an optional setting.

And there's nothing non-compliant about patches that can undo a change. It would emit a pair of patches every change, instead of a single patch. One patch that can change the model state from the old to the new, and one patch that can change the model state from the new to the old.

warpech commented 5 years ago

Thanks for active participation in this open source project.

This proposal would change the nature of this project from single-purpose to multi-purpose, which I think is not desired

My advice is to create a fork of it, call it like ReverseJSONPatcherProxy. It would perform only the reverse patches. Then you can put both proxies on your objects.

Or, I think this library could accept a PR that adds target, key, newValue, oldValue arguments to the generate callback allowing you to extend it with custom functionality.

WDYT?

jonahbron commented 5 years ago

@warpech I've decided to go in another direction to solve this problem on my end, but I do like your idea of extending with a callback.