Thanks for the amazing works.

I've just started experimenting with yorkie and stumbled upon an issue. I noticed that we can display peers accessing a document in realtime (as found in this example). However, it is limited to get the peer ids (the id which is returned by the agent on client activation).

Is there a way to get more details (perhaps the key used on activation)?

My use case is that I'm trying to build a real time text editor, similar with Google Docs. I would like to display all user names accessing the document (not just the ids). I'm thinking of putting the client's key with the user id, so that I could retrieve more details (e.g. username) later in the other connected peer sessions. However, I could not find out how to do that. Maybe I'm missing something here.

During the code development process, I checked the following errors.

It occurred because of the code below.

catch(err){
  logger.error(`[AC] c:"${this.getKey()}" err :"${err}"`);
}

I am wondering whether it is better to be able to receive everything even an object, or whether it is better to pass an only string as the argument.

If the logger receives all values, it will proceed as follows.

  info: (...messages: unknown[]): void => {
    if (level > LogLevel.Info) {
      return;
    }

    console.log('YORKIE I: ', ...messages);
  },

Yorkie supports seven primitive types: boolean, integer, long, double, string, byte array, and date. For now, We implemented only three types: boolean, integer and string in JS SDK.

When implementing the rest of the types, it would be nice to refer to Yorkie's primitive.

relevant test:
https://github.com/yorkie-team/yorkie-js-sdk/blob/master/test/yorkie_test.ts#L159-L175

value to bytes:
https://github.com/yorkie-team/yorkie/blob/master/pkg/document/json/primitive.go#L128-L158

bytes to value:
https://github.com/yorkie-team/yorkie/blob/master/pkg/document/json/primitive.go#L42-L66

What happened:

Cannot pass test code

Yorkie Can handle concurrent moveBefore operations FAILED
        AssertionError: expected '{"k1":[1,2,0]}' to equal '{"k1":[2,1,0]}'

What you expected to happen:

PASSED

How to reproduce it (as minimally and precisely as possible):
When I edit test code like below

      d1.update((root) => {
        const next = root['k1'].getElementByIndex(0);
        const item = root['k1'].getElementByIndex(2);
        root['k1'].moveBefore(next.getID(), item.getID());
        assert.equal('{"k1":[2,0,1]}', root.toJSON());
      });
      await c1.sync();
      d1.update((root) => {
        const next = root['k1'].getElementByIndex(0);
        const item = root['k1'].getElementByIndex(2);
        root['k1'].moveBefore(next.getID(), item.getID());
        assert.equal('{"k1":[1,2,0]}', root.toJSON());
      });

from

Anything else we need to know?:

I am researching moveAfter functions and I cannot found the purpose of some codes.

When I was debugging some codes,
prevNode from const prevNode = this.findByCreatedAt(prevCreatedAt, value.getCreatedAt()); was not same with as I expected.

I cannot catch the purpose of this code.

Environment:

• Operating system: (Mac OS 10.15.6)
• Browser and version: Chrome Headless 85.0.4183.83
• Yorkie version (use yorkie version): 0.0.12
• Yorkie JS SDK version: 0.0.9

Not long ago we introduced the Garbage Collection feature in Yorkie.

• About Garbage Collection in Yorkie: yorkie-team/yorkie#3

And we implemented Document.GarbageCollect in Go.

• Document.GarbageCollect: yorkie-team/yorkie#55

We also need to implement Document.GarbageCollect function in JS SDK. Then we need to add logic to execute GarbageCollect by receiving the MinSyncedTicket in PushPull's response.

• https://github.com/yorkie-team/yorkie/blob/0597a6275ede944012fc7d19cb59f4bf8b4d2996/pkg/document/document.go#L135
• https://github.com/yorkie-team/yorkie-js-sdk/blob/master/src/document/document.ts#L141

I found the modified code without prettier applied in some files during code modification.
And it can include changes that are independent of the commit.
I sent a pull request for prevent this. (#52)

If the contributor modifies the code without using prettier, this problem will occur again.
So how about recommending the use of 'prettier' in contribution guide?

We need to add reference docs so that users can use the JS SDK in more detail. It would be nice to take a look at TypeDoc, which has been introduced by other TypeScript based libraries. And we'll also need to add TSDoc comments as well.

• Firebase JS SDK

Also, it would be nice to think about how to integrate it naturally in yorkie.dev.

I am currently implementing increase operation. And there was a problem during implementation.
This problem occurred because the ObjectProxy get the value property of JSONPrimitive.
https://github.com/yorkie-team/yorkie-js-sdk/blob/master/src/document/proxy/proxy.ts#L39

In my opinion, like the go version, ObjectProxy needs to get a JSONPrimitive instance to use a function like increase.
And this seems to be consistent with the go version.

Description:
If you check yorkie_pb.d.ts after building protobuf, the editor warns you.

Array can be declared generic in actual typescript,
In yorkie_pb.d.ts, Array is a phenomenon that occurs because there is a conflict because Array class exists after protobuf build.

Is it safe to use message names that can conflict with the data types provided by the language?

Why:
Assuming you add yorkie-java-sdk, I wonder if it will be safe after the build result. This is because Object and Object messages are expected to collide in Java. I think this is worth testing.

What happened

While running the toy example (docker-compose up => npm start) I noticed that if I restart the frontend server, synchronization happens only after a client makes some local changes to the document. So for example if I make changes on a client, the changes are not reflected in any other clients until they create some events on their part (e.g., moving the cursor around, writing to the document et cetera)

What you expected to happen: Seamless synchronization

How to reproduce it (as minimally and precisely as possible):

1. Run docker-compose up in docker/
2. Run npm start
3. Quit the frontend server with a keyboard interrupt (Ctrl + C)
4. Again run npm start
5. Make any changes to a client document and notice that the change is not going to propagate to other clients until they do something.

Anything else we need to know?:
When I quit the frontend server, it does show an error: Assertion failed: (0), function uv_close, file ../deps/uv/src/unix/core.c, line 178.

Then after restarting the server, it produces this error whenever I close a client browser tab: [HPM] Error occurred while trying to proxy request /api.Yorkie/WatchDocuments from localhost:9000 to http://localhost:8080 (ECONNRESET) (https://nodejs.org/api/errors.html#errors_common_system_errors)

Environment:

• Operating system: macOS Catalina 10.15.6
• Browser and version: Chrome, Version 84.0.4147.105 (Official Build) (64-bit)
• Yorkie version (use yorkie version): Yorkie: 0.0.10 (Commit: 165cdbf / Go: go1.14.3)
• Yorkie JS SDK version: 0.0.9

updatedAt is used to indicate that the relative position of the element has changed by move operation in the array, not the value change. Therefore, updatedAt has changed to movedAt in Yorkie and should be reflected in yorkie-js-sdk as well.

yorkie-team/yorkie#70

A script error occurs when a local update comes in while applying remote changes. JavaScript has no multi-routines or mutex, so We should think about a solution that is different from the traditional approach.

Suggestion

Currently, several tests rely on setTimeout to wait for changes to be applied.
However, this is not a good practice as it is nondeterministic and could lead to different results (for example, poor network condition can lead to sync occurring later than the time set by the setTimeout).

Therefore, I suggest that we remove setTimeout and make it event driven.

What happened:
As the title suggests, I've tried to attach to a document that has been previously detached.
This emits the following error message: "document already attached"

What you expected to happen:
I expected the client to reattach to the document with no problem.

How to reproduce it (as minimally and precisely as possible):

1. Create a client
2. Attach it to a document
3. Detach it from the document
4. Attach it to the document again

To reproduce it with minimal efforts, go to test/yorkie_test.ts, then insert the following lines after withTwoClientsAndDocuments:L57 (after detaching from the document)

  await client1.attach(doc1, true);
  await client2.attach(doc2, true);
  await client1.detach(doc1);
  await client2.detach(doc2);

Description:

Not long ago, when we introduced strict mode, we found out that we use null and undefined together. We need to use undefined, not null.

// tsconfig.json
...
  "compilerOptions": {
    ...
    "strictNullChecks": false, // We need to remove this line
  }
...

https://github.com/yorkie-team/yorkie-js-sdk/blob/main/tsconfig.json#L10

Why:

Since grpc-web uses undefined, it is better for us to use undefined rather than null to keep the code consistent. And projects are written in TypeScript also tend to use undefined.

What happened:
The attribute option is not applied for richText

it('should handle edit operations', function () {
    const doc = Document.create('test-col', 'test-doc');
    assert.equal('{}', doc.toSortedJSON());

    //           -- ins links ---
    //           |              |
    // [init] - [ABC] - [\n] - [D]
    doc.update((root) => {
      const text = root.createRichText('k1');
      text.edit(0, 0, 'ABCD', { b: '1' }); // <-- bold attr
      text.edit(3, 3, '\n');
    }, 'set {"k1":"ABC\nD"}');

    doc.update((root) => {
      assert.equal(
        '[0:00:0:0 ][1:00:2:0 ABC][1:00:3:0 \n][1:00:2:3 D][1:00:1:0 \n]',
        root['k1'].getAnnotatedString(),
      );
    });

    assert.equal(
      '{"k1":[{"attrs":{},"content":ABC},{"attrs":{},"content":\n},{"attrs":{},"content":D},{"attrs":{},"content":\n}]}',
      doc.toSortedJSON(),
    ); // This test should not passe, but it is pass now
  });

What you expected to happen:
The attribute should be applid for richText

How to reproduce it (as minimally and precisely as possible):

1. open document_test.ts
2. find "should handle edit operations" test
3. change to text.edit(0, 0, 'ABCD', { b: '1' });
4. run test

Anything else we need to know?:

Environment:

• Operating system:
• Browser and version:
• Yorkie version (use yorkie version):
• Yorkie JS SDK version:

After several typing, it looks like data is twisted at some point.

https://yorkie.dev/demo

What happened:

After twisting, Quill does not work when changing the style of the entire text. ex) H1, H2, list order

What you expected to happen:

In the picture, Fail in quill lib,
The data looks like remote 35-35 {"list":"ordered"} but It should come remote 35-36 {"list":"ordered"}. retain: 1 is missing.

How to reproduce it (as minimally and precisely as possible):
This occurs intermittently when constantly changing style and text.

Anything else we need to know?:

Environment:
Now yorkie.dev environment

When we receive changes from remote, the changes is applied to the document in Yorkie. However, when we use an external model like CodeMirror example, we need to also apply the changes to the external model.

To handle this, we need an interface that can notify the outside of the SDK.

I found insertAfter and moveBefore at https://github.com/yorkie-team/yorkie-js-sdk/blob/master/src/document/proxy/array_proxy.ts

But I can't figure out how to put an item in front of an array. Do I have to get the first item and use it as an argument of moveBofore?

What happened:
The result of the two replicas is not the same.

What you expected to happen:
The result of the two replicas should be the same

How to reproduce it (as minimally and precisely as possible):

1. Set two browsers with network speed as Fast 3g and Slow 3g
2. Open code mirror examples in Yorkie.dev
3. Select each piece including overlapping blocks
4. Edit first from the slower browser and the faster browser

Anything else we need to know?:

Environment:

• Operating system: OS X
• Browser and version: Chrome 86.0.4240.198
• Yorkie version (use yorkie version): public yorkie dev server version
• Yorkie JS SDK version: public yorkie dev server version

For a consistent code style, it usually used eslint + prettier

What happened:

If there are more than 6 peers, there is no response from yorkie

What you expected to happen:

If there are more than 6 people, it should be possible to connect.

How to reproduce it (as minimally and precisely as possible):

If more than 6 people are connected, there is no response afterward, and if the connected peer goes out, the connection is possible from the next time.

Anything else we need to know?:

The request was made by yorkie-sdk-js, but the request was not received by yorkie.
It may be an envoy configuration issue

Environment:

• Operating system: macOS Catalina 10.15.5
• Browser and version: Chrome 88.0.4324.96
• Yorkie version (use yorkie version): branch master commit f9b2bb6658e70f7ba52844e26c6e2c074a172bc2
• Yorkie JS SDK version: branch main 0bf9127
  yorkie (Most recent, unreleased)

When developing new applications, users can use Yorkie's document directly as a model. However, applications that have already been developed have their own models. So We need to find a way to work with the model of an application that has already been developed.

Currently, Text provides a change handler for integration with CodeMirror.
https://github.com/yorkie-team/yorkie-js-sdk/blob/master/dist/index.html#L155-L176

And Document provides a change event stream.
https://github.com/yorkie-team/yorkie-js-sdk/blob/master/dist/drawing.html#L66

webpack/webpack#10518

We currently only support Quill editor fonts and paragraph styles, not media.

To support images and videos, we need to handle JSON as well as text in the insert command.

// Insert a bolded "Text"
{ insert: "Text", attributes: { bold: true } }

// Insert a link
{ insert: "Google", attributes: { link: 'https://www.google.com' } }

// Insert an embed
{
  insert: { image: 'https://octodex.github.com/images/labtocat.png' },
  attributes: { alt: "Lab Octocat" }
}

// Insert another embed
{
  insert: { video: 'https://www.youtube.com/watch?v=dMH0bHeiRNg' },
  attributes: {
    width: 420,
    height: 315
  }
}

Perhaps if we don't support partial modification of the insert's value, we can serialize the JSON and treat it as a string.

https://github.com/quilljs/delta#insert-operation

While working on the CodePair example, I noticed that many errors occur in Typescript strict mode. We can change it to the strict mode by adding a property to tsconfig.json.

// tsconfig.json
{
  "compilerOptions": {
    "sourceMap": true,
    "target": "esnext",
    "removeComments": false,
    "allowSyntheticDefaultImports": true,
    "strict": true // <-- this line to check strict at compile time
  },
  "include": ["src/**/*"],
  "exclude": ["node_modules"],
  "typeRoots": ["node_modules/@types"]
}

I think that checking in strict mode will further reduce the possibility of bugs.

What happened:

I am working on issue #109. I worked so that ClientEventType can be exported from yorkie.ts.
And I tested at codepair.

The result was that the imported ClientEventType was undefined.
I felt weird and also checked the RichText class that was originally exporting. This is also undefined.

// CodeEditor.ts of codepair

...
import { ClientEventType, RichText } from 'yorkie-js-sdk';

console.log(RichText);
console.log(ClientEventType);
...

What you expected to happen:
I expected the information of ClientEventType to be displayed on the console.

How to reproduce it (as minimally and precisely as possible):
I pushed to the export-event-type-undefined branch so I could run this test right away.

1. Import and export ClientEventType from yorkie.ts.

// yorkie.ts
...
import { Client, ClientOptions, ClientEventType } from './core/client';
import { Document, DocEventType } from './document/document';

export { Client, Document, ClientEventType, DocEventType };
...

2. Install the working branch from the codepair project.

// package.json of codepair

"dependencies": {
  ...
  "yorkie-js-sdk": "yorkie-team/yorkie-js-sdk#export-event-type-undefined",
  ...
}

3. Add output code by importing ClientEventType from CodeEditor.tsx of codepair. And run.

// CodeEditor.tsx of codepair

...
import { ClientEventType } from 'yorkie-js-sdk';
console.log(ClientEventType);
...

Anything else we need to know?:
I did some tests to see how to solve this problem. And I actually solved it.

You can see what the export-event-type branch did.

The solution is not to build using webpack, but build using tsc. When building with tsc, js files for each package are created instead of a single js file.

tsc build :::

webpack build :::

And this really worked.

after build using tsc :::

I don't know exactly why this result was. We should consider using tsc if the library built with webpack doesn't improve to import other objects.

Environment:

• Operating system: macOS Big Sur 11.0.1
• Browser and version:
• Yorkie version (use yorkie version): 0.1.1
• Yorkie JS SDK version: 0.1.1

Currently, In Yorkie project, we enforce comments for important functions and added comments. yorkie-team/yorkie#114
In my view, it would be great to also apply TSLint completed-docs rule for writing comments in yorkie-js-sdk

https://circleci.com/docs/2.0/code-coverage/#codecov

Support network auto recovery

• Reconnect stream when network is restored
• Flush local changes after applying them to the server

We need to check the logic for drawing the selection.

yorkie-team/yorkie-team.github.io#1 (comment)

client, document provided by yorkie currently showing d.ts information.

yorkie.d.ts

// client 
import { Client, ClientOptions } from './core/client';
// document
import { Document } from './document/document';

During the PR, I felt that the values below would be needed.

// client.d.ts
export declare enum ClientStatus {
    Deactivated = "deactivated",
    Activated = "activated"
}
export declare enum StreamConnectionStatus {
    Connected = "connected",
    Disconnected = "disconnected"
}
export declare enum DocumentSyncResultType {
    Synced = "synced",
    SyncFailed = "sync-failed"
}
export declare enum ClientEventType {
    StatusChanged = "status-changed",
    DocumentsChanged = "documents-changed",
    DocumentsWatchingPeerChanged = "documents-watching-peer-changed",
    StreamConnectionStatusChanged = "stream-connection-status-changed",
    DocumentSyncResult = "document-sync-result"
}

// document.d.ts 
export declare enum DocEventType {
    Snapshot = "snapshot",
    LocalChange = "local-change",
    RemoteChange = "remote-change"
}

Description:
#133 (comment)
From the above PR comment, we found out that depending on the version of protobuf, the behavior may not match our intention.
So we think we need to figure out the proper protobuf version and write it to README.md.

Why:
It can reduce unnecessary confusion for developers.

Is there any way to use RichText or Markdown?
If not, how about supporting RichText or Markdown?

When I import Yorkie JS SDK from CodePair created with create-react-app with TypeScript enabled, I confirmed that it does not work properly.

./node_modules/yorkie-js-sdk/src/yorkie.ts 24:33
Module parse failed: Unexpected token (24:33)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
| //  e.g) yorkie.createClient(...)
| export default {
>   createClient: function (rpcAddr: string, opts?: ClientOptions): Client {
|     return new Client(rpcAddr, opts);
|   },

We need to modify the deployment script of JS SDK by following TypeScript's publishing guide.

Suggestion

Google Docs shows that the document is either 'Saved to Drive' or is 'Working offline'. Latter is shown above.

Likewise, it would be useful for our users as well to know whether the document is being synced in real time, or is experiencing some sort of errors.

Thus @hackerwins suggested that we add a status icon in our examples.

What happened:
Fail 'Can watch documents' test.

HeadlessChrome 84.0.4147 (Mac OS X 10.15.6) Yorkie Can watch documents FAILED
	AssertionError: expected '{}' to equal '{"k1":"v1"}'
	    at Context.<anonymous> (test/webpack:/test/yorkie_test.ts:163:12)

What you expected to happen:

How to reproduce it (as minimally and precisely as possible):
Run script npm run test or npm run test:watch

Anything else we need to know?:

Environment:

• Operating system: Mac OS X 10.15.6
• Browser and version: Chrome 84.0.4147
• Yorkie version (use yorkie version): 0.0.10
• Yorkie JS SDK version: 0.0.9

We introduce JavaScript Proxy to allow users to use yorkie-js-sdk's documents like plain objects. At that time, I didn't know the proxy and Vue.js works together.

Recently @lqez added a kanban board example using Vue.js and yorkie-js-sdk. And It is really amazing that yorkie-js-sdk's document and vue.js work together in a few lines.

https://github.com/yorkie-team/yorkie-team.github.io/blob/master/static/js/demo-kanban.js#L101-L106
https://yorkie.dev/demo

We need to know how to pass the remote changes to Vue.js efficiently when some of the fields changed in the document.

We separated the tests according to their scope of responsibility through the yorkie-team/yorkie#118 issue. I think yorkie-js-sdk also needs to improve the test structure according to yorkie.

This code is an infinite loop.

So PushPull do action continuesly.

If the application has its own model, Yorkie will provide a change handler that can synchronize with the application's model in the document.

On the other hand, we decided to find a way to use Yorkie directly as a model for the application.

First, Yorkie provides a native interface using Javascript Proxy like Immer, so we will find a way how to interact with the React framework in a similar way to Immer.

https://github.com/immerjs/immer

Description

We implemented GC of the text type datatype in yorkie-team/yorkie#58. We also need to implement GC in Text and RichText implemented in JS SDK.

Why?

Implementing GC on the data type used by Agent, we can reduce the size of the snapshot stored in the DB. However, if we do not implement it in the SDK, the garbages of the replica document are not collected in the Client.

How do you think about creating the setter in ArrayProxy?
It will be good to use or not?

I accidentally added npm-check-updates and committed. It must be removed.
https://github.com/yorkie-team/yorkie-js-sdk/blob/master/package.json#L55

Question

Currently, if there's a network error, client.activate() will simply reject and finish. This means that even when the network comes back up, it's not going to attempt to activate the client anymore. Therefore, the user would have to do the following in order for everything to work.

1. Wait until the network comes back up, and then
2. Explicitly refresh the page (or activate the client again somehow)

But in case of 1., the network failure might be on our part (e.g., envoy went down) which the user wouldn't know of. In that case, the client would have to blindly refresh the page until it works.

Therefore I'm wondering if we should attempt to activate the client in a loop as in runSyncLoop and runWatchLoop, and never reject the promise.

I think this question applies to all methods that send a gRPC request (which include .activate(), .deactivate(), .attach() and .detach())