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.

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

Consider supporting JSONPatch #16

Open
LifeIsStrange opened this issue Nov 24, 2021 · 10 comments
Open

Consider supporting JSONPatch #16

LifeIsStrange opened this issue Nov 24, 2021 · 10 comments

Comments

@LifeIsStrange
Copy link

LifeIsStrange commented Nov 24, 2021

http://jsonpatch.com/
as the format is "standard" and is widely supported on backend stacks

@AsyncBanana
Copy link
Owner

That could be done, but it would require a breaking change and might come at a bit of a performance cost because of the value handling (the value can be an array or object instead of just going a level deeper).

@AsyncBanana
Copy link
Owner

I am returning to this, as this is more possible than I initially thought.
Question to everyone: Would you be willing to go through a breaking change if it meant that everything was compatible with JSONPatch? This would mean you would have to update some of your code, but it shouldn't be much more than some find and replace since the formats are very related, and it would allow for more integration without a compatibility layer. Also, this would make patching much easier since the libraries already exist (although I still might fully release Micropatch for the same reason I made Microdiff, but for JSONPatch patching).

@AsyncBanana AsyncBanana pinned this issue Jan 16, 2022
@LifeIsStrange
Copy link
Author

LifeIsStrange commented Jan 16, 2022

I am not an existing user but my frontend basically necessitate a diff library that output JSON-Patch given that libraries on the backend commonly expect (or generate) this format.
I think this breaking change is for the greater good as it allows standardization and interoperability with a wider ecosystem.

@AsyncBanana
Copy link
Owner

I am looking at doing this again. However, a feature some people rely on in Microdiff is the oldValue property, which does not exist in JSON Patch. I could add oldValue while otherwise keeping compatibility with JSON Patch. This would still be compatible with most things and would be simple enough to filter out if there is an incompatible library.
Another option would be to add a JSON Patch config option. This would allow for perfect JSON Patch compatibility while still making it possible to see the past value for those who need it and could even allow for no breaking change (although I would probably still release a new major version for other things). However, I try to minimize the configuration, as each config option increases size and decreases performance.
What do you think?

@LifeIsStrange
Copy link
Author

LifeIsStrange commented Jul 28, 2022

I would personally vote for the config but it's your codebase, do as you please :)

to be clear what you mean is: for the given input

{
  "baz": "qux",
  "foo": "bar"
}

and following patch

[
  { "op": "replace", "path": "/baz", "value": "boo" },
  { "op": "add", "path": "/hello", "value": ["world"] },
  { "op": "remove", "path": "/foo" }
]

microdiff would output:

{
"oldValueBaz": "qux",
"oldValueFoo": "bar",
"oldValueBaz": "???"
  "baz": "boo",
  "hello": ["world"]
}

instead of

{
  "baz": "boo",
  "hello": ["world"]
}

? What's the exact format
I believe in most cases those oldvalues would need to be filtered out (cpu time and increase network payload size and libraries incompatibility) although emitting them might be very useful for debugging and some validations/custom logics. I'm not aware of other use cases but maybe there are.

@AsyncBanana
Copy link
Owner

I am little confused about what you mean. I am not talking about adding patching functionality for Microdiff. What I am saying is that Microdiff would output JSON Patch statements. For example, if oldValue was included, the output could be:

[
  { "op": "replace", "path": "/baz", "value": "boo" , "oldValue": "qux"},
  { "op": "add", "path": "/hello", "value": ["world"] },
  { "op": "remove", "path": "/foo" , "oldValue": "bar"}
]

As for use cases, it is anything that needs the past value, as getting it from the path and the old object can be slow and annoying. For example, the old value would be helpful if you built a diff viewer.
About the config, I am leaning towards that, but the code would be made much larger and potentially slower.

@Julienng
Copy link

Julienng commented Dec 5, 2022

Hello, I think keeping "oldValue" is reasonable if any other key is compatible with json-patch.
One other way would be to expose a patch and a reverse patch, so you can apply / reverse some diff changes.

Maybe making json-patch available through a new file to import might help to avoid config / breaking changes.

import { diff, patch, reversePatch } from "microdiff/json-patch"

@Julienng
Copy link

Julienng commented Dec 5, 2022

I'm going to play with microdiff and json-patch soon by transforming the result to a patch.

Do you have a branch with json-patch? I might play with microdiff locally and try to implement something

@AsyncBanana
Copy link
Owner

@Julienng There is an implementation of patching functionality, although it is currently only on GitHub, not NPM (https://github.com/AsyncBanana/micropatch).
I could make it accessible through a subpath, although I could also just release a new major version with json-patch and continue maintaining v1 (although there is not much maintenance needed to do the library being compact and dependency-less).
Currently, I do not have a branch with json-patch, although it would be trivial to implement.

@GabrielCTroia
Copy link

+1 for this! Love your micro library :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants