Improve .initializeStorageDocument() API
#1736
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Ignored Deployments
|
| roomId: string, | ||
| document: PlainLsonObject | ||
| document: LiveObject<S> | PlainLsonObject | ||
| ): Promise<PlainLsonObject> { |
There was a problem hiding this comment.
Actual question: I just noticed we're returning the document in plain LSON format here. This format is of course technically needed to get the Live objects over the wire, but I don't think in actual apps it's a nice format to work with. Should we not lift the level of abstraction a bit higher here, and either return:
void(so people can use.getStorageDocument(..., "json" /* their preferred format */))LiveObjectinstance- Simplified JSON (i.e. the equivalent of calling
storage.toImmutable())
All of those three seem better than returning a PlainLsonObject.
@CTNicholas @nimeshnayaju What do you think?
There was a problem hiding this comment.
Right! I think LiveObject is not particularly useful on the back end, so I'd probably skip that. I think void or JSON makes the most sense to me. However, bear in mind that getStorageDocument returns LSON by default, and we want our APIs to be consistent.
There was a problem hiding this comment.
Agreed, so the consistency argument rules out option 3, too. So… should we make this void instead? That, however, would be a breaking change, so maybe not change anything after all?
Co-authored-by: Marc Bouchenoire <[email protected]>
|
Nice! This would definitely be easier to use. I'm assuming Why not just have a plain object at the root of the argument? Since it's always a await client.initializeStorageDocument("my-room", {
layers: new LiveMap(),
layerIds: new LiveList([]),
}); |
Yeah, they do, until you start "using" them, i.e. start calling
We could do that too, can update 👍 |
|
Closing all PRs that have been opened for more than a year. |
This now takes a
LiveObjectinstance directly, so that users no longer have to bother with knowing about or callingtoPlainLson()manually.Creating a new room from the backend is now as easy as:
Fixes LB-884.