Conflict is not recognised when there are multiple key assignments about automerge-classic HOT 8 CLOSED

This is a bug; thank you for the nice test case. The conflict ought to show up regardless in which order the documents are merged. I found that to reproduce the bug, you don't even need nested objects — the following is sufficient to reproduce the issue:

const s1 = Automerge.change(Automerge.init('22'), doc => { doc.x = 1 })
const s2 = Automerge.change(Automerge.init('11'), doc => { doc.x = 2; doc.y = 3 })
const m1 = Automerge.merge(s1, s2)
console.log(Automerge.getConflicts(m1, 'x'))

Internal details follow. The problem occurs when the first argument to merge() has the greater actorId. In this case, mergeDocChangeOps first applies the change op (the assignment to x from the second argument to merge), then loads the next change op (the assignment to y from the second argument to merge) while docOp is still the assignment to x from the first argument to merge. At this point, keyMatches, listElemMatches, and foundListElem are all false, and so we don't take the branch at backend/new.js:1169, and in particular we never reach the call to updatePatchProperty at backend/new.js:1203. Hence, docOp is not added to the patch and so it doesn't show up in the conflict.

The bug only shows up if the second argument to merge contains another op which is loaded into changeOp; if there is no such op (e.g. if we remove the assignment to y in the example above), changeOp still contains the previous op, so keyMatches == true, and therefore the patch is updated as it should be.

I'm not sure right now how to best fix this bug. Given that we're moving towards making the Rust/Wasm implementation of Automerge the primary one, the most important thing is to ensure this bug doesn't occur there.

from automerge-classic.

It should be almost 100% compatible, aside from requiring waiting for the automerge-wasm blob to load (browsers block synchronous WASM loading). We're currently working through a last few things before an offical release (like improving docs) but you can try https://github.com/automerge/automerge-rs/tree/main/automerge-js in the meantime and pass along any feedback if you run into problems.

from automerge-classic.

(You should be able to yarn add automerge-wasm automerge-js. The latter will become automerge as part of the change-over process.)

from automerge-classic.

I pushed a failing test case for this to the conflict-test branch (9857eeb)

from automerge-classic.

FWIW the failing test passes using the WASM backend.

from automerge-classic.

Great, then I think we don't need to fix this bug in the JS implementation since it's going to be deprecated anyway.

from automerge-classic.

How straightforward is it to migrate from the JS implementation to using the WASM backend?

from automerge-classic.

Thanks! I'll try it out at some point and see if any issues arise.

from automerge-classic.