Skip to content

Commit

Permalink
add notes about patch in place bug
Browse files Browse the repository at this point in the history
  • Loading branch information
jbakse committed Feb 8, 2022
1 parent 54f0e2d commit 150096c
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 1 deletion.
54 changes: 54 additions & 0 deletions notes/patch_problem.js
@@ -0,0 +1,54 @@
{
const before = {
obj: {
a: 1,
b: 2,
c: 3,
},
arr: [{ id: "a" }, { id: "b" }, { id: "c" }],
};
}


// these next two things start the same
// change the test in different ways
// that look the same in the end
// but the ref ends up pointing to different things
// can patch in place work in a way that preserves refrences?

// sort of this is what a human user would do
{
const test = [{ a }, { b }, { c }];
const ref = test[1];
test.remove({ b });
// test = [{a}, {c}]
// ref = {b} (removed)
}

// sort of this is how patch and place would deal with it
{
const test = [{ a }, { b }, { c }];
const ref = test[1];
test.remove({ c });
test[1] -> mutate to -> { c };
// test = [{a}, {c}]
// ref = { c } (test[1])
}

// patch in place doesn't know what happenend:
// when [{ a }, { b }, { c }] -> [{ a }, { c }]
// was b removed?
// was c removed AND b mutated to match c?



// is possible to preserve refrences into arrays
// is possible to preserve refrences into objects
// on possible (hard) way to do this would (maybe) be to give hidden and sync'd uuid's to each object

// does the incoming data sent to patch place for local initiated changes have the proper identies to detect the difference?
// does the incoming data sent to patch place for remote initiated changes have the proper identies to detect the difference?

// if its not possible to promise refrences stick, should we try at all?
// or would it be better to go with a niave overwrite everything approach, document loudly not to store reference, and maybe try to detect if someone does store a refrence and warn them? (can't detect creating the refrence, but can detect if someone reads/writes to it probably, since theyare proxies)

9 changes: 8 additions & 1 deletion notes/todo.todo
Expand Up @@ -4,6 +4,10 @@


## Bugs
-- Patch in place doesn't remove items from an array correctly
tries to remove index 0, 1, 2
but removing index 0 shifts 1, 2 to 0, 1
etc.


## Enhancements
Expand Down Expand Up @@ -133,7 +137,10 @@
I wrote a custom merge function in Record, but maybe it would be better to use a library?
* [Lodash.merge](https://lodash.com/docs/#merge)
* [Lodash.mergeWidth](https://lodash.com/docs/#mergeWith)
mergeWidth should be flexible enough to add debug reporting and customize behavior
these probably don't work, because they don't _remove_ properties
mergeWidth might be flexible enough to customize behavior add debug reporting
might be good to just study their merge and compare to the current party implementation
might be worth also studying the merge function in deepstream to see how the approaches differ, if one is better
https://github.com/deepstreamIO/deepstream.io/blob/892c0fea1b348cc5152e3b75cf19e3241ece3edc/src/utils/utils.ts#L77


0 comments on commit 150096c

Please sign in to comment.