Open to two PRs?

[hunterloftis]

Hi, I found symmetry from your HN comment. I'm developing something similar (optimizing for both speed & size) and I'd rather contribute to / use Symmetry if possible.

Would these two PRs align with your goals for the project?

1. make "none" and "reset" actionable / transferable patch types

Instead of sending the string"reset," then leaving it to the app to handle resets out-of-band, it seems like the actually-valid patch for something that needs to be reset should be something like { t: "r", s: ObjectPatchSet<T> }.

For "none," it might make sense to export a const (to avoid stringly-typed typos), but it seems like { t: "n" } would align even better with the patterns already here.

The benefit here is that if you're communicating patches - say, over a websocket connection - then you can just blindly send whatever the output of diff is, and on the other end you know that the patched object will end up identical ("symmetrical") to the local one. No need for special-casing on "none" or "reset" to transmit data out-of-band.

2. add a test for sliding window operations on arrays

One of the things I think is cool about this lib is that it's fast while also creating relatively small diffs. I'm curious how a sliding window diff looks at the moment, since it's a common operation for me:

[1, 2, 3, 4] -> [2, 3, 4, 5] -> [3, 4, 5, 6]

If those sound like something you'd consider, I'll work up PRs for them.

[stephank]

1. make "none" and "reset" actionable / transferable patch types

I have special handling for these, but hadn't considered separate t types. I think because it wasn't really possible in the applyPatch API of previous versions that also supported in-place patching. But now that we treat everything as immutable, separate types makes a lot more sense. I'll happily merge a PR and do a major version. 🙂

Though I think I prefer { t: "r", v: PlainObject<T> }. The s in an object patch is short for 'set', but I think v for 'value' makes more sense in a reset patch? Plus ObjectPatchSet is incorrect, I think, because it wraps with Partial, while we need a full value in this case.

2. add a test for sliding window operations on arrays

It creates splices to be applied in order:

> createPatch([1, 2, 3, 4], [2, 3, 4, 5])
{ t: 'a', s: [ [ 4, 0, 5 ], [ 0, 1 ] ] }
> createPatch([2, 3, 4, 5], [3, 4, 5, 6])
{ t: 'a', s: [ [ 4, 0, 6 ], [ 0, 1 ] ] }

In this simple example, it's not really shorter, but I built this for larger arrays containing objects.

Adding a test for this sounds good. 🙂

[hunterloftis]

{ t: "r", v: PlainObject<T> } sounds good, :+1:

I tested the sliding window myself (also using arrays of objects) and realized it would make more sense to convert the array to a number-keyed object, at least for my particular use-case. That way every patch specifies just two operations (1 set and 1 delete).