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

feat(comlink)!: public api improvements #2207

Merged
merged 5 commits into from
Nov 28, 2024
Merged

feat(comlink)!: public api improvements #2207

merged 5 commits into from
Nov 28, 2024

Conversation

rdunk
Copy link
Member

@rdunk rdunk commented Nov 26, 2024

This PR introduces some changes to the public API of @sanity/comlink that I've wanted to make for a while. There are no new features, this is mainly refactoring, but there are breaking changes. All consumers of the package in this repo have been updated accordingly, including the compatibility actors.

This isn't strictly necessary from our point of view, but as this package is likely going to be used by other teams, I felt the improvements would be useful.

Terminology

In the initial version, I think some concepts were incorrectly named. The concepts of Channels and Connections have now been swapped, which I think makes much more sense:

(Apologies for the unorthodox use of diff block here 😄 )

-- Controllers create Connections which maintain Channels to Nodes

++ Controllers create Channels which maintain Connections to Nodes.

post and fetch signatures

The signatures for .post() and .fetch() methods have changed:

-- comlink.post({type: 'visual-editing/toggle', data: {enabled: true}})

++ comlink.post('visual-editing/toggle', {enabled: true})

This is more concise and aligns with .on(), which already looks like this:

comlink.on('visual-editing/toggle', (data) => {})

Type argument order

The order of type arguments has been updated in many interfaces and functions. Placing Sends before Receives feels more natural and aligns better with expectations:

-- export interface ChannelInstance<R extends Message, S extends Message> {}

++ export interface ChannelInstance<S extends Message, R extends Message> {}

Copy link

vercel bot commented Nov 26, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
live-visual-editing-next ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 28, 2024 10:20am
visual-editing-astro ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 28, 2024 10:20am
visual-editing-next ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 28, 2024 10:20am
visual-editing-next-with-i18n ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 28, 2024 10:20am
visual-editing-nuxt ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 28, 2024 10:20am
visual-editing-page-builder-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 28, 2024 10:20am
visual-editing-remix ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 28, 2024 10:20am
visual-editing-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 28, 2024 10:20am
visual-editing-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 28, 2024 10:20am
visual-editing-svelte ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 28, 2024 10:20am

@rdunk rdunk changed the title refactor(comlink)!: swap channels and connection terminology refactor(comlink)!: public api improvements Nov 27, 2024
@rdunk rdunk changed the title refactor(comlink)!: public api improvements feat(comlink)!: public api improvements Nov 27, 2024
Copy link
Member

@stipsan stipsan left a comment

Choose a reason for hiding this comment

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

Stellar work @rdunk! Absolutely love this 💖
As far as I'm concerned we should merge and publish this asap 🏎️ 💨
Looks like the new API signature has better typing and autocomplete as well :chefs-kiss:
A nice follow up could be to move the compatibility actor out of @repo/visual-editing-helpers and to @sanity/comlink, perhaps on its own export path, to resolve #2165 💖

@rdunk
Copy link
Member Author

rdunk commented Nov 28, 2024

A nice follow up could be to move the compatibility actor out of @repo/visual-editing-helpers and to @sanity/comlink, perhaps on its own export path, to resolve #2165 💖

That's a great idea! Thanks.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants