It would be nice if the sync protocol function `readSyncMessage` wrote the update to the encoder

[chanced]

readSyncMessage currently only writes the update to the encoder provided if it is a SyncStep1

export const readSyncMessage = (decoder, encoder, doc, transactionOrigin) => {
  const messageType = decoding.readVarUint(decoder)
  switch (messageType) {
    case messageYjsSyncStep1:
      readSyncStep1(decoder, encoder, doc)
      break
    case messageYjsSyncStep2:
      readSyncStep2(decoder, doc, transactionOrigin)
      break
    case messageYjsUpdate:
      readUpdate(decoder, doc, transactionOrigin)
      break
    default:
      throw new Error('Unknown message type')
  }
  return messageType
}

It would be nice if the decoded message were written to the encoder for SyncStep2 or Update.

[dmonad]

Hi @chanced, could you please elaborate what you what to achieve?

[chanced]

@dmonad sure. So readSyncMessage accepts an encoder. If the the sync message is a syncStep1, it writes the decoded syncStep2 to the encoder:

export const readSyncStep1 = (decoder, encoder, doc) =>
  writeSyncStep2(encoder, doc, decoding.readVarUint8Array(decoder))

If the message is either a syncStep2 or syncUpdate, it decodes the message and applies it to the document:

export const readSyncStep2 = (decoder, doc, transactionOrigin) => {
  try {
    Y.applyUpdate(doc, decoding.readVarUint8Array(decoder), transactionOrigin)
  } catch (error) {
    // This catches errors that are thrown by event handlers
    console.error('Caught error while handling a Yjs update', error)
  }
}

What I was suggesting is that regardless of the step, encode and apply the steps to the encoder as it currently does for syncStep1, except also apply those updates to the document.

The reason I'm suggesting this is that in order to both apply an update and encode it, you currently have the clone the message:

private applySyncUpdate(decoder: decoding.Decoder): Uint8Array | null {
    const encoder = encoding.createEncoder();
    encoding.writeVarUint(encoder, MessageType.Sync);
    const clone = decoding.clone(decoder);
    syncProtocol.readSyncMessage(decoder, encoder, this.doc, null);
    decoding.readVarUint(clone); // disgarding the first uint8
    const update = decoding.readVarUint8Array(clone);
    syncProtocol.writeUpdate(encoder, update);
    if (encoding.length(encoder) > 1) {
        return encoding.toUint8Array(encoder);
    }
    return null;
}

[chanced]

I just realized this would break your logic here:

https://github.com/yjs/y-websocket/blob/caa6304ba30c0e6d5921e735470f6470b045cb6a/bin/utils.js#L173-L175

      case messageSync:
        encoding.writeVarUint(encoder, messageSync)
        syncProtocol.readSyncMessage(decoder, encoder, doc, null)
        if (encoding.length(encoder) > 1) {
          send(doc, conn, encoding.toUint8Array(encoder))
        }
        break

I think I'll just skip using syncProtocol.readSyncUpdate and handle the logic on my end.

It solves a potential bug on my side where if a syncStep1 came through, I would replicate the encoding anyway.

Thanks.

[dmonad]

Got it. I highly recommend to listen to the update event instead of forwarding the update after decoding it. This solves that the same message is ping-ponged between client and server.

[chanced]

Ah, that makes sense. Thank you.