automerge / automerge-classic

A JSON-like data structure (a CRDT) that can be modified concurrently by different users, and merged again automatically.
http://automerge.org/
MIT License
14.76k stars 467 forks source link

Generating `deps` of `Change` #429

Closed begor closed 3 years ago

begor commented 3 years ago

Hello!

From documentation it's a bit unclear how we can generate deps field of a Change object:

The responsibilities between frontend and backend are assigned as follows:

* The frontend assigns sequence numbers and operation IDs to locally generated
  changes, and produces a JSON representation of the change.
* The backend translates the JSON change into the binary encoding, **computes its
  hash, and fills in the dependency hashes as described below.**

But it doesn't look like this for me, reading existing frontend code, in frontend/index.js:makeChange() we actually set deps.

Moreover, when I'm writing my own frontend for automerge, it looks like I should fill up deps on my own, because backend actually checks my startOp against them here.

So there are two questions, basically:

1) Who should fill up deps for a Change? Documentation states that this is a backend responsibility, but code looks different to me. 2) What the exact semantics of deps? Suppose I generate a change, what deps should it have? Only the "head" of history (as with automerge.Backend.getHeads)? Or hash of every change that our new change "references" (e.g. it mutates the object that was created by some other change)?

ept commented 3 years ago

We went back and forth on this a bit, so the documentation is not quite up-to-date with the code. In the current implementation, the frontend fills in deps for any changes received from other actors, but the backend adds a dependency on the previous change by the local actor. See the comment in the linked code for an explanation.

Deps only needs to include the heads (which is typically just a single hash, except in the case of a "merge commit" which would have two or more deps). All changes that can be transitively reached through the chain of deps are implicitly included as dependencies, so there is no need to reference specific changes in the past.

begor commented 3 years ago

Deps only needs to include the heads (which is typically just a single hash, except in the case of a "merge commit" which would have two or more deps). All changes that can be transitively reached through the chain of deps are implicitly included as dependencies, so there is no need to reference specific changes in the past.

Currently I'm doing something like this for every change:

const change = {
     ...
     deps: automerge.Backend.getHeads(backendState)

The change then goes straight to automerge.Backend.applyChanges(...). Is this a correct approach?

ept commented 3 years ago

Yes I think that should be fine.

begor commented 3 years ago

Thanks, Martin!