Comments (10)
Why deleteProperty trap is not able to catch it correctly?
Because that's what the JS engine actually does. If you watch the traps in slow-motion (so to say) you see the engine replacing (shifting left) array elements one by one starting from the spliced item until it reaches the end then it removes the last element, which only then triggers the deleteProperty
trap.
And this makes sense. You want arr[5]
value to be different before and after you do splice(2,1)
. Right? You want the elements shifted.
from json-patch.
👍 Same issue here.
from json-patch.
@Pavek Thanks for reporting, and nice test case :) It seems it comes from limitations of Object.observe
that we use. I will try to figure something out, maybe try how Array.observe
affects performance, and how hard is it to shim.
@warpech have we ever consider Array.observe
?
from json-patch.
@Pavek, @winduptoy I have created branch for this fiddling with Array.observe
.
@Pavek test case passes with native (Object && Array).observe
, I have also added test case with full featured .splice
removes and adds many items. Please, check if it works fine for you.
If so, I'll move on with performance and shimming work.
from json-patch.
It seems working without native Array.observe
is much more complicated. I created algorithm to generate nicer patches, also form manual generate
calls (.compare
, .generate
, shimmed observer): https://github.com/tomalec/JSON-Patch/tree/issues/65_ArrayObserve_shim
and it works nice with native observers.
The problem comes with shim, as we keep mirror as deep clone so there is no way to check that
for
obj = { items: [{name:"obj1"}, {name:"obj2"}, {name:"obj3"}]};
new obj.items[1]
is equal to any of old obj.items[?]
.
@warpech any ideas?
Maybe we could serve better and optimized patches with native support, and leave it as it was for shimmed? - then, naturally, we would not serve different patches for same scenario for shimmed and native observers; equivalent but not same.
from json-patch.
Related SO issue http://stackoverflow.com/questions/29414710/is-there-any-way-to-preserve-the-order-while-generating-patch-for-json-files
from json-patch.
We could keep a shallow clone of arrays to check that. This way you could compare arr.indexOf(obj)
with arrShallowClone.indexOf(obj)
from json-patch.
The question is where to keep those clones? as currently we keep only one deepClone
of everything as Mirror
from json-patch.
Please see #204 (comment)
I don't think we'll implement a fix for this. Because I think our observation efforts will be mainly focused on JSONPatcherProxy.
Closing but feel free to reopen.
from json-patch.
Why deleteProperty
trap is not able to catch it correctly?
const JSONPatcherProxy = require('jsonpatcherproxy');
var myObj = { items: ["a", "b", "c"]};
var jsonPatcherProxy = new JSONPatcherProxy( myObj );
var observedObject = jsonPatcherProxy.observe(true);
myObj.items.splice(1, 1); // <<-- removing second item in-place
var patches = jsonPatcherProxy.generate();
https://runkit.com/tomalec/5b151a476d7288001244bd25
from json-patch.
Related Issues (20)
- Add user-defined hooks into the deepClone function
- Inefficient patch encoding when prepending to arrays HOT 2
- rrrr HOT 1
- Add performance comparison with jsondiffpatch
- Array patching, over empty elements.. HOT 2
- Any plans for a new release? HOT 1
- Maybe it's already discussed, but anyone think of how to patch array member in a more flexible approach?
- Release 3.1.1 request based on PR merged of #262 HOT 2
- Inaccurate documentation HOT 1
- Error index always 0? HOT 1
- Optional observe callback not called HOT 1
- applyPatch() validator index is not incrementing
- Backport fix for GHSA-8gh8-hqwg-xf34 to v2 HOT 5
- package.json should use `conditional exports`, else esm won't work inexplicitly HOT 1
- Node12+ import documentation incorrect
- Documentation for `generate` mentions wrong signature
- auto add tests to "add/remove" operations like for "replace"
- Mixing default exports and named exports breaks parcel HOT 2
- Attempting to use if mirror has a toJSON and object does not breaks HOT 2
- applying a replace patch is inconsistent with path="" and path="/"
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from json-patch.