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

Core: Merge channels into a single package #23032

Merged
merged 11 commits into from
Jun 13, 2023
Merged

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Jun 12, 2023

Telescoped off: #22940

  • Moving all the code for the 2 channels we have to @storybook/channels
  • Moving the empty channels to ./code/deprecated, added shims to the old content that's moved.
  • Adjusted our code to import createPostMessageChannel from @storybook/channels
  • Reverted the change to createPostMessageChannel, and introduced a createBrowserChannel function
  • Updated the builders to use createBrowserChannel

Custom builders will need this change applied as well, or their browser won't listen to the serverChannel.

@ndelangen ndelangen added the maintenance User-facing maintenance tasks label Jun 12, 2023
@ndelangen ndelangen self-assigned this Jun 12, 2023
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

Seems like a good step forward but I still don't think createPostMessageChannel() should end up creating both a PM and WS transport, that's just confusing.

My advice would be to have two files, postmessageTransport.ts and websocketTransport.ts that are responsible for the transports, and are "dumb". Then a second file createBrowserChannel or similar that uses the two transports and is used by the builders.

Base automatically changed from norbert/server-channel-for-addons to next June 13, 2023 06:04
@ndelangen ndelangen changed the title move code into lib/channels deprecate the individual channels Maintenance: Deprecate channel packages, merge into a single browserChannel, update builders Jun 13, 2023
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

LGTM thanks @ndelangen!

@socket-security
Copy link

socket-security bot commented Jun 13, 2023

New and updated dependency changes detected. Learn more about Socket for GitHub ↗︎

@ndelangen ndelangen merged commit 4bbd7e8 into next Jun 13, 2023
@ndelangen ndelangen deleted the norbert/merge-channels branch June 13, 2023 08:23
@shilman shilman changed the title Maintenance: Deprecate channel packages, merge into a single browserChannel, update builders Core: Merge channels into a single package Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants