automerge-classic
automerge-classic copied to clipboard
Add OpId to remove actions in Patch
Currently we put OpId
(operation id) for SingleInsertEdit
and UpdateEdit
, but omit it for RemoveItem
. The same thing is true about map deletions -- we simply do {attr: {}}
and there's no way to know which operation is responsible for this change.
This is sometimes annoying, because we'd like to have some ordering of actions inside one patch and it's tricky (or impossible) without OpId
.
Automerge CI / browsertest (pull_request) Failing after 11m — browsertest
It also fails on main branch, so it has nothing to do with this PR. @ept, FYI.
Converted to draft -- this'll need more work, we also want removes of map keys to include op id.
E.g. currently we have {props: {}}
, but we want {props: {deletionOpId: null | RemoveItem}
. I'll try to implement it.
Hi @begor, thanks for this. The browsertest failure can be ignored (it's been flaky since I moved from Travis CI to GitHub Actions and I haven't yet figured out how to make it reliable again).
I am happy to consider this, but could you please tell me more about why you need this information in the patches? I don't currently see a need for it, and I am worried you might be doing something that goes against the grain of the intended use of Automerge. For map keys, {props: {deletionOpId: null}}
would not work, since that would be interpreted as setting the key to null, which is not the same as deleting the key.
Also, a heads up – we are planning to change the patch format/API with a view towards improving performance. We can also include deletion opIds in the new API if we have a need for it.
@ept I've updated the PR.
Why do we need this: we use Patch
to reconstruct state of the model when we load history. For this we use our own automerge frontend (which is more like a "patch interpreter") that doesn't keep the model in memory, but instead maps changes in JSON tree (as described in Patch
) to some actions (API calls, updates in other modules, etc.).
For this we'd like to sort updates inside a Patch by opId
to apply them in order of increasing lamport clocks. It also helpful to know which operation is responsible for which action in patch for various other reasons. Currently there's no way to do this, because opId
s are missing from deletes. So we add them here.
The implementation is a bit peculiar when it comes to map deletes -- here we use {deletionOpId: '___DELETED___'}
and update code to handle this situation. If there's a better way to do this -- let me know!