FirebaseExtended / reactfire

Hooks, Context Providers, and Components that make it easy to interact with Firebase.
https://firebaseopensource.com/projects/firebaseextended/reactfire/
MIT License
3.55k stars 407 forks source link

useSigninCheck suspends indefinitely #405

Open Zeyuzhao opened 3 years ago

Zeyuzhao commented 3 years ago

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.

jacomarcon commented 3 years ago

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.

jhuleatt commented 3 years ago

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.

Zeyuzhao commented 3 years ago

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?

steurt commented 3 years ago

@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 commented 3 years ago

@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.

jhuleatt commented 3 years ago

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

Zeyuzhao commented 3 years ago

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

seanaguinaga commented 1 year ago

This still happens with v4