ottypes / json0

Version 0 of the JSON OT type
447 stars 64 forks source link

Adding presence to ot-json0 #25

Closed houshuang closed 5 years ago

houshuang commented 5 years ago

In fact, that PR was submitted in error - I was trying to submit against a different upstream :) I thought I closed it, but I guess I didn't. Anyway we are absolutely happy to share this work, just not quite done yet (but feedback welcome!). Here's a quick demo video with Quill and presence/shared cursors. https://www.youtube.com/watch?v=CVjH56Q18cU

I'll probably not handle nested subtypes right now, but would love to see someone tackle that. I will however work on text0, and also a very simple approach to registering presence in a field (with no transformations). I guess it makes no sense to submit PR to this repo unless they merge the main presence PR to sharedb core... Unfortunately ot-json0 is hard-coded into sharedb, which I want to try to change, because we would prefer to use @teamwork fork (or main sharedb if presence gets merged), however right now that's impossible with a different ot-json0 implementation. (It complains "Wrong basic type" or something like that).

Also want to add tests. Need to see what already exists, and what I can add.

curran commented 5 years ago

@houshuang Thanks for your response!

It is indeed unfortunate that ot-json0 is hard-coded into ShareDB. My gut feeling is that ShareDB should be able to handle a different type as the base type for documents, but I've never tried it.

I'm leaning towards using the @Teamwork fork as well, mainly for its presence implementation.

I'm thrilled to hear you're interested in working on text0. I will be following this work closely and perhaps we can collaborate on it, as I'm keen to implement presence text0 strings that are deeply nested within json0 documents.

I stumbled on this, which may be interesting: Interesting Cursor Transform Work by Robert Lord #27

houshuang commented 5 years ago

Here's the code that errors out:

      if (types.map[message.type] !== types.defaultType) {
        err = new ShareDBError(4020, 'Invalid default type');
        return this.emit('error', err);

(lib/client/connection.js:198)

We can either remove that (I did, and it worked), or instead of this line in lib/types.js:

exports.defaultType = require('ot-json0').type;

instead have the user pass in an ot-json0 library as an option when instantiating ShareDB...

On Tue, Apr 9, 2019 at 3:08 PM Curran Kelleher notifications@github.com wrote:

@houshuang https://github.com/houshuang Thanks for your response!

It is indeed unfortunate that ot-json0 is hard-coded into ShareDB. My gut feeling is that ShareDB should be able to handle a different type as the base type for documents, but I've never tried it.

I'm leaning towards using the @Teamwork https://github.com/Teamwork fork as well, mainly for its presence implementation.

I'm thrilled to hear you're interested in working on text0. I will be following this work closely and perhaps we can collaborate on it, as I'm keen to implement presence text0 strings that are deeply nested within json0 documents.

I stumbled on this, which may be interesting: Interesting Cursor Transform Work by Robert Lord #27 https://github.com/ottypes/json0/pull/27

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ottypes/json0/pull/25#issuecomment-481244003, or mute the thread https://github.com/notifications/unsubscribe-auth/AADwh8hddK1tJb2mkpd_4-LhKs2_cY0rks5vfJDRgaJpZM4ccpLb .

-- http://reganmian.net/blog -- Random Stuff that Matters

curran commented 5 years ago

I started continuing this work on presence over here https://github.com/datavis-tech/json0/pull/2

curran commented 5 years ago

@houshuang That error might make a good issue on the ShareDB repo. My gut feeling is that, in principle, you should be able to set the default OT type.

houshuang commented 5 years ago

Superseded by #31