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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: signIn infer provider type #4679

Merged
merged 3 commits into from Jun 23, 2022
Merged

fix: signIn infer provider type #4679

merged 3 commits into from Jun 23, 2022

Conversation

ArthurPedroti
Copy link
Contributor

@ArthurPedroti ArthurPedroti commented Jun 6, 2022

☕️ Reasoning

Re-opening #4623

Sorry, I really don't saw this string validation, I put a ternary validation to check it's really a RedirectableProviderType, and if is, it's will pass the "P" at the provider props. I checked and the build it's passing now.

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

ArthurPedroti and others added 3 commits May 26, 2022 10:36
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 Jun 6, 2022

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

1 Ignored Deployment
Name Status Preview Updated
next-auth ⬜️ Ignored (Inspect) Jun 6, 2022 at 10:37PM (UTC)

@github-actions github-actions bot added client Client related code core Refers to `@auth/core` labels Jun 6, 2022
@ArthurPedroti
Copy link
Contributor Author

ArthurPedroti commented Jun 6, 2022

@balazsorban44 Commenting the issue #4513 that you have mentioned, I think this is exactly what you said, the response of signIn really can be "undefined", and this PR doesn't change that.

But I think will be interesting to improve the docs to be more understandable these possibly at it, because the way that is now, it's really confusing, because it affirms the responses is "SignInResponse" type and not "SignInResponse | undefined"

If you think it's a good idea, I can do this little improvement on the docs and PR too.

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!

@balazsorban44 balazsorban44 merged commit e03e234 into nextauthjs:main Jun 23, 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