Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add modifyStorage api to Node #1515

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

add modifyStorage api to Node #1515

wants to merge 1 commit into from

Conversation

jrowny
Copy link
Contributor

@jrowny jrowny commented Mar 6, 2024

  • Adds a new modifyDocumentStorage method to node client

Requires backend modification

Use it like this:

import { Liveblocks } from "@liveblocks/node";

const liveblocks = new Liveblocks({ secret:  "sk_dev_.....",});

await liveblocks.modifyStorageDocument("roomId", (root) => {
  const obj = root.get("someObject");
  obj.set("99", 12345);
});

Root will be a fully hydrated LiveObject.

With types:

type MyRoomStorage =  {
    someObject: LiveObject<{
      [key: string]: number | LiveObject<{ a: number }>;
    }>
}
await liveblocks.modifyStorageDocument<MyRoomStorage>("roomId", (root) => {
  const obj = root.get("someObject");
  obj.set("99", 12345);
});

Copy link
Collaborator

@nvie nvie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conceptually, I love this! The flow is so easy to follow:

  • Download the full storage document
  • Mutate it
  • Discard the results
  • Todo: error if an error happened on the server

See my code review comments on the server side. I think we'll need to figure out how to best define the concept of a "server session". I think the current "si" prefix we use to initialize a room from the REST API should become just a special case of whatever server session abstraction we land on.

Amazing work! 🙌

Comment on lines +663 to +665
// We need a real actor ID...
generateId: () => `999:${clock++}`,
generateOpId: () => `999:${opClock++}`,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For server side room initialization, we use "si" as the prefix.

Comment on lines +652 to +673
const nodes = new Map<string, LiveNode>();
const result = await this.get(url`/v2/rooms/${roomId}/storage-stream`);
const items = (await result.json()) as IdTuple<SerializedCrdt>[];
let clock = 0;
let opClock = 0;
const storageOps: Op[] = [];
const pool: ManagedPool = {
roomId,
getNode: (id: string) => nodes.get(id),
addNode: (id: string, node: LiveNode) => void nodes.set(id, node),
deleteNode: (id: string) => void nodes.delete(id),
// We need a real actor ID...
generateId: () => `999:${clock++}`,
generateOpId: () => `999:${opClock++}`,
dispatch(ops: Op[]) {
storageOps.push(...ops);
},
assertStorageIsWritable: () => true,
};
// TODO: not exatly sure why this can't pull the types, may be beacuse I'm linked locally
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call
const root: LiveObject<T> = LiveObject._fromItems<T>(items, pool);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to expose a proper API for this right from the core package, to create a new client-side Storage context and initialize it from a stream of IdTuple<SerializedCrdt> values, rather than exposing the ManagedPool / CRDT internals here. But conceptually this is really great! 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants