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

Forked version with encoding/json replaced with fastjson - Copyright/License attribution #137

Closed
jsimnz opened this issue Mar 28, 2021 · 4 comments

Comments

@jsimnz
Copy link

jsimnz commented Mar 28, 2021

Hi

I have recently forked your project with the goal of replacing encoding/json with fastjson. You can find the project here: https://github.com/lens-vm/jsonmerge.

I have kept the license the same but didn't did create a "Github Fork", just copied over the elements I needed.

I just wanted to check in with you to make sure you're happy with the copyright attribution I gave to you in the code and readme (It's compliant with BSD, but I wanted to check in with you on a personal level).

Thanks for the great project!

@evanphx
Copy link
Owner

evanphx commented Apr 5, 2021

Sure, that looks fine. I'm open to figuring out how to support them together in one repo if you'd like as well.

@jsimnz
Copy link
Author

jsimnz commented Apr 5, 2021

Awesome. Do you mean replacing encoding/json with fastjson all together in your repo, or making it a configurable "backend" so to speak, so the user can choose which json package they want?

@evanphx
Copy link
Owner

evanphx commented Apr 5, 2021 via email

@jsimnz
Copy link
Author

jsimnz commented Apr 5, 2021

The main reason I wanted a fastjson alternative for json-patch is for the TinyGo WebAssembly support. TinyGo doesn't support the standard encoding/json library due to some reflection` issues.

Fastjson's approach is to keep everything as raw json values for as long as possible, similar to json.RawMessage. It only converts to a native go value if you access a field using one of the get functions, like GetString. As a result, you get amazing performance and don't rely on the Go reflect package.

As for the approach I used in replacing encoding/json with fastjson in this library, the main focus was replacing the lazyNode and partialDoc/partialArray with the fastjson.Value struct. The changes are very minimal as all the necessary operations, set, get, delete are done on the Value struct directly.

At the moment my fork only supports MergePatch and MergeMergePatch public functions as those were the only elements I needed for my use case, but I suspect updating the Patch/ApplyPatch system would be a similar endevor.

From what I can tell, most of the json-patch library internals rely on the lazyNode, partialDoc, partialArray, and Operation types. All of which should be easy to replace with a fastjson.Value type.

Eg. Replacing partialDoc and partialArray was simple in the MergePatch system since you can do a quick check using Value.Type() == fastjson.TypeObject or Value.Type() == fastjson.TypeArray, like so. Moreover, the way the fastjson handles arrays is to just return a []*fastjson.Value slice, which already matches the partialArray type, which means most of the code designed for partialArray works out of the box.

My fork only took about an hour or so to right, and short of a small bug I ran into, was rather pain free. Had all the merge related tests passing rather quickly.

@evanphx evanphx closed this as completed Jan 12, 2024
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

2 participants