Additional validation for operations + shallow copies when applying operations #19
Conversation
By default, splice does no bounds checking and leads to null entries and undesired data. Adding additional validation here also helps detect divergence and sync errors before the local operations have been applied and sent to the server.
There are times when it is convenient and useful to attach "metadata" like properties to operation components. The validity of this data is independent of transformation so we would like to maintain that data regardless of transformation.
validate proper 'od' and 'ld' when replacing or deleting to ensure nothing is inadvertently lost.
- This is similar to the ImmutableJS pattern and makes json0.appy more React-friendly - No noticeable performance changes - allows for better object reference equality checks
ryanwe
force-pushed
the
master
branch
3 times, most recently
from
0d7b3e4
to
abd235a
Compare
|
Cool. Tests are passing which also means better node and browser compatibility. 👍 |
ryanwe
changed the title
|
@josephg what's happening with this PR? I think these checks are hugely valuable. |
|
Wow; there is a lot of great stuff here. I wish I merged a lot of this ages ago - my focus has been on newer, green-fields stuff. For what its worth, I agree that I'm not going to take this PR for two reasons:
I'd love to merge most of this. But I'm going to close this issue as-is. Feel free to re-open individual PRs with the changes and we can talk about them. I know its a pain. I'm sorry. But I don't want to blindly take responsibility for this code as-is. Going forward it'd be nice to get some more committers direct access to this repo. Once I merge a few commits I'll be happy to just give you commit access and you can do the rest yourself |
This library is awesome and has been super helpful! In the course of using it, we found it helpful to add some validation to ensure that the operations being submitted are valid and fail fast. This means validating that the
ldorodobject is actually the expected object being deleted, range checks forl*operations, and thatoioperations includeodwhen they are overwriting/deleting an existing object.This PR also includes a change to clone the entire operation instead of rebuilding it from scratch. We found places where attaching additional properties to an operation was helpful, and we want to maintain those properties after transforming the operations.
We use
webpack(and sometimesbrowserify) to include this library on the client side and are able torequire('assert')but if that doesn't work, I can add a customassert.deepEqual()function to the code.If this is considered a breaking change (due to the new errors and assertions), I can bump the version number too.
Like I said, this library has been great to work with and I'm happy to contribute back. Let me know if these changes look good or if they require additional modification.
Thanks!