search

ottypes / docs

277 stars 29 forks source link

change return type of transform #32

Closed yiminghe closed 3 years ago

yiminghe commented 3 years ago

for example: when transform a move operation of tree, you need to return a array of operation to break the loop situation:

https://github.com/yiminghe/ot-tree/blob/8ea07b973972a5a479d21dff313bfc81ea779074/src/index.ts#L71

https://github.com/yiminghe/ot-tree/blob/8ea07b973972a5a479d21dff313bfc81ea779074/src/index.ts#L183

josephg commented 3 years ago

What you’re calling an op is what the other libraries describe as an operation component. An operation for the other libraries can contain many changes. Maybe Op for you should be TreeOp | TreeOp[]

yiminghe commented 3 years ago

What you’re calling an op is what the other libraries describe as an operation component. An operation for the other libraries can contain many changes. Maybe Op for you should be TreeOp | TreeOp[]

If so the transform function should be

transform(op:TreeOp | TreeOp[], otherOp:TreeOp | TreeOp[]): TreeOp | TreeOp[]{
}

I need to do a matrix transform(many to many) inside transform function, but I think this kind of matrix transform should be inside ot control algorithm(sharedb?), tranform function only need to deal with one to one transforming.

josephg commented 3 years ago

I need to do a matrix transform(many to many) inside transform function

Yes, but that transformation function is only a few lines of code. It should look something like this.

I think this kind of matrix transform should be inside ot control algorithm(sharedb?), tranform function only need to deal with one to one transforming.

That perspective makes sense. Some OT types (eg ot-text-unicode, json1) do not need this matrix transform function at all. Because it is only needed in some cases, I'm happier if that code is the responsibility of the OT type rather than the control system (like sharedb).

Maybe if you want you could pull the transform list vs list code into a separate library in NPM for reuse instead.

If we change this API now, there are many projects which would need to be updated. My attention at the moment is on future software which I hope will use CRDTs instead. It is too late in the lifecycle of ottypes to make this breaking API change.

yiminghe commented 3 years ago

Yes, but that transformation function is only a few lines of code. It should look something like this.

In tree situation, transform list function will be recursive:

https://github.com/yiminghe/ot-tree/blob/18bb9afef67d2a9adca9fb780d233a95af9c9ac8/src/utils.ts#L390

If we change this API now, there are many projects which would need to be updated.

This change is compatible.

My attention at the moment is on future software which I hope will use CRDTs instead.

I am relatively new to OT area, but I think ot is more practical compared to CRDT.

Summary

I will make TreeOp a list type: https://github.com/yiminghe/ot-tree/commit/18bb9afef67d2a9adca9fb780d233a95af9c9ac8 and close this pr. Thanks for your reply 👍

josephg commented 3 years ago

Oh ok - it looks like your ot-tree library is similar in many ways to ot-json1. Good luck with it!