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.

Sign up for GitHub

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

Jump to bottom

feat(remix-react): add WebSocket reconnect to LiveReload #859

Merged
merged 7 commits into from
Jul 11, 2022
Merged

feat(remix-react): add WebSocket reconnect to LiveReload #859

merged 7 commits into from
Jul 11, 2022

Conversation

garand
Copy link
Contributor

@garand garand commented Dec 2, 2021

Currently if the LiveReload socket closes, for instance if the dev server restarts, the page loses connection to the socket and needs to be manually refreshed to reconnect.

This wraps the WebSocket connection script in a function that is immediately called, and adds an onclose method letting the user know the connection has closed. A timeout is created (1 second) to reconnect and pass an optional config object with an onOpen callback to force a reload when a new connection is opened.

This was born out of my use case of building a component library outside of Remix and setting up a separate script to restart the dev server when my component library changed.

Original discussion on Discord.

https://discord.com/channels/770287896669978684/911308489769488485/911314308967841832

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Dec 2, 2021

Hi @garand,

Welcome, and thank you for contributing to Remix!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected].

Thanks!

- The Remix team

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Dec 2, 2021

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

@machour
Copy link
Collaborator

machour commented Mar 18, 2022

Seems like this is now implemented

@machour machour closed this Mar 18, 2022
@garand
Copy link
Contributor Author

garand commented Mar 21, 2022

@machour Are you sure? I'm not seeing this implemented yet.

@machour machour reopened this Mar 21, 2022
@machour
Copy link
Collaborator

machour commented Mar 21, 2022

@garand you're absolutely right, got my wires mixed up 😅
Could you fix conflicts so that the team may give this a look? Thank you, and sorry again!

@machour machour added needs-response We need a response from the original author about this issue/PR feat:dx Issues related to the developer experience labels Mar 21, 2022
@MichaelDeBoey MichaelDeBoey changed the title feat(LiveReload): add WebSocket reconnect feat(remix-react): add WebSocket reconnect to LiveReload Mar 28, 2022
@MichaelDeBoey
Copy link
Member

@garand Please rebase your branch onto latest dev & fix conflicts?

@garand
Copy link
Contributor Author

garand commented Apr 7, 2022

@MichaelDeBoey I'll try to work on this soon! 😄

@github-actions github-actions bot removed the needs-response We need a response from the original author about this issue/PR label Apr 7, 2022
@MichaelDeBoey MichaelDeBoey added the needs-response We need a response from the original author about this issue/PR label Apr 7, 2022
@MichaelDeBoey MichaelDeBoey removed the needs-response We need a response from the original author about this issue/PR label Jun 2, 2022
@MichaelDeBoey
Copy link
Member

@garand Could you please provide us with an update on this PR?

@MichaelDeBoey MichaelDeBoey added the needs-response We need a response from the original author about this issue/PR label Jun 2, 2022
@garand
Copy link
Contributor Author

garand commented Jun 2, 2022

Yeah, didn't have a chance to get to it yet, I'll prioritize getting it cleaned up tonight.

@github-actions github-actions bot removed the needs-response We need a response from the original author about this issue/PR label Jun 2, 2022
@MichaelDeBoey MichaelDeBoey added the needs-response We need a response from the original author about this issue/PR label Jun 2, 2022
@garand
Copy link
Contributor Author

garand commented Jun 3, 2022

@MichaelDeBoey Updated! 😅

@github-actions github-actions bot removed the needs-response We need a response from the original author about this issue/PR label Jun 3, 2022
@changeset-bot
Copy link

changeset-bot bot commented Jul 11, 2022 •
edited
Loading

🦋 Changeset detected

Latest commit: 2e5ff21

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
remix Patch
@remix-run/react Patch
@remix-run/dev Patch
@remix-run/eslint-config Patch
@remix-run/serve Patch
@remix-run/server-runtime Patch
@remix-run/cloudflare Patch
@remix-run/node Patch
@remix-run/deno Patch
@remix-run/cloudflare-pages Patch
@remix-run/cloudflare-workers Patch
@remix-run/express Patch
@remix-run/netlify Patch
@remix-run/vercel Patch
create-remix Patch
@remix-run/architect Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

mcansh
mcansh approved these changes Jul 11, 2022
View reviewed changes
Copy link
Collaborator

@mcansh mcansh 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 @garand!

@mcansh mcansh merged commit 2d46b91 into remix-run:dev Jul 11, 2022
@garand garand deleted the dev branch July 11, 2022 14:45
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-de9fc05-20220712 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@fknop fknop mentioned this pull request Aug 10, 2022
Closed
@fknop
Copy link

fknop commented Aug 10, 2022

Hello, I think this PR might cause issue with redirects #2997 (comment)

@danieljb danieljb mentioned this pull request Dec 7, 2022
Merged
@Varedis Varedis mentioned this pull request Mar 26, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcansh mcansh mcansh approved these changes

Assignees

@mcansh mcansh

Labels
CLA Signed feat:dx Issues related to the developer experience renderer:react
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@garand @machour @MichaelDeBoey @fknop @mcansh