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

LiveReload - Indefinite loop in Firefox #4692

Closed
Vivalldi opened this issue Nov 26, 2022 · 7 comments
Closed

LiveReload - Indefinite loop in Firefox #4692

Vivalldi opened this issue Nov 26, 2022 · 7 comments
Labels
bug:unverified bug Something isn't working

Comments

@Vivalldi
Copy link

Vivalldi commented Nov 26, 2022

What version of Remix are you using?

v1.7.6

Steps to Reproduce

  1. create-remix using defaults
  2. Add a loader to app/routes/index.tsx that sleeps for 1 second
  3. Open http://localhost:3000 in Firefox Developer Edition (I'm using 108.0b6 (64-bit) macOS silicon)
  4. Make a change to the file to cause remix rebuild while the firefox window is in view (focus state doesn't seem to matter)
  5. Page continually reloads until window is no longer in view (focus state doesn't seem to matter)
export const loader: LoaderFunction = async () => {
        await new Promise((resolve) => setTimeout(resolve,1000));
        const data = { key: "value" };
        return json(data);
}

Expected Behavior

The page should only live reload once

Actual Behavior

The page reloads in a looping manner. Normal behavior can be observed in chrome or by removing the sleeping timeout.

@seldo
Copy link

seldo commented Nov 26, 2022

I have been banging my head against a wall for 24 hours about why my OAuth 2 flow doesn't work and the reason is this bug. My login flow reloads infinitely in Firefox but works as expected in Chrome. Full repro is at this commit: https://github.com/seldo/alpaca/tree/ae5270fe728e0cef05119e0fe650a305ad79c24b

import { useLoaderData, Form } from "@remix-run/react";
import authenticator from "../../services/auth.server";

export let loader = async ({request}) => {

  let user = await authenticator.isAuthenticated(request)
  if( user ) {
    console.log("Login says: authenticated!")
    console.log(user)
    return user
  } else {
    console.log("Login says: not authenticated")
    console.log(user)
    return false
  }

}

export default function Login() {
  let user = useLoaderData()
  if(user) {
    return (
      <div>You are logged in!!!</div>
    )  
  } else {
    return (
      <div>
        <Form method="get" action="/auth/mastodon">
          <button>Login</button>
        </Form>
      </div>
    )   
  }
}

@seldo
Copy link

seldo commented Nov 27, 2022

After further digging I think this might be a result of an existing issue with LiveReload: #2997 but I'm not sure.

@ryanflorence
Copy link
Member

I don't even have to change a file, just the initial load in firefox makes it trip up.

ryanflorence added a commit that referenced this issue Nov 30, 2022
Firefox infinitely reloads the page as long as `<LiveReload>` is rendering.

The problem is:

1. Firefox is calling `ws.onclose` immediately upon connecting (?!)

2. Then we’re trying to reconnect, and upon reconnection, we reload the page.

3. Firefox then calls `ws.onclose` again after reconnecting and the loop starts over

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.

Closes #4692
ryanflorence added a commit that referenced this issue Nov 30, 2022
Firefox infinitely reloads the page as long as `<LiveReload>` is rendering.

Closes #4692
@ryanflorence
Copy link
Member

Alright, soon as the tests pass we'll merge that PR and then the fix will be available in the nightly releases until the next official release goes out next week.

So you can either wait for the nightly tonight, wait for next week, or copy/paste this and use it instead:

copy/paste this
export const LiveReload =
  process.env.NODE_ENV !== "development"
    ? () => null
    : function LiveReload({
        port = Number(process.env.REMIX_DEV_SERVER_WS_PORT || 8002),
        nonce = undefined,
      }: {
        port?: number;
        /**
         * @deprecated this property is no longer relevant.
         */
        nonce?: string;
      }) {
        let js = String.raw;
        return (
          <script
            nonce={nonce}
            suppressHydrationWarning
            dangerouslySetInnerHTML={{
              __html: js`
                function remixLiveReloadConnect(config) {
                  let protocol = location.protocol === "https:" ? "wss:" : "ws:";
                  let host = location.hostname;
                  let socketPath = protocol + "//" + host + ":" + ${String(
                    port
                  )} + "/socket";
                  let ws = new WebSocket(socketPath);
                  ws.onmessage = (message) => {
                    let event = JSON.parse(message.data);
                    if (event.type === "LOG") {
                      console.log(event.message);
                    }
                    if (event.type === "RELOAD") {
                      console.log("💿 Reloading window ...");
                      window.location.reload();
                    }
                  };
                  ws.onopen = () => {
                    if (config && typeof config.onOpen === "function") {
                      config.onOpen();
                    }
                  };
                  ws.onclose = (event) => {
                    console.log("Remix dev asset server web socket closed. Reconnecting...");
                    if (event.code === 1006) {
                      setTimeout(
                        () =>
                          remixLiveReloadConnect({
                            onOpen: () => window.location.reload(),
                          }),
                        1000
                      );
                    }
                  };
                  ws.onerror = (error) => {
                    console.log("Remix dev asset server web socket error:");
                    console.error(error);
                  };
                }
                remixLiveReloadConnect();
              `,
            }}
          />
        );
      };

ryanflorence added a commit that referenced this issue Nov 30, 2022
Firefox infinitely reloads the page as long as `<LiveReload>` is rendering.

Closes #4692
@machour machour added the awaiting release This issue has been fixed and will be released soon label Dec 1, 2022
@massimopalmieri
Copy link

I'm on v1.8.0 and I'm still getting this issue, only on Firefox on the initial page load, without making any file change

@seldo
Copy link

seldo commented Dec 3, 2022

I think I've seen it once or twice since upgrading to 1.8, also on Firefox, but I've had trouble reproducing.

@machour
Copy link
Collaborator

machour commented Dec 3, 2022

The PR was merged after 1.8 was cut, fix will be available in next release.

Re-opening this issue until then to avoid confusion.

@machour machour reopened this Dec 3, 2022
@machour machour added bug Something isn't working bug:unverified and removed bug:unverified labels Dec 3, 2022
pcattori added a commit that referenced this issue Dec 16, 2022
* 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>
@brophdawg11 brophdawg11 removed the awaiting release This issue has been fixed and will be released soon label Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:unverified bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants