reactfire icon indicating copy to clipboard operation
reactfire copied to clipboard

useSigninCheck suspends indefinitely

Open Zeyuzhao opened this issue 4 years ago • 8 comments

I would like to create a protected route with reactfire and react-router-dom. To achieve this, I created a PrivateRoute component as follows:

export default function PrivateRoute(routeProps: RouteProps) {
  const { status, data: signInCheckResult, error } = useSigninCheck();

  if (signInCheckResult.signedIn) {
    return <Route {...routeProps} />;
  } else {
    return (
      <Redirect
        to={{ pathname: "/signin", state: { from: routeProps.path } }}
      />
    );
  }
}

I enabled suspense mode for reactfire. Oddly, when staying on a single page for around ~1min, navigating to another page would cause the component to suspend indefinitely. Reloading the page, however, does momentarily unsuspend the page (but the effect would reappear after idle).

Version info

"firebase": "8.7.1",
"react": "17.0.2",
"react-dom": "17.0.2",
"react-router-dom": "5.2.0",
"react-scripts": "4.0.0",
"reactfire": "3.0.0-rc.2"

Test case

https://codesandbox.io/s/reactfire-suspend-demo-i3j1e

Steps to reproduce

  1. Click on Sign in with Google and proceed.
  2. You should land in a signed in page with a log out button and a link to test page.
  3. Wait for at least 30 seconds (idle).
  4. Click on Link to test page (React Router Link)
  5. Page would suspend indefinitely.

Expected behavior

Page shouldn't suspend indefinitely.

Actual behavior

Page suspends indefinitely.

Please let me know if there are any good implementations for integrating reactfire with react-router-dom.

Zeyuzhao avatar Jul 19 '21 16:07 Zeyuzhao

Having the same issue of strange random suspense. Reverting to version 3.0.0-rc.0 fixed the issue, appears to be introduced in one of the last 2 release candidates.

jacomarcon avatar Jul 20 '21 20:07 jacomarcon

Thanks for the detailed repro, @Zeyuzhao. Does the same issue still occur if you use a version of React that supports concurrent mode (react@experimental and react-dom@experimental)? React 17 doesn't officially support Suspense outside of the codesplitting use case.

As a side note: we label ReactFire's concurrent mode features experimental, and given the recent updates to concurrent mode in React 18 (especially the fact that Suspense for data fetching is still a long way off), I don't recommend relying on them at this time.

jhuleatt avatar Jul 21 '21 14:07 jhuleatt

I created a new fork that uses React 18 (with the new ReactDOM.createRoot(rootNode).render(<App />); that supports concurrent mode), and the problem still persists.

Fork: https://codesandbox.io/s/reactfire-suspend-q5wll

I tried other hooks, such as useFirestore(), and also suspends indefinitely (wait for a minute, click on the link twice).

Looking at the source, it seems that some of the reactfire hooks are powered by preloadObservable, which uses SuspenseSubject. There is a parameter for SuspenseSubject to timeout - which seems suspect as this indefinite suspense triggers after a couple seconds.

Also, I haven't debugged the map extensively, but is it possible for some the id's used by preloadObservable to collide?

Zeyuzhao avatar Jul 21 '21 21:07 Zeyuzhao

@Zeyuzhao I've had indefinite suspending issues as well with v3.0.0-rc.*. To me it still isn't very clear where my issues originate, however I did find a way to get it working. I don't know if you're running in to the same issue as me, but you could give it a try.

If in my App component I preload every Firebase SDK I use, the indefinite loading issues are resolved. I preload the SDK's in the same way as in the preloadSDKs of the example with suspense: https://github.com/FirebaseExtended/reactfire/blob/33ce2500e51abdb0ace885373903b4ff16f51cf0/example/withSuspense/App.tsx#L79

I'm not sure why yet, but this solves my issues. Maybe it will work for you as well?

steurt avatar Jul 22 '21 12:07 steurt

@Zeyuzhao I just tried it quickly in your CodeSandbox repro, and it seems to solve the issue in the repro as well! No more indefinite suspending.

At the top of App.tsx add:

const preloadSDKs = (firebaseApp) => {
  return Promise.all([
    preloadFirestore({ firebaseApp }),
    preloadAuth({ firebaseApp })
  ]);
};

At the top of the App component in App.tsx put:

export default function App() {
  const firebaseApp = useFirebaseApp();

  // Kick off fetches for SDKs and data that
  // we know our components will eventually need.
  preloadSDKs(firebaseApp);
  
  ...
}

@jhuleatt loading the SDKs using the regular hooks without preloading them seems to cause the indefinite suspending.

steurt avatar Jul 22 '21 12:07 steurt

Very interesting, thank you for investigating this, @steurt!

jhuleatt avatar Jul 22 '21 16:07 jhuleatt

@steurt Thanks for the preloading tip. After adding preloading, I haven't seen any infinite suspense issues.

Zeyuzhao avatar Jul 22 '21 16:07 Zeyuzhao

This still happens with v4

seanaguinaga avatar Apr 11 '23 17:04 seanaguinaga