search

peer-base / js-delta-crdts

Delta State-based CRDTs in Javascript
192 stars 16 forks source link

embed ormap failing test #19

Closed salzhrani closed 5 years ago

salzhrani commented 5 years ago

Not sure if embedding ormap inside an ormap is supported. but adding seems to work fine ... removing fails. this PR adds failing tests

pgte commented 5 years ago

@salzhrani @samio nice catch, there's definitely something fishy going on. Thank you for the tests!

salzhrani commented 5 years ago

when I try to remove a key a get an error here. it seems that dots are undefined on dotStore. and it might get sorted out whenever this gets addressed.

salzhrani commented 5 years ago

after an attempt at debugging it seems that this line returns false after removing a causing the om key state not being set, thus the whole state for the ORMap becoming {}

salzhrani commented 5 years ago 
rreplica.applySub('om', 'ormap', 'applySub', 'a', 'mvreg', 'write', 'A1')
// state { om: a: [['A1']] }
expect(rreplica.value().om.a).to.deep.equal(new Set(['A1']))
// pass
rreplica.applySub('om', 'ormap', 'remove', 'a')
// state {}, both 'om' and 'a' sub-states gone
pgte commented 5 years ago

@salzhrani that line is required because we need to test that the removal supersedes the content. I'm not sure about what should be the fix here, will have to think a bit about this one...

pgte commented 5 years ago

@salzhrani I've been discussing this issue, and it looks like it's a consequence of how the ORMap works.

Because the absence of a key is defined by the value of the CRDT it points to being at the "bottom", which is the case.

Looks like this is a "feature" (that should be explained in the README).

Anyway, nice catch!

salzhrani commented 5 years ago

@pgte I changed the test to show the problem we are currently running into. I don't think it is related to the "bottom" behaviour.

pgte commented 5 years ago

This is happening because a lwwreg is not a causal CRDT. In the docs you can check which causal CRDTs support being embedded: https://github.com/ipfs-shipyard/js-delta-crdts#maps

salzhrani commented 5 years ago

Thank you. Sorry for taking more of your time.

pgte commented 5 years ago

No worries, thanks for reporting this!