automerge / automerge-classic

A JSON-like data structure (a CRDT) that can be modified concurrently by different users, and merged again automatically.
http://automerge.org/
MIT License
14.75k stars 466 forks source link

Load and save "type branding" makes it difficult to instantiate. #408

Closed JohnTheGray closed 3 years ago

JohnTheGray commented 3 years ago

The Typescript type BinaryDocument is used by Automerge.save() and Automerge.load(). As I understand it, when save is called, the resulting binary document can be stored to disk or in a DB. However, when this document is retrieved from storage, it needs to be converted into the BinaryDocument type. I do not see any easy way of doing this, unless I'm missing some piece of the puzzle?

My best effort, which seems a bit overkill, is:

class BinaryDocument extends Uint8Array {
  __binaryDocument: true

  constructor(val: Uint8Array) {
    super(val)
  }

  static from(buffer: Uint8Array): BinaryDocument {
    return new BinaryDocument(buffer)
  }
}

It seems that BinaryDocument needs some factory method to create it from a byte array?

ept commented 3 years ago

@JohnTheGray I'm not a TypeScript user so I'm not sure what's the most idiomatic, but I think you could just use a typecast (yourByteArray as BinaryDocument)?

JohnTheGray commented 3 years ago

@ept Same here, I'm just kicking off with TypeScript. Thanks, your suggestion works, although I didn't see this initially as I was working with Buffer. For example:

This doesn't work even though Buffer is a subclass of Uint8Array:

Type 'Buffer' is not comparable to type '{ __binaryDocument: true; }

const buffer = Buffer.from(dto.document, 'base64')
const document: BinaryDocument = Automerge.load(buffer as BinaryDocument)

However, then I tried casting buffer to Uint8Array and then to a BinaryDocument, then all was good.

const buffer = Buffer.from(dto.document, 'base64')
const document: BinaryDocument = Automerge.load(buffer as Uint8Array as BinaryDocument)

I don't think I quite understand how Uint8Array can be cast to BinaryDocument, but Buffer can't. In fact I can't understand the former either.

ept commented 3 years ago

Okay, good to know that works. We should probably document this, maybe simply as a comment in the type definitions file. Do you want to make a PR that adds such a comment?

@pvh: pinging you FYI since you introduced the branded types.

pvh commented 3 years ago

I'm out this week but I suspect there's something wonky with Buffers vs. UInt8Arrays that hasn't come up in my usage which we can improve here, yes.

I have also heard secondhand reports of similar confusion between buffers and UInt8Arrays between node and browser code. Not sure if those have percolated into a GitHub issue.

On Tue, Jun 15, 2021, 9:45 AM Martin Kleppmann @.***> wrote:

Okay, good to know that works. We should probably document this, maybe simply as a comment in the type definitions file. Do you want to make a PR that adds such a comment?

@pvh https://github.com/pvh: pinging you FYI since you introduced the branded types.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/automerge/automerge/issues/408#issuecomment-861660108, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAWQDF52R77UDZZOMJXJTTS57SHANCNFSM46WZWDJQ .