dmonad / lib0

Monorepo of isomorphic utility functions
MIT License
363 stars 65 forks source link

IntDiffOptRleEncoder's toUint8Array returns a different result each time. #60

Closed ObuchiYuki closed 11 months ago

ObuchiYuki commented 1 year ago

Describe the bug After writing the content to IntDiffOptRleEncoder (also UintOptRleEncoder, IncUintOptRleEncoder), when reading the content with toUint8Array(), the content is different each time due to flush.

Expected behavior Since toUint8Array() is obviously a function for reading values, its contents should not change with each call.

To fix IntDiffOptRleEncoder should have a flag like mutated. In that case, the implementation would be as follows

export class IntDiffOptRleEncoder {
  constructor () {
    this.encoder = new Encoder()
    /**
     * @type {number}
     */
    this.s = 0
    this.count = 0
    this.diff = 0
    this.mutated = false
  }

  /**
   * @param {number} v
   */
  write (v) {
    this.mutated = true
    if (this.diff === v - this.s) {
      this.s = v
      this.count++
    } else {
      flushIntDiffOptRleEncoder(this)
      this.count = 1
      this.diff = v - this.s
      this.s = v
    }
  }

  toUint8Array () {
    if (this.mutated) {
      flushIntDiffOptRleEncoder(this)
      this.mutated = false
    }
    return toUint8Array(this.encoder)
  }
}

Appendices In yjs, there is no part of IntDiffOptRleEncoder that calls toUint8Array() twice. I also confirmed that IntDiffOptRleEncoder with this modification passes all tests in yjs.

(I am personally working on yjs for Swift yswift. I discovered this in debugging).

dmonad commented 1 year ago

Hi @ObuchiYuki,

I should add more documentation on this. The idea is that you call toUint8Array only once to serialize the data. Serializing it several times is not supposed to work. It's also not supported by the other RLE encoders - to decrease complexity. Instead, you could create a new RLE encoder if you want to encode new data.

What's the use-case for calling toUint8Array twice?

FYI: We are also working on a Swift binding based on uniffi: https://github.com/y-crdt/y-uniffi