Skip to content

refactor!: remove HotBroadcaster #19988

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

Merged
merged 5 commits into from
May 29, 2025

Conversation

sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented May 2, 2025

Description

built on top of #19987

This PR removes HotBroadcaster type and changes the type of server.hot to NormalizedHotChannel.

  • 17db814: simply reduces the code by using the fact that the number of channel is 1
  • 9e7b2cf: correct the comment. server.hot is more like an alias to server.environments.client.hot instead of a separate concept
  • ad9baa3: remove deprecated no-op HotBroadcaster.addChannel method
  • ec6eb8e: deprecate HotBroadcaster.channels property. HotBroadcaster is deprecated but server.hot is not deprecated, maybe users won't expect this property to be removed.
  • 53b8bc1: remove HotBroadcaster.channels property. If removing this property directly without a deprecation phase, this commit can be included. I guess it's fine to remove it directly. It's always [server.ws] and easy to replace. While I found server.hot used in many places, I only found hot.channels used in a single place.

I didn't deprecate server.hot as it's used in many places and keeping that won't be much burden.

@sapphi-red sapphi-red added p1-chore Doesn't change code behavior (priority) breaking change labels May 2, 2025
@sapphi-red sapphi-red added this to the 7.0 milestone May 2, 2025
@github-project-automation github-project-automation bot moved this to Discussing in Team Board May 2, 2025
@sapphi-red sapphi-red force-pushed the refactor/remove-hot-broadcaster branch from c8adc16 to 53b8bc1 Compare May 16, 2025 03:13
@sapphi-red sapphi-red marked this pull request as ready for review May 16, 2025 03:14
Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

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

I agree it is better to remove hot.channels in Vite 7
@AlemTuzlak, this would break react-router-devtools (here and in a few others places) as pointed out by @sapphi-red in the description. Maybe you could release a patch to make the access optional to avoid disruption when Vite 7 is released?

@AlemTuzlak
Copy link
Contributor

@patak-dev Thank you so much for the heads up! On it!

@AlemTuzlak
Copy link
Contributor

@patak-dev opened up a PR on react-router-devtools, mind checking the plugin file, I think this is all that is needed (I tested it out, all seems to work, but I'd like to confirm):
forge-42/react-router-devtools#208

@patak-dev
Copy link
Member

Looks good to me Alem, thanks for the quick change!

@sapphi-red sapphi-red merged commit cda8c94 into vitejs:main May 29, 2025
16 checks passed
@sapphi-red sapphi-red deleted the refactor/remove-hot-broadcaster branch May 29, 2025 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change p1-chore Doesn't change code behavior (priority)
Projects
Status: Discussing
Development

Successfully merging this pull request may close these issues.

3 participants