-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Compose does not preserve invertable operations #20
Comments
Yep good find. This is a known issue, but we should document it and fix it. From here:
First step, we should update the readme to talk about this stuff. Having a footgun like this is bad form. Until compose has been fixed, use const invertedOp = json1.type.invertWithDoc(op, doc) .. and that will fix the issue. Alternately call const op = json1.type.makeInvertible(json1.type.compose(op1, op2), doc) |
I have never wanted to use the |
You need a copy of the document when you call ... Are you calling compose on old operations? How are you using it? |
I misunderstood record (op, doc) {
if (op.length === 0) {
return
}
this.stack.redo = []
let undoOperation = json1.type.invert(op)
const timestamp = Date.now()
if (
this.lastRecorded + this.options.delay > timestamp &&
this.stack.undo.length > 0
) {
const { operation: lastOperation } = this.stack.undo.pop()
undoOperation = json1.type.makeInvertible(json1.type.compose(undoOperation, lastOperation), doc)
} else {
this.lastRecorded = timestamp
}
if (!undoOperation || undoOperation.length === 0) {
return
}
this.stack.undo.push({ operation: undoOperation })
} and it works well now after by using |
Yeah that looks ok.
Huh? The document you pass needs to be the state before you apply the operation. Otherwise deleted content will have already been removed before we can copy it into the op. |
it equal to this.options.delay > timestamp - this.lastRecorded I think it is greater than timestamp is right, because if the two operations are within the delay, the two operations will be merged into one operation through |
Bad case:
Expect
result2
equal todoc
? But they are not equal.The text was updated successfully, but these errors were encountered: