bug: LiveReload causes infinite reload loops on Firefox if the loader is slow

[cmd-johnson]

• [ ] Docs
• [x] Tests

I recently upgraded Remix from v1.4.3 to v1.7.5 on one of my projects. Since the update, LiveReload sometimes didn't work properly on Firefox anymore.

After some investigation, it turns out that the script used by the LiveReload component to reload the page causes an infinite reload loop when the page takes too long to load. This is easily reproducable by adding a loader with the following code to an empty page:

export async function loader() {
  await new Promise((resolve) => {
    setTimeout(resolve, 2000);
  });

  return json({});
}

Digging deeper, this looks to be an issue with how Firefox is handling WebSockets differently than e.g. Chrome when it comes to reloads. When the page reloads in Chrome, the WebSocket connection never closes and the WebSocket's onclose handler is never called. Firefox however closes the WebSocket connection right after calling window.location.reload(), leading to the following scenario: (I'm referencing line numbers from the LiveReload implementation)

1. after detecting a file change and rebuilding, the remix server sends the RELOAD event to the client, which triggers a window reload (L1547)
2. the window reload leads to Firefox closing the WebSocket connection, calling the onclose handler (L1555)
3. reloading the page takes longer than 1 second. The timeout in the onclose handler (L1557) calls remixLiveReloadConnect(...)
4. a new connection to the WebSocket is established (L1539)
5. this new connection calls the onOpen callback that was passed from L1559
6. the onOpen callback calls window.location.reload() (L1560), cancelling the previous reload request and starting a new one
7. this again causes the onclose handler to set a new timeout and the cycle starts again.

As far as I can tell there aren't any integration tests yet that use cli.js dev to watch for file changes, so I added a startDevServer function to handle this scenario.

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: b33a0840c1d40ce27a5b2597caf6aa4f52f797de

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[cmd-johnson]

That's... weird. Looks like the cli.js dev process doesn't work on CI, but it definitely works on my machine™.

Edit: it might have to to with the env property replacing the spawned processes' environment instead of augmenting it. Maybe adding the PATH back to it helps :shrug:

[DiederikvandenB]

I've been experiencing this issue too, but unfortunately this fix doesn't solve the issue for me.

Strangely enough, even a new instance of Remix using the Indie stack also gives me Remix dev asset server web socket closed. Reconnecting... after the first reload. No issues in Safari. I've disabled all extensions in Firefox, which didn't help, and the issue does not occur in Safari. I'm perplexed...

[cmd-johnson]

@DiederikvandenB Now that you mention it, that's something I didn't even test for. I also get infinite reload loops when I reload an already open app.

I looked into that and I think I have a fix for that, too:

live-reload.tsx

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
      }) {
        const 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 alreadyReloading = false;

                  window.addEventListener('beforeunload', () => {
                    alreadyReloading = true;
                  });

                  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 = (error) => {
                    if (alreadyReloading) {
                      return;
                    }
                    console.log("Remix dev asset server web socket closed. Reconnecting...");
                    setTimeout(
                      () =>
                        remixLiveReloadConnect({
                          onOpen: () => window.location.reload(),
                        }),
                      1000
                    );
                  };
                  ws.onerror = (error) => {
                    console.log("Remix dev asset server web socket error:");
                    console.error(error);
                  };
                }
                remixLiveReloadConnect();
              `,
            }}
          />
        )
      }

At least for me, this now works correctly in both Chrome and Firefox when

• Refreshing the page (pressing F5)
• Triggering a reload due to file changes
• Reloading when the dev server is stopped and then started again

[DiederikvandenB]

@cmd-johnson Thank you for investigating this further. I will hopefully find some time these next few days to try your newest fix. I will report back.

As a side note, are we the only two souls using Firefox and Remix? I would expect this issue to get much more attention than it does. Which makes me think, are there some weird environmental issues causing this?

[DiederikvandenB]

I'll probably open a separate issue to raise this, but I'd like to reference it here too:

There's some wacky things going on with Firefox (without using the fix in this PR). There was a bug causing my entry.server.tsx to crash. Firefox went on like everything was fine. I only found out after I tried adding a new route. Firefox kept showing my app's 404 page. I thought I was crazy, until I remembered this issue and I opened the page in Safari. Immediately, Safari showed a Server cannot be reached, and only then, the terminal in which I ran yarn dev logged the error that crashed my entry.server.tsx.

I've now reached a point where I will stop using Firefox for developing Remix apps, at least until these issues are fixed.

[pcattori]

This should be fixed by v1.9.0