remix icon indicating copy to clipboard operation
remix copied to clipboard

[Bug]: App crash on fast browser navigation

Open axel-planfox opened this issue 3 years ago • 15 comments

What version of Remix are you using?

1.1.3

What version of Node are you using? Minimum supported version is 14.

16.0.0

Steps to Reproduce

  1. Create a new Remix app with npx create-remix@latest.
  2. Choose the Remix dev server.
  3. Install dependencies using yarn install.
  4. Next to app/routes/index.tsx, add app/routes/other.tsx with a component that simply renders <>Other</>
  5. In app/routes/index.tsx, add <Link to="/other">Other</Link> somewhere, with import { Link } from "remix";.
  6. Run your app using yarn dev or yarn start; the error happens for both dev and prod builds.
  7. Open your browser and create a new browser tab (the bug happens with both Firefox and Chrome).
  8. Go to your app's URL.
  9. Click the link to Other.
  10. Click the browser back button two times. The tab should now show your browser's default UI for new tabs.
  11. Now very quickly click the browser's forward button twice.
  12. The app should now crash. If it doesn't you didn't click the forward button fast enough.

Expected Behavior

The page content should be displayed without any errors.

Actual Behavior

In the simple repo above, the app crashes and there is the following stack trace in the dev console. In my real app in a production build, this issue then causes #1678, basically making the entire browser tab unresponsive.

components.js:470 Uncaught TypeError: Cannot read properties of undefined (reading 'meta')
    at Meta (components.js:470:21)
    at renderWithHooks (react-dom.development.js:14985:18)
    at mountIndeterminateComponent (react-dom.development.js:17811:13)
    at beginWork (react-dom.development.js:19049:16)
    at HTMLUnknownElement.callCallback2 (react-dom.development.js:3945:14)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:3994:16)
    at invokeGuardedCallback (react-dom.development.js:4056:31)
    at beginWork$1 (react-dom.development.js:23964:7)
    at performUnitOfWork (react-dom.development.js:22776:12)
    at workLoopSync (react-dom.development.js:22707:5)
Meta @ components.js:470
renderWithHooks @ react-dom.development.js:14985
mountIndeterminateComponent @ react-dom.development.js:17811
beginWork @ react-dom.development.js:19049
callCallback2 @ react-dom.development.js:3945
invokeGuardedCallbackDev @ react-dom.development.js:3994
invokeGuardedCallback @ react-dom.development.js:4056
beginWork$1 @ react-dom.development.js:23964
performUnitOfWork @ react-dom.development.js:22776
workLoopSync @ react-dom.development.js:22707
renderRootSync @ react-dom.development.js:22670
performSyncWorkOnRoot @ react-dom.development.js:22293
scheduleUpdateOnFiber @ react-dom.development.js:21881
updateContainer @ react-dom.development.js:25482
(anonymous) @ react-dom.development.js:26021
unbatchedUpdates @ react-dom.development.js:22431
legacyRenderSubtreeIntoContainer @ react-dom.development.js:26020
hydrate2 @ react-dom.development.js:26086
(anonymous) @ entry.client.tsx:4```

axel-planfox avatar Feb 01 '22 10:02 axel-planfox

https://github.com/remix-run/remix/issues/1618 might be related to this

revelt avatar Feb 02 '22 21:02 revelt

This seems to be caused by a race condition in the Remix code.

As far as I understand, the loadRouteModule function in routeModules.ts loads the JavaScript file related to a route and then adds an entry to the routeModules cache.

The Meta component uses this cache to do its thing.

If you're doing the browser navigation slowly, everything works as expected: loadRouteModule loads the JS file, updates the cache and only then does Meta try to use it. If you're fast, however, loadRouteModule hasn't updated the routeModules cache yet, but Meta tries to access it anyway, resulting in the observed crash.

I'm not sure how to fix this, the code that is involved is quite... complicated and I've not yet been able to completely understand it.

axel-habermaier avatar Feb 20 '22 18:02 axel-habermaier

After further experimentation, it is quite clear that React Router immediately wants to render app/routes/other.tsx (the second route in the repo case above), completely skipping rendering the first route. But the second route's loader hasn't been executed yet.

This can easily be seen by adding a console.log statements to loadRouteModule and by adding console.log("rendering", useMatches()) to the Routes function in components.tsx

Update

Actually, it's the browser that skips the first page. This can be seen by adding a console.log to the root document in a script tag.

So it seems that the app can be reached via "server-side" navigation where the Remix code expected it to be reachable via client-side navigation only. In other words: The SSR state is invalid for the route that is supposed to be shown by the client-side code and the code that is supposed to update the SSR state doesn't get executed.

So that would mean that before hydration, Remix should check whether it needs to update the SSR state, e.g., downloading additional JavaScript and updating the routeModules cache plus potentially other things that get updated that I'm not aware of.

@ryanflorence, @kentcdodds Any thoughts on what would be a good strategy to tackle this issue?

axel-habermaier avatar Feb 20 '22 18:02 axel-habermaier

I just faced the same issue. Did someone found a solution for that?

tomslutsky avatar May 08 '22 07:05 tomslutsky

A similar error occurred when I stupidly rendered <Scripts/> twice in root.tsx. Maybe this is helpful for someone.

import {
  Links,
  LiveReload,
  Meta,
  Outlet,
  Scripts,
  ScrollRestoration,
} from "@remix-run/react";

export default function App() {
  return (
    <html>
      <head>
        <Meta />
        <Links />
      </head>
      <body>
        <Outlet />
        <ScrollRestoration />
        <Scripts />
        <LiveReload />
        <Scripts /> {/* Duplicate! */}
      </body>
    </html>
  );
}

components:js:

Uncaught TypeError: Cannot destructure property 'default' of 'routeModules[id]' as it is undefined.
    at RemixRoute (components.js:179:14)
    at renderWithHooks (react-dom.development.js:14985:18)
    at updateFunctionComponent (react-dom.development.js:17356:20)
    at beginWork (react-dom.development.js:19063:16)
    at HTMLUnknownElement.callCallback2 (react-dom.development.js:3945:14)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:3994:16)
    at invokeGuardedCallback (react-dom.development.js:4056:31)
    at beginWork$1 (react-dom.development.js:23964:7)
    at performUnitOfWork (react-dom.development.js:22776:12)
    at workLoopSync (react-dom.development.js:22707:5)

The error occurs only on <Link/> navigation. On refresh the route loads fine.

fnebenfuehr avatar May 16 '22 09:05 fnebenfuehr

I'm also getting a components.js:176 Uncaught TypeError: Cannot destructure property 'default' of 'routeModules[id]' as it is undefined. at RemixRoute (components.js:176:14), but having a hard time telling when. Still investigating.

designbyadrian avatar May 25 '22 10:05 designbyadrian

@F1ow thank you so much, i had the same issue, and also rendered Scripts twice! The team could maybe add some errors, when used the Script tag more the once?

Manuelbaun avatar Jul 09 '22 19:07 Manuelbaun

I encountered same issue today with latest @remix-run/react

seyaobey-dev avatar Jul 31 '22 22:07 seyaobey-dev

Same. The error appears on the first hydration with a quick navigation to another page (in my case, a nested one).

synthlace avatar Sep 07 '22 13:09 synthlace

any update on this?

nabeelpkl avatar Nov 16 '22 09:11 nabeelpkl

I'm running into this as well.

A trivial repro is:

  1. clone the remix repository and create the default playground app.
  2. cd to playground/ start it
  3. open localhost:3000, in chrome set your network speed to fast 3g
  4. click the "signup" link
  5. click back twice, so you're on the default chrome page (no url loaded)
  6. click browser forward button once, and before entry.client.tsx loads, click forward again
  7. url should now be localhost:3000/join
  8. 💥 app crashes

hypothesis:

  1. routeModules is loaded from the the root route
  2. when entry.client.tsx finishes loading it tries to load the route module for /join which is not part of the routeModules map and it blows up

hypothetical fix:

  1. before running any client code, ensure the route module for the given route is loaded

n8agrin avatar Nov 23 '22 19:11 n8agrin

Running into the same issue caused by fast navigation between pages

elledienne avatar Nov 24 '22 14:11 elledienne

Any ideas? I don't have a duplicate <Script /> or another other top level tag. Occurred when I was showing off remix to my boss 😅

daxaxelrod avatar Jan 03 '23 20:01 daxaxelrod

Crafted a PR that demonstrates a repro case for this issue: https://github.com/remix-run/remix/pull/5166

cc/ @jacob-ebey

n8agrin avatar Jan 20 '23 23:01 n8agrin

I think I'm gonna call this the goldilocks bug. It's not just an issue of clicking too fast, or too slow. It seems to happen if you click forward, then wait just the right amount of time for the modules to start loading for index.tsx and then click the forward button a second time during the index chunk loads.

I could never reproduce it normally, but once I turned on network throttling and waited long enough to see the .js files start loading I could reproduce it.

I believe this is what's happening:

  • Forward click - Remix renders / (index route) on the server
    • Remix sends back the proper modules for entry-client and the index route
    • Network requests go out for entry-client and index
  • While they are loading, second forward click
    • popstate is ignored since we haven't finished loading the chunks yet and therefore haven't instantiated our client side router and it's popstate listener
    • Once the chunks load and go to hydrate, it attempts to hydrate /other since that's the current URL

A few off-the-cuff options:

  1. Easier fix - track the URL at page load and if it's not the same at hydration start (once JS chunks have loaded) we force a hard reload of /other
  2. Harder fix - same detection, but instead of a hard reload we use the manifest to kick off loads for the /other chunks and delay hydration until they complete. That introduces the case for the same issue on a third click, so that would have to do the check each time the new "initial" route chunks load.

I prefer 1 :)

brophdawg11 avatar Jan 23 '23 21:01 brophdawg11

@brophdawg11 any chance there is an update on this? We're still running into the issue which causes the app to arbitrarily crash:

image

n8agrin avatar Mar 02 '23 01:03 n8agrin

Added a PR with suggested fix here: https://github.com/remix-run/remix/pull/6409

n8agrin avatar May 16 '23 18:05 n8agrin

Having issues with this as well. Commenting to track

armando-herastang avatar May 23 '23 15:05 armando-herastang

This is fixed by #6409 and should be available once the next release goes out (should be 1.18.0)

brophdawg11 avatar Jun 14 '23 21:06 brophdawg11

🤖 Hello there,

We just published version v0.0.0-nightly-ad9adee-20230615 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

github-actions[bot] avatar Jun 15 '23 07:06 github-actions[bot]

🤖 Hello there,

We just published version v0.0.0-nightly-12440f3-20230616 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

github-actions[bot] avatar Jun 16 '23 07:06 github-actions[bot]

🤖 Hello there,

We just published version 1.18.0-pre.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

github-actions[bot] avatar Jun 21 '23 17:06 github-actions[bot]

🤖 Hello there,

We just published version 1.18.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

github-actions[bot] avatar Jun 26 '23 17:06 github-actions[bot]