ottypes/docs spec is not supported correctly wrt (de)serialisation

[gkubisa]

Spec: https://github.com/ottypes/docs#standard-properties

First of all, according to the spec, initialData, snapshot and data may all be completely different data types in the following function specs create([initialData]) -> snapshot, serialize(snapshot) -> data, deserialize(data) -> snapshot. text-tp2 is an example of such a type. Please note also that create([initialData]) -> snapshot returns a non-serialized snapshot.

Secondly, apply(snapshot, op) -> snapshot' expects a non-serialized snapshot as the first parameter. Again, text-tp2 is an example of a type which relies on the first param being a non-serialized snapshot.

I found a number of problems in ShareDB related to the above:

1. ShareDB applies an operation to initialData, instead of a snapshot, in tryCompose. I have already opened a PR with a fix.
2. ShareDB passes a non-serialized snapshot to deserialize here.
3. ShareDB uses createDeserialized, which is not in the spec and is not needed because the standard create already returns a non-serialized snapshot.
4. ShareDB never calls serialize and relies on the OT types to define toJSON instead. This is not documented anywhere and violates the spec.
5. ShareDB applies operations to serialized snapshots on the server-side because it does not deserialize the data coming from the database.

[DetweilerRyan]

The contract for custom (de)serialization of ot types in ShareDB differs from the spec you reference. This was done for good reason and it is "documented" here #108.

I'm always glad to hear of new use cases for custom ot types. At Retrium we have been using custom ot types with custom serialization for about three years in production and I assure you the current implementation in sharedb works without fail.

[gkubisa]

The contract for custom (de)serialization of ot types in ShareDB differs from the spec you reference.

From reading ShareDB source code, I see the differences affect the following functions: create, createDeserialized, apply and serialize. The biggest issue for me is that those differences are not documented clearly and in a place where users would actually find out about them.

This was done for good reason and it is "documented" here #108.

As I understand, the reason is performance, which is a good reason indeed. :-) However, couldn't the same benefits be achieved while maintaining compatibility with ottypes/docs? For example, add a new optional function to the spec: applySerialized(serializedSnapshot, op) -> serializedSnapshot' and use it only on the server as follows:

• if applySerialized is available, call applySerialized. (fast for types with custom (de)serialization)
• else if serialize and deserialize are available, call deserialize -> apply -> serialize. (MAY be slow for types with custom (de)serialization but compatible with ottypes/docs)
• else, call apply. (fast for types without custom serialization)

I assure you the current implementation in sharedb works without fail.

I never questioned that it works. I just pointed out that it implements a different spec than the one referenced in the readme file.

IMHO, the written ottypes/docs spec is clearer and simpler than the implied spec implemented by ShareDB, so I'd still argue for maintaining compatibility with ottypes/docs.