Reputation: 88
I'm working on route authentication, and am storing the authed status in a context so that other React components can check if a user is logged in. The relevant part of code is:
const [loggedInUser, setLoggedInUser] = useState(null)
const authed = () => !!loggedInUser
useEffect(() => {
async function fetchData() {
const response = await fetch('/loggedInUser')
.then(res => res.json())
setLoggedInUser(response)
}
fetchData()
}, [])
The quick explanation of this code is, I need user data (such as id) in parts of code so I need to store the loggedInUser
object. however, for simpler tasks such as checking if a user is logged in, I'm using the function authed
to check if the loggedInUser
variable contains an object (user is logged in) or is null.
To check if a user is logged in, I'm using passport.js and my '/loggedInUser'
route looks like this:
app.get('/loggedInUser', async (req, res) => {
if (!req.isAuthenticated())
return null
const { id, name } = await req.user
.then(res => res.dataValues)
return res.json({ id, name })
})
The problem is, code consuming this context and checking authed()
is running before the useEffect()
and fetch()
are able to hit the GET route and use the API response to setLoggedInUser(response)
. so use of authed()
always returns false, even if the API response later sets loggedInUser
to some object value where now authed()
is true. obviously this is a race condition, but I'm not sure how to address it.
Is there an elegant solution where I can 'lock' authed()
from returning a value until the useEffect() has fully 'set up' the state of loggedInUser
?
One (awful) solution I'm envisioning may be something like:
const [loggedInUser, setLoggedInUser] = useState(null)
const isFinishedLoading = false // <--
function authed() {
while (!isFinishedLoading) {} // a crude lock
return !!loggedInUser
}
useEffect(() => {
async function fetchData() {
const response = await fetch('/loggedInUser')
.then(res => res.json())
setLoggedInUser(response)
isFinishedLoading = true // <--
}
fetchData()
}, [])
Is there a better way to 'lock' the function authed()
until loading is complete?
Edit:
To clarify my usage of authed()
for the comments, here is a trimmed down version of my App.js
export default function App() {
return (
<>
<AuthProvider>
<Router className="Router">
<ProtectedRoute path="/" component={SubmittalTable} />
<Login path="/login" />
</Router>
</AuthProvider>
</>
)
}
function ProtectedRoute({ component: Component, ...rest }){
const { authed } = useContext(AuthContext)
if (!authed()) // due to race condition, authed() is always false
return (<Redirect from="" to="login" noThrow />)
return (<Component {...rest} />)
}
Upvotes: 1
Views: 1251
Reputation: 6053
I don't understand why you need authed()
to be a function, and don't use isLoggedIn
directly. Also I don't see where you are setting the Context value, but anyway ...
Generally: In React, try to think about
In your case:
The user is authorized or is not authorized to use your app at any given moment. That is a state, and you would store this state somewhere. Let's call this state isAuthorized
.
You can store isAuthorized
in the Context, as long as you know it is available when you need it. If the Context is not available at the moment when you want to know if the user is authorized (which seems to be the case in your app), then you can not use the Context to store isAuthorized
(at least not alone).
You can fetch isAuthorized
every time when you need it. Then isAuthorized
is not available until the fetch responds. What is the state of your app right now ? It is notReady
(probably). You can store the state notReady
somewhere, e.g. again in the Context. (notReady
will be always initially true
, so you know the app is ready only if you explicitly say so.) The App might display a Spinner and do nothing else as long as it is notReady
.
You can store isAuthorized
in e.g. the browser storages (e.g. sessionStorage
). They are available across page loads, so you don't have to fetch the state every time. The browser storages are supposed to be synchronous, but indeed I would treat them as being asynchronous, because things I have read about the browser storages are not inspiring confidence.
What you are trying to do is to store isAuthorized
in the (1) Context AND (2) fetch it every time, so you have 2 states, which need to be synchronized. Anyway, you do need to fetch isAuthorized
at least once in the beginning, without that, the app is not ready to be used. So you do need synchronization and a state (3) notReady
(or isReady
).
Synchronizing state is done with useEffect
in React (or with 'dependencies'), e.g.:
useEffect(() => {
setIsFinishedLoading( false ); // state (3) "app is ready"
fetchData().then( response => {
setLoggedInUser( response ); // state (2) "isAuthorized" from fetch
setIsFinishedLoading( true );
}).catch( error => {
setLoggedInUser( null );
setIsFinishedLoading( true );
});
}, []);
useEffect(() => {
if( isFinishedLoading ){
setIsAuthorized( !!response ); // state (1) "isAuthorized" in local state or Context
}
}, [ isFinishedLoading, response ]);
You probably don't need it anyway, but:
Blocking code execution in that sense is not possible in Javascript. You would instead execute the code at some other time, e.g. using Promises. This again requires thinking in a slightly different way.
You can not do:
function authed(){
blockExecutionBasedOnCondition
return !!loggedInUser
}
But you can do:
function authed(){
return !!loggedInUser;
}
function executeAuthed(){
someConditionWithPromise.then( result => {
authed();
});
}
Upvotes: 1