Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: signIn infer provider type #4623

Merged
merged 1 commit into from May 31, 2022
Merged

fix: signIn infer provider type #4623

merged 1 commit into from May 31, 2022

Conversation

ArthurPedroti
Copy link
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

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
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
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.

None yet

2 participants