Skip to content

fix: signIn infer provider type#4623

Merged
balazsorban44 merged 1 commit intonextauthjs:mainfrom
ArthurPedroti:main
May 31, 2022
Merged

fix: signIn infer provider type#4623
balazsorban44 merged 1 commit intonextauthjs:mainfrom
ArthurPedroti:main

Conversation

@ArthurPedroti
Copy link
Copy Markdown
Contributor

@ArthurPedroti ArthurPedroti commented May 26, 2022

The "P" type it's not passed in any props, so the result type doesn't understand and returns the "false type" always, Adding the "P" at provider type props.

My local test implementation:

// next-auth.d.ts
import 'next-auth'
import {
  BuiltInProviderType,
  RedirectableProviderType
} from 'next-auth/providers'
import {
  LiteralUnion,
  SignInAuthorizationParams,
  SignInOptions,
  SignInResponse
} from 'next-auth/react'

declare module 'next-auth/react' {
  export * from 'next-auth/react'

  export declare function signIn<
    P extends RedirectableProviderType | undefined = undefined
  >(
    provider?: LiteralUnion<P | BuiltInProviderType>,
    options?: SignInOptions,
    authorizationParams?: SignInAuthorizationParams
  ): Promise<
    P extends RedirectableProviderType ? SignInResponse | undefined : undefined
  >
}

before:
image

after:
image

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

The "P" type it's not passed in any props, so the result type doesn't understand and return the false type always, Adding the "P" at provider type props.
@vercel
Copy link
Copy Markdown

vercel bot commented May 26, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
next-auth ⬜️ Ignored (Inspect) May 26, 2022 at 1:39PM (UTC)

@github-actions github-actions bot added client Client related code core Refers to `@auth/core` labels May 26, 2022
Copy link
Copy Markdown
Member

@balazsorban44 balazsorban44 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Do you this fixes the issue #4513?

@balazsorban44 balazsorban44 merged commit 46089eb into nextauthjs:main May 31, 2022
@balazsorban44
Copy link
Copy Markdown
Member

@ArthurPedroti this, unfortunately, broke tests: https://github.com/nextauthjs/next-auth/runs/6674906912?check_suite_focus=true so I have to revert it. Feel free to have another look.

balazsorban44 added a commit that referenced this pull request May 31, 2022
balazsorban44 added a commit that referenced this pull request May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

client Client related code core Refers to `@auth/core`

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants