search

nextauthjs / next-auth

Authentication for the Web.
https://authjs.dev
ISC License
24.1k stars 3.34k forks source link

`signIn` with `RedirectableProviderType` type: `response` is possibly `undefined` #4513

Open b-novikov-ipersonality opened 2 years ago

b-novikov-ipersonality commented 2 years ago

Environment

System:
    OS: Linux 5.17 EndeavourOS
    CPU: (12) x64 AMD Ryzen 5 4600H with Radeon Graphics
    Memory: 5.85 GB / 15.07 GB
    Container: Yes
    Shell: 5.8.1 - /bin/zsh
Binaries:
    Node: 16.14.2 - ~/.nvm/versions/node/v16.14.2/bin/node
    Yarn: 1.22.18 - ~/.nvm/versions/node/v16.14.2/bin/yarn
    npm: 8.7.0 - ~/.nvm/versions/node/v16.14.2/bin/npm
npmPackages:
    next: ^12.1.5 => 12.1.5 
    next-auth: ^4.3.4 => 4.3.4 
    react: ^18.1.0 => 18.1.0

Reproduction URL

https://codesandbox.io/s/objective-mendel-ixpsco?file=/src/App.tsx

Describe the issue

According to documentation when you supply email or credentials provider to signIn function it returns promise of type SignInResponse but in fact return type of the signIn function is Promise<SignInResponse | undefined>.

This typing results in typescript error Object is possibly 'undefined' if you don't check if response exists beforehand.

How to reproduce

  1. Specify signIn type to be RedirectableProviderType
  2. Chain then to function call
  3. Check for response.error
  4. See typescript error

Expected behavior

When specifying signIn type to be one of RedirectableProviderType the return type should be Promise<SignInResponse> and not Promise<SignInResponse | undefined>

balazsorban44 commented 2 years ago

The return type can be undefined (and it is by default):

https://github.com/nextauthjs/next-auth/blob/641d917175026ec974d1d255064b2591db0c56e6/packages/next-auth/src/react/index.tsx#L227-L233

b-novikov-ipersonality commented 2 years ago

@balazsorban44 with redirect: false option it still warns that response is possibly undefined.

ArthurPedroti commented 2 years ago

The fact of redirect is false doesn't infer the response can be undefined, it's only used to show a use case of this feature.

But I agree with you, the docs were not clear about the response can be undefined, I think it's good to improve and make it clear.

If you want a simple solution to this problem, only use optional chaining on the response.