MomenSherif
useGoogleLogin hook crashes if function is invoked before google client is loaded. "Cannot read properties of undefined (reading 'requestCode')"
Summary
Using this library to wire up my custom google login button. When flow is auth-code and I invoke the login function returned by useGoogleLogin before the google client is loaded, the workflow fails with the following error:
Cannot read properties of undefined (reading 'requestCode')
at ? (scratch/node_modules/@react-oauth/google/dist/index.esm.js:145:67
This is because the clientRef is undefined at invocation time. While I solved this by disabling my login button until the provider's onScriptLoadSuccess is called, I think this is an easy trap to fall in to and probably be called out in the docs.
Reproduction Steps:
- Use the following code Container:
<GoogleOAuthProvider clientId={config.googleClientId} >
<GoogleLoginButton />
</GoogleOAuthProvider>
Button:
export const GoogleLoginButton: React.VFC = () => {
const { googleLogin, googleLoginFailure } = {}; // API Method here
const login = useGoogleLogin({
onSuccess: (codeResponse) => googleLogin(codeResponse),
onError: (err) => googleLoginFailure(err),
flow: 'auth-code'
});
return (
<Button
className={styles.google_button}
type="secondary"
onClick={() => {
login();
}}
>
<img className={styles.google_icon} src={Icons.iconGoogle} alt="googleIcon" />
<span>Sign in with Google</span>
</Button>
);
};
- Open devtools, load the page
- Find request to
https://accounts.google.com/gsi/client, and block network request. - Reload the page, and try to use the button
- Button fails. This triggers the crash, but the same would apply if the button was clicked before the client is loaded.
Suggestions
As noted, I fixed this myself by handling the success/error state in the provider and dealing with my custom button accordingly. At the very least, I think the docs should be updated for Custom login button to indicate that success/failure should be handled.
I might also suggest adding some additional error handling to the hook so that it expects that clientRef is nullable. The any provided to the useRef hook hides the fact that clientRef.current is nullable. If this were changed to be a thin typing wrapper of the Google client, with strict mode set that would likely highlight any potential cases where clientRef.current is undefined. The library would then have to handle these cases gracefully. For instance, throwing a more meaningful error that clientRef is not defined in loginImplicitFlow and loginAuthCodeFlow.
I'm more than happy to submit a PR here if you're keen on any of the above suggestions.
I think we shouldn't throw an error it will be the same issue, we can just optional chaining for the method after the clientRef.current to be safe
I think we shouldn't throw an error it will be the same issue, we can just optional chaining for the method after the clientRef.current to be safe
Yea that should be fine. I would suggest a stronger wording in the docs too, as clicking the button will still result in a no-op and could lead to unexpected errors.
Out of curiosity, can the context support a loading flag, so that custom buttons can handle their own loading state?
If window.google is undefined, it's not going to load the code. I disable the button if it is undefined. @taylor-tugboat
@MomenSherif I believe this issue is not really fixed, currently it is still hard crashing at this line though there was not check added specifically to either check for request code or do optional chaining
@MomenSherif I believe this issue is not really fixed, currently it is still hard crashing at this line though there was not check added specifically to either check for request code or do optional chaining
@MomenSherif I can confirm this is happening on a private window with Brave browser
