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

Simplify liveblocks.config.ts generics for the common case #1493

Open
GuillaumeSalles opened this issue Feb 26, 2024 · 1 comment
Open

Simplify liveblocks.config.ts generics for the common case #1493

GuillaumeSalles opened this issue Feb 26, 2024 · 1 comment
Assignees

Comments

@GuillaumeSalles
Copy link
Contributor

Context

  • We receive a lot of support requests about how to type our Presence, Storage, UserInfo, RoomEvent and ThreadMetadata.
  • If you need to specify only one generic that is not Presence, the DX is awkward. createRoomContext<{}, never, never, never, { resolved: boolean }>. This used to be okay because Presence and Storage were the core of Liveblocks. With new developers coming to our platform for Comments or Y.js, this happens more and more frequently.
  • We introduced the factory pattern (createRoomContext) to let developers use different types per room.
  • The relationship between our hooks and generics is not always obvious. For example useBroadcastEvent ( https://github.com/liveblocks/liveblocks.io/issues/1984)

Proposal

I'm still not entirely convinced this is a good idea, but I couldn't come up with something better yet.

Support type overrides with Module Augmentation. Inspired by Slate. The goal would be to support the factory for the advanced use cases but recommend module augmentation as the default in our docs and examples.

Before

// liveblocks.config.ts

type ThreadMetadata = {
   resolved: boolean
}

const client = createClient({ /* ...  */ });

export { } = createRoomContext<{}, never, never, never, ThreadMetadata>(client);

After

// liveblocks.config.ts

module `@liveblocks/client` {
  type ThreadMetadata = {
     resolved: boolean
  }
}

const client = createClient({ /* ...  */ });

export { } = createRoomContext(client);

Other benefits:

  • It also works for users that do not use @liveblocks/react. No need to pass generics to enterRoom. It would simplify our docs quite a bit I think.
  • We might also re-export our @liveblocks/react hooks directly instead of exporting them from the config? It's starting to make more sense as we're re-introducing LiveblocksProvider. We could provide a code mod for an upgrade? 🤔
@GuillaumeSalles GuillaumeSalles added the feature request Feature requested by the community label Feb 26, 2024
@nvie
Copy link
Collaborator

nvie commented Feb 27, 2024

This is an interesting idea. Would definitely make things simpler, at the cost of only supporting one room "type" globally for your app. If you want to support multiple room types, you still have the option to stop using module augmentation like this, and switch back to the 5-type-params-form for each room types. So easy for the common case, but an escape hatch for the less-common case.

There are a couple questions we should figure out about this:

  • What is the DX going to be like if you don't augment a mandatory type (e.g. you don't specify a Presence type)?
  • What is the DX going to be like if you augment a type in an invalid way (e.g. you extend your Storage type with non-LSON data, or you specify a UserMeta shape that isn't compatible)
  • What kind of errors will that produce and will they be helpful to pin-point the problem?
  • Where will those errors be happening? Those TS errors should be happening inside the user's code base, not in Liveblocks library code (which to a developer lives inside node_modules and many projects have skipLibCheck enabled).
  • Does "jump to definition" still work as expected if you want to look up a specific inferred type from a random place in the code base?

@adigau adigau added the public repo label Apr 15, 2024 — with Linear
@adigau adigau removed feature request Feature requested by the community public repo labels Apr 15, 2024
@adigau adigau added the 🎪 Room label Apr 24, 2024 — with Linear
@adigau adigau added the Engineering label May 9, 2024 — with Linear
@adigau adigau removed the Engineering label May 9, 2024
@adigau adigau removed the 🎪 Room label May 9, 2024
@nvie nvie self-assigned this May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants