next-auth icon indicating copy to clipboard operation
next-auth copied to clipboard

fix(ts): match TS types better with implementation

Open balazsorban44 opened this issue 2 years ago • 2 comments

Historically, we have been lying in some places about the public interface to simplify the user-facing types, but this gave false security. This PR attempts to remove many // @ts-expect-error comments which have uncovered these mistakes.

Notable changes:

  • Account is re-exported as AdapterAccount from next-auth/adapters for convenience
  • There have been some TS issues on adapters because of how Omit<User, "id"> obscured the underlying User interface when it extended Record<string, unknown>. Locally it seems like this was a bug in TS, and is no longer needed, but this will need another pair of eyes. to verify :eyes:. (Direct solution to part of the below-mentioned tweet :crossed_fingers:)
  • When using the Email provider, we should be failing fast (i.e. in dev) when required methods aren't present.
  • AdapterSession has required id, although in practice we never used it, so this PR removes it from the default interface (module augmentation will still let the user define it if needed, but we use our own sessionToken anyway to not leak the DB through the session cookie)
  • TIL: Prisma @@id, so we don't need to have explicit ids. (MongoDB still requires it, so we need the ugly delete foo.id things :disappointed: )
  • Anything else, WIP

Some of these changes are inspired by this report from Twitter: https://twitter.com/_clem/status/1505305187614920709

~(Might split this PR into multiple ones.)~ Reverted any unrelated change so it should be slightly easier to review. :pray:

TODO: Add tests!

balazsorban44 avatar Jul 16 '22 18:07 balazsorban44

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

1 Ignored Deployment
Name Status Preview Updated
next-auth ⬜️ Ignored (Inspect) Oct 9, 2022 at 2:45PM (UTC)

vercel[bot] avatar Jul 16 '22 18:07 vercel[bot]

I committed a few more improvements:

  • [x] refactored Adapter interface -> type, got rid of the "verified in assertConfig" comments & ts-expected-error
  • [x] improved parseProvider to get rid more ts-expected-error. Added InternalOAuthConfig
  • [x] Added some tests for new fail fast logic

ThangHuuVu avatar Sep 10 '22 08:09 ThangHuuVu

So, if I understand this correctly, email is not nullable anymore?

Prisma schema example on the doc still shows email String? @unique.

Still, the Facebook provider doesn't save an email to the database, so I feel like this is an incorrect typing?

baptisteArno avatar Oct 16 '22 08:10 baptisteArno

Running into the same issue as @baptisteArno above. Shouldn't we be allowed to have a null email if we're using a non-email provider? e.g. I want to build a custom provider & adapter for logging in via SMS.

longlostnick avatar Oct 19 '22 18:10 longlostnick