Skip to content

[lexical-yjs][lexical-playground] Breaking change: remove clientID from collab context#7816

Merged
etrepum merged 1 commit intofacebook:mainfrom
james-atticus:remove-client-id-from-collab-context
Sep 18, 2025
Merged

[lexical-yjs][lexical-playground] Breaking change: remove clientID from collab context#7816
etrepum merged 1 commit intofacebook:mainfrom
james-atticus:remove-client-id-from-collab-context

Conversation

@james-atticus
Copy link
Copy Markdown
Contributor

Description

Removes clientID from collaboration context. This ID was unreliable as it could be updated by nested collab editors. Consumers should retrieve the client ID by looking up the correct document in the context's yjsDocMap, or switch to using a different ID.

Also updated PollComponent to use name rather than client ID, so two clients with the same name will update each other's vote, which arguably is more correct.

@vercel
Copy link
Copy Markdown

vercel bot commented Sep 15, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
lexical Ready Ready Preview Comment Sep 15, 2025 6:57am
lexical-playground Ready Ready Preview Comment Sep 15, 2025 6:57am

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 15, 2025
Copy link
Copy Markdown
Collaborator

@etrepum etrepum left a comment

Choose a reason for hiding this comment

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

It seems like there should be some sort of context where you get access to the binding (and if it's changed), but I don't see one. I wonder if there is some sort of need to get the clientID?

I looked a little closer and was horrified to see that CollaborationContext is just a global variable in a React Context disguise, so none of the changes to it are observable and it's not safe for multiple collaboration instances. You can tell because CollaborationContext.Provider is never used!

@james-atticus
Copy link
Copy Markdown
Contributor Author

@etrepum I don't think we can do that, at least with this context provider. Nested editors use the same collab context (because they need the same yjsDocMap) but create their own binding.

If someone needs the clientID, they can get it from yjsDocMap.get(id).clientID. Well, in theory... because of #2637, #6271 and strict mode, the doc gets removed from the map in dev. I'll fix that in v2 but I don't see a good way to fix it in v1 without a big breaking change.

@etrepum
Copy link
Copy Markdown
Collaborator

etrepum commented Sep 18, 2025

I think this seems reasonable but we should probably get some feedback from a major consumer of collab. @ivailop7?

Copy link
Copy Markdown
Collaborator

@ivailop7 ivailop7 left a comment

Choose a reason for hiding this comment

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

We don't rely on it. LGTM.

@etrepum etrepum added this pull request to the merge queue Sep 18, 2025
Merged via the queue into facebook:main with commit 6970ba5 Sep 18, 2025
73 of 74 checks passed
This was referenced Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. extended-tests Run extended e2e tests on a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants