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

fix(remix-react): Firefox LiveReload #4725

Merged
merged 1 commit into from Nov 30, 2022
Merged

fix(remix-react): Firefox LiveReload #4725

merged 1 commit into from Nov 30, 2022
+26 −10

Conversation

ryanflorence
Copy link
Member

Firefox infinitely reloads the page as long as <LiveReload> is rendering.

Closes #4692

@changeset-bot
Copy link

changeset-bot bot commented Nov 30, 2022 •
edited

🦋 Changeset detected

Latest commit: 28f803c

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

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

@ryanflorence
fix: Firefox LiveReload
28f803c 
Firefox infinitely reloads the page as long as `<LiveReload>` is rendering.

Closes #4692
@ryanflorence ryanflorence merged commit 95b4b28 into dev Nov 30, 2022
@ryanflorence ryanflorence deleted the fix-firefox-livereload branch November 30, 2022 23:51
@github-actions github-actions bot added the awaiting release This issue has been fixed and will be released soon label Nov 30, 2022
@OllyHodgson OllyHodgson mentioned this pull request Dec 1, 2022
Closed
@machour machour mentioned this pull request Dec 1, 2022
Closed
@MichaelDeBoey MichaelDeBoey changed the title fix: Firefox LiveReload fix(remix-react): Firefox LiveReload Dec 2, 2022
danieljb
danieljb reviewed Dec 7, 2022
View reviewed changes
.changeset/thin-oranges-boil.md

This fix is to check `event.code === 1006` before actually trying to reconnect and the reload the page. 1006 means the connection was closed abnormally (https://www.rfc-editor.org/rfc/rfc6455#section-7.4.1). In our case, that means the server was shut down in local dev and then the socket can reconnect again when the server is back up.

It’s unclear to me why Firefox is calling `onclose` immediately upon connecting to the web socket, but it does.
Copy link

Choose a reason for hiding this comment

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

Hi. When I first had this issue I did some digging and might have an idea of what’s happening:

  1. files change
  2. [Chrome, Firefox] websocket receives message type: RELOAD, triggers window.location.reload
  3. [Chrome, Firefox] start reload Navigated to http://localhost:3000/
  4. [Firefox] closes the websocket connection code: 1001
  5. [Firefox] sets timeout to call remixLiveReloadConnect
  6. [Firefox] timeout completes before reload is complete and calls:
    remixLiveReloadConnect({ onOpen() { /* … */ } })
  7. [Firefox] re-connects to the websocket, calls onOpen method that triggers another window.location.reload starting the infinite reloading

My conclusion is Firefox does not in fact call onclose upon connecting. It is the other way around, the connection is closed and tries to reconnect before the browser fully unloads the page. Both Chrome and Safari seem to only send the websocket close event when the dev server is stopped and not when the page reloads.

The reason why the onOpen config was introduced was to enable automatic re-connection when the dev server is stopped and restarted. When that happens the CloseEvent is sent with code 1006 which is why both of us came up with the event.code === 1006 check.

This fixes the infinite reloads. But the issue on Firefox is actually a race condition. When the timeout is increased (e.g. to 10000) the page reload reliably finishes before the timeout re-initializes, re-connects to the websocket and the reloads loop kicks in.

This was referenced Dec 13, 2022
Closed
Closed
Merged
pcattori added a commit that referenced this pull request Dec 16, 2022
@pcattori @mcansh
@ascorbic @remix-run-bot @chaance @ryanflorence @lucasferreira @AndrewLeedham @brophdawg11 @dabdine @jacob-ebey @jrestall @MichaelDeBoey @machour @dmarkow @lordofthecactus @akamfoad @Andarist @gyx1000
fix(dev): build modules for ts->js conversion (#4891)
eb818e4 
* fix(remix-dev): convert `config.appDirectory` to relative unix path (#4709)

* fix(remix-dev): convert appDirectory to unix style for fast-glob

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* chore: relative path

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* chore: add test

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* fix test

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* fix: typo

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* chore: update test

Signed-off-by: Logan McAnsh <logan@mcan.sh>

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* chore: add changeset for #4709 (#4718)

* ci: add typechecking for deno (#4715)

* ci: add typechecking for deno

* ci: install deno for integration tests

* chore: format

* chore: format

* fix: Firefox LiveReload (#4725)

Firefox infinitely reloads the page as long as `<LiveReload>` is rendering.

Closes #4692

* fix(remix-dev): allow defining multiple routes for the same route module file (#3970)

* Allow multiple routes for same route module

* Update packages/remix-dev/config/routes.ts

Co-authored-by: Andrew Leedham <AndrewLeedham@outlook.com>

* Update routes.ts

- Better name for automated ID variable;
- Small adjust in `id` option comment;

* - Removing redundant IF

* Update routes.ts

Revert complex custom ID in routes

* Non unique custom routes ID error and test

* Update assets.ts

Trying to solve a conflict

* Revert "Update assets.ts"

This reverts commit 2064c57.

* Error on collisions with non-custom routeIds

* Create big-spoons-grab.md

Co-authored-by: Andrew Leedham <AndrewLeedham@outlook.com>
Co-authored-by: Matt Brophy <matt@brophy.org>

* feat: Allow pass-through script props in `ScrollRestoration` (#2879)

* ci(nightly): add deno for typechecking deno files (#4738)

* ci: fix race condition writing globals.d.ts shim (#4717)

Co-authored-by: Chance Strickland <hi@chance.dev>

* chore: bump @playwright/test to latest (#4749)

Signed-off-by: Logan McAnsh <logan@mcan.sh>

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* Fix 4199: TypedResponse allows incompatible types (#4734)

* Fixes #4199: Do not allow assignment of incompatible TypedResponses

* Add myself to contributors.yml

* Create light-sheep-give.md

* slight changeset tweak

* additional changeset tweaks

Co-authored-by: Pedro Cattori <pcattori@gmail.com>

* chore: format

* test: add transition integration tests (#4739)

test: useTransition to wait for states

This approach could probably be applied across other flakey tests and could also be documented as a good approach of if there is user-feedback for a specific action when running integration tests.

* feat: testing helpers (#4539)

Co-authored-by: James Restall <james.restall@gmail.com>
Signed-off-by: Logan McAnsh <logan@mcan.sh>

* chore: format

* chore(remix-testing): update dependencies (#4756)

* fix(remix-testing): fix deps (#4757)

* Fix deps for new remix-testing package

* fix lint

* Remove ENABLE_REMIX_ROUTER flags (#4732)

* chore: add `@remix-run/testing` to changesets (#4781)

* chore: add `@remix-run/testing` to changesets

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* chore: update `ADDING_A_PACKAGE.md`

Signed-off-by: Logan McAnsh <logan@mcan.sh>

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* refactor(remix-react): upgrade Remix to `react-router-dom@6.4` (non-data-router) and drop `history` (#4731)

* Bump remix to react-router-dom@6.4.4 (#4668)

Co-authored-by: Mehdi Achour <machour@gmail.com>

* Bump remix to RR 6.4.4 and drop history dependency (#4702)

Co-authored-by: Mehdi Achour <machour@gmail.com>

* ci(nightly): move git operations after build (#4797)

* ci(nightly): move git operations after build

when adding deno typechecking (#4715), nightly now refers to the version we just calculated which hasnt been published when trying to build and typecheck

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* ci: update tmp branch name

Signed-off-by: Logan McAnsh <logan@mcan.sh>

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* perf(remix-dev): Optimize `parentRouteId` lookup in `defineConventionalRoutes` (#4538)

* Use object for parentRouteId lookup

* Sign CLA

* Move parentRouteId logic to a separate function

* Update test fixture path

* chore: format

* chore(dev): add changeset for PR #4538 (#4800)

* chore: format

* refactor(remix-react): use `react-router-dom` import instead of `react-router` (#3325)

* chore: manually bump remix-testing version due to not being on main just yet

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* fixup! chore: manually bump remix-testing version due to not being on main just yet

* fixup! chore: manually bump remix-testing version due to not being on main just yet

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* chore: unify error usage (#4696)

* chore: format

* Add fetcher state/type tests (#4803)

* chore(deps): bump esbuild to latest (#4754)

* chore: add invariant instead of using `!`

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* chore(deps): bump esbuild to latest

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* Create fresh-shrimps-join.md

Signed-off-by: Logan McAnsh <logan@mcan.sh>
Co-authored-by: Pedro Cattori <pcattori@gmail.com>

* test(integration): close server synchronously  (#4785)

* chore: normalize afterAll

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* test: close server synchronously

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* chore: appFixture.close isnt async anymore

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* Update integration/helpers/create-fixture.ts

Co-authored-by: Pedro Cattori <pcattori@gmail.com>

Signed-off-by: Logan McAnsh <logan@mcan.sh>
Co-authored-by: Pedro Cattori <pcattori@gmail.com>

* feat: remix optional segments (#4706)

* feat: transform optional routes from remix to react router

* Add to contributors

* Add changeset

* fix(optional-segments): fix escaping of parenthesis

* small function fix

* Update packages/remix-dev/__tests__/routesConvention-test.ts

Co-authored-by: Pedro Cattori <pcattori@gmail.com>

Co-authored-by: Pedro Cattori <pcattori@gmail.com>

* chore: format

* chore: edit the optional segments changeset (#4815)

to make it clear that Remix won't support optional segments until integrated with React Router 6.5

* chore: format

* docs: rearrange docs, give everything its own page (#4821)

* chore: format

* fix: wrong parentheses in the optional segments changeset (#4825)

* fix: wrong parentheses in the optional segments changeset

* add: akamfoad to the contributers

* ci(nightly): add workflow_call for testing purposes

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* Revert "ci(nightly): add workflow_call for testing purposes"

This reverts commit f3fa77e.

* chore(scripts): Use relaxed peer dependencies to avoid triggering major version bumps (#4736)

- Add script to bump peer deps with changesets version

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>

* Add integration tests for request structures (#4829)

* fix(remix-dev): resolve asset entry full path to support mono-repo import of styles (#4855)

* chore: have eslint report unused eslint comments (#4863)

* chore: have eslint report unused eslint comments

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* chore: remove additional comment

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* perf(remix-architect,remix-netlify): improve performance of `isBinaryType` (#4761)

Co-authored-by: Logan McAnsh <logan@mcan.sh>
Co-authored-by: Pannatier Guillaume <Guillaume.Pannatier@hopitalvs.ch>

* chore: format

* chore(remix-testing): remove internal installGlobals (#4755)

* chore: remove internal installGlobals

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* chore: add README

Signed-off-by: Logan McAnsh <logan@mcan.sh>

* Create quick-cats-fix.md

* Update .changeset/quick-cats-fix.md

Co-authored-by: Michaël De Boey <info@michaeldeboey.be>

* chore(deps): remove jsdom and happydom from devDependencies

Signed-off-by: Logan McAnsh <logan@mcan.sh>

Signed-off-by: Logan McAnsh <logan@mcan.sh>
Co-authored-by: Michaël De Boey <info@michaeldeboey.be>

* fix(dev): build js modules for ts->js conversion

The TS->JS migration was removed from the CLI codemod options, but still
used for TS->JS conversion when creating a new Remix project from the
CLI. The TS modules responsible for the TS->JS conversion were
incorrectly removed from the Rollup build, resulting in the
corresponding built JS modules being absent. That caused the CLI to
error when trying to perform TS->JS conversion. This changes
reintroduces the wiring to build the modules responsible for the TS->JS
conversion.

Fixes #4854

Signed-off-by: Logan McAnsh <logan@mcan.sh>
Co-authored-by: Logan McAnsh <logan@mcan.sh>
Co-authored-by: Matt Kane <m@mk.gg>
Co-authored-by: Remix Run Bot <hello@remix.run>
Co-authored-by: Chance Strickland <hi@chance.dev>
Co-authored-by: Ryan Florence <rpflorence@gmail.com>
Co-authored-by: Lucas Ferreira <panchorf@gmail.com>
Co-authored-by: Andrew Leedham <AndrewLeedham@outlook.com>
Co-authored-by: Matt Brophy <matt@brophy.org>
Co-authored-by: dabdine <1955040+dabdine@users.noreply.github.com>
Co-authored-by: Jacob Ebey <jacob.ebey@live.com>
Co-authored-by: James Restall <james.restall@gmail.com>
Co-authored-by: Michaël De Boey <info@michaeldeboey.be>
Co-authored-by: Mehdi Achour <machour@gmail.com>
Co-authored-by: Dylan Markow <dylan@dylanmarkow.com>
Co-authored-by: Daniel Rios <ieldanr@gmail.com>
Co-authored-by: Akam Foad <41629832+akamfoad@users.noreply.github.com>
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Co-authored-by: Guillaume Pannatier <guillaume.pannatier@gmail.com>
Co-authored-by: Pannatier Guillaume <Guillaume.Pannatier@hopitalvs.ch>
@remix-run-bot remix-run-bot mentioned this pull request Dec 16, 2022
Merged
@mcansh mcansh mentioned this pull request Jan 10, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danieljb danieljb danieljb left review comments

@chaance chaance chaance left review comments

Assignees
No one assigned
Labels
awaiting release This issue has been fixed and will be released soon CLA Signed renderer:react
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ryanflorence @danieljb @chaance @MichaelDeBoey