CSFrequency / react-firebase-hooks

React Hooks for Firebase.
Apache License 2.0
3.55k stars 304 forks source link

useHttpsCallback should reject when the call fails #321

Open KaidenR opened 5 months ago

KaidenR commented 5 months ago

I'm not sure if this was the intended behavior, but useHttpsCallable's callCallable function returns a Promise<HttpsCallableResult<ResponseData> | undefined> which is being used to say "if the call succeeds return the expected ResponseData shape. But if it fails, STILL resolve the promise but with undefined.

I'd like to suggest that it should reject with the error if the call fails. That way your calling code can handle the err response the same way it handles the success data. Right now you have to write separate code that watches for changes to the hook's returned error object.

Specifically I'm trying to advance the user to the next page if the call succeeds, or call some logging and notification functionality if it fails. I'd like to be able to do this:

const [joinTeam, isJoiningTeam] = useHttpsCallable<Req, Res>(functions, 'jointeam')()

const handleSubmitClick = async () => {
  try {
    await joinTeam()
    navigate('/team-page')
  } catch (err) {
    logger.error('Error joining team', err)
    notificationContext.showErrorNotification({ title: 'Unable to join team', message: err.userFriendlyMessage })
  }
}

Since the promise returned by joinTeam currently doesn't ever reject, I can't handle the error this way.

It looks like all we'd have to do is to re-throw the error after updating the state here: https://github.com/CSFrequency/react-firebase-hooks/blob/09bf06b28c82b4c3c1beabb1b32a8007232ed045/functions/useHttpsCallable.ts#L41

I can look into submitting a PR but I'd like to get everyone's opinions on the change first since it's technically an API change.

Thanks for the great library!