reactfire icon indicating copy to clipboard operation
reactfire copied to clipboard

useUser returns value null within AuthCheck children after signing out

Open andrei-yanovich opened this issue 5 years ago • 5 comments

Reopening issue #155 , since AuthCheck is not entirely working.

Version info

React: 16.12

Firebase: 7.14

ReactFire: 2.0.3

Test case

https://codesandbox.io/s/pensive-fire-2v5qn?file=/src/config.js

Steps to reproduce

  • Use a private component (App) as a child of AuthCheck and Login as a fallback
  • Login using the form, App component with the user email should be rendered
  • reload the page
  • press sign out

Expected behavior

All children of AuthCheck shouldn't be rendered if there is no user logged in

Actual behavior

App component render fails with Cannot read property 'email' of null, because of useUser returning null. Happens only if an application is loaded with an active session.

andrei-yanovich avatar Apr 13 '20 19:04 andrei-yanovich

Thanks for reporting, @andrei-yanovich! We have a test to specifically guard against this, so I will revisit this test and see why it isn't catching this case: https://github.com/FirebaseExtended/reactfire/blob/21b6702f36ac6e06a29e74585278d0e3a10d36cc/reactfire/auth/auth.test.tsx#L128

jhuleatt avatar May 28 '20 17:05 jhuleatt

And this test specifically tests the "loaded with an active session" case:

https://github.com/FirebaseExtended/reactfire/blob/21b6702f36ac6e06a29e74585278d0e3a10d36cc/reactfire/auth/auth.test.tsx#L103

jhuleatt avatar May 28 '20 18:05 jhuleatt

Probably has to do with our mocking. The Auth Emulator can't come soon enough!

Still investigating.

jhuleatt avatar May 28 '20 18:05 jhuleatt

@jamesdaniels has been looking into similar Auth issues in AngularFire

jhuleatt avatar Jun 04 '20 15:06 jhuleatt

I faced the same issue yesterday. My guess is that the problem is in how the return value of the useUser hook is calculated. Usually, things like auth, currentUser, theme, etc. are passed through context, so when something changes react tree reacts appropriately, rebuilding stuff from top to bottom.

In this case, user value doesn't come from the context, so when it changes, it's then just being sent to the subscribers, instead of propagating through the react tree. Kind of a race condition.

For instance, if we declare a middleware like this

import React, { createContext, useContext, useEffect, useState } from 'react';
import { useUser } from 'reactfire';

const UserContext = createContext()

export const UserProvider = (props) => {
  const [user, setUser] = useState();
  const u = useUser();
  
  // I had to use JSON.stringify because useUser returns new instance on every call,
  // triggering an infinite update loop. It's bad and should be changed either.
  useEffect(() => setUser(u), [JSON.stringify(u)]) 

  return (
    <UserContext.Provider value={user}>
      {props.children}
    </UserContext.Provider>
  )
}

export const useUser2 = () => useContext(UserContext);

then the wrapper like this (maybe somewhere inside the <AuthCheck />?) would work as we expect it to work:

function InnerWrapper(props) {
  const u = useUser2()
  return u?.data ? props.children : null
}

That's pretty much what I was able to find out, hope this helps.

maxprilutskiy avatar Dec 30 '20 16:12 maxprilutskiy