Do not `invert` automatically in `Doc.prototype._rollback`

[antoinelyset]

Hey ShareDB's team!

While using https://github.com/ottypes/json1 with ShareDB, I found a small problem in Doc.prototype._rollback. It automatically uses JSON1.type.invert but since we don't use invertible operations (our remove ops do not contain the removed data), it does not work.

sharedb/lib/client/doc.js

Lines 950 to 957 in fa9179d

	if ('op' in op && op.type.invert) {
	op.op = op.type.invert(op.op);
	// Transform the undo operation by any pending ops.
	for (var i = 0; i < this.pendingOps.length; i++) {
	var transformErr = transformX(this.pendingOps[i], op);
	if (transformErr) return this._hardRollback(transformErr);
	}

Moreover JSON1 support for invertible operations and composition is known to not be without flaws.

Do you think we could introduce a configuration option to deactivate this functionality?

[alecgibson]

Quick hacky fix is to just:

delete types.json1.invert;

But obviously that's pretty ugly, and you may or may not want to use invert elsewhere.

In general, I'd expect if a type implements invert for the ops to be invertible. That being said, Pull Requests are always welcome!

[curran]

Is this a duplicate of #451?

[hsource]

I did a more targeted and backwards-compatible fix in #520 - Fall back to doing a _hardRollback when op.type.invert fails. Let me know if you have any feedback on that! We're running that in a patched production build already

[antoinelyset]

Hi! I Just saw this change.
For the record, it won't fix the issue with ot-json1 since invert exists and will not fail, it will just invert incorrectly see #443 (comment).

[alecgibson]

Noted. I think I'm in favour of deprecating "soft" rollback as default behaviour. I'm not sure invert() is particularly bullet-proof in any of the widely-used OT types.

[happy15]

One more possible case:

Say we have op1, op2, op3, op4, and all of op2 ~ op4 depend on the existence of op1 to be meaningful. Op1 is rejected by server and _rollback successful, which leaves op2 ~ op4 transformed against invert(op1). This results in transformed op2', op3', op4'.

My question is: will op2', op3', op4' still be meaningful? Ideally, op2 ~ op4 should be removed along with op1. In the current framework of sharedb, the best bet may be to make the transform result of op2~op4 against invert(op1) to be no-op.