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(providers): profile types #4202

Merged
merged 8 commits into from Apr 22, 2022
Merged

fix(providers): profile types #4202

merged 8 commits into from Apr 22, 2022

Conversation

ubbe-xyz
Copy link
Collaborator

Reasoning 💡

When reviewing this PR #4170 we noticed that the types in our provider files were incorrect.

When declaring the options for the provider profile, we were doing this:

export default function Discord<P extends Record<string, any> = DiscordProfile>(

but TS is not able to infer the types correctly as you can see:

Screenshot 2022-03-16 at 21 32 15

However changing it like this:

interface DiscordProfile extends Record<string, any> {
  // ...
}

export default function Discord<P extends DiscordProfile>(

enables TS to pick up the types correctly:

Screenshot 2022-03-16 at 21 32 00

I believe both syntaxes are more or less equivalent to what we want to achieve which is typing the profile object of the provider but allowing extra fields for user flexibility.

Checklist 🧢

  • Documentation
  • Tests
  • Ready to be merged

Affected issues 🎟

None

@vercel
Copy link

vercel bot commented Mar 16, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nextauthjs/next-auth/FyyGsKMSx6juN6X7qRw8qeR3EfcC
✅ Preview: https://next-auth-git-fix-providers-types-nextauthjs.vercel.app

[Deployment for 43bd01a canceled]

@github-actions github-actions bot added core Refers to `@auth/core` providers labels Mar 16, 2022
@@ -15,10 +15,11 @@ export interface WorkOSProfile {
email: string
lastName: string
firstName: string
picture: string
Copy link
Collaborator Author

@ubbe-xyz ubbe-xyz Mar 16, 2022

Choose a reason for hiding this comment

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

I'm adding this field as it was read optimistically in the below profile() function.

Now that the types work TS was complaining that it wasn't declared in the interface. I believe the contributor just forgot to do so 💭

Copy link
Member

@ThangHuuVu ThangHuuVu left a comment

Choose a reason for hiding this comment

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

LGTM! I think this would solve #3350 too. I wasn't sure how to solve that issue back then. Nice work! 👍

Copy link
Member

@ndom91 ndom91 left a comment

Choose a reason for hiding this comment

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

Yeah great catch!

@vercel vercel bot temporarily deployed to Preview April 10, 2022 17:37 Inactive
@vercel vercel bot temporarily deployed to Preview April 14, 2022 08:47 Inactive
@vercel
Copy link

vercel bot commented Apr 20, 2022

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

Name Status Preview Updated
next-auth ✅ Ready (Inspect) Visit Preview Apr 22, 2022 at 11:17AM (UTC)

@vercel vercel bot temporarily deployed to Preview April 20, 2022 12:30 Inactive
@vercel vercel bot temporarily deployed to Preview April 21, 2022 13:37 Inactive
@ubbe-xyz ubbe-xyz merged commit 8288ae5 into main Apr 22, 2022
@ubbe-xyz ubbe-xyz deleted the fix/providers-types branch April 22, 2022 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Refers to `@auth/core` providers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants