Migrate faunadb adapter to TS

[johannes-scharlach]

Amazing to see how easy it was to use this adapter! Since it's pretty small, I figured it would make sense to actually move the file to TypeScript directly rather than adding a type declarations file.

There is one breaking change which I wanted to introduce this early: The default export is now the FaunaAdapter directly, which is in line with the PrismaAdapter. Previously this was an object with a .Adapter property.

[willllhallll]

Hi @johannes-scharlach! This is great stuff, love to see the Fauna adapter evolving.

I have opened up a PR over on your fork with some proposed changes to the Fauna adapter. Would love to hear your thoughts, and hopefully we can work on it and merge into this PR before review.

johannes-scharlach#1

Cheers, Will.

[balazsorban44]

Just an update here. First, thank you for this PR! We are finally getting this repo in shape and can move forward with it.

@lluia and/or I am going to see how we could improve the now built-in Adapter interface for a much nicer experience when adding custom adapters!

This is the current location: https://github.com/nextauthjs/next-auth/blob/main/types/adapters.d.ts

My idea is to be able do something like:

import type { Adapter } from "next-auth/adapters"
const FaunaAdapter: Adapter<FaunaConfig, ???> = ... // Pass any generics optionally, if needed. Have to look at the current implementation to get a sense of what is useful here

export default FaunaAdapter

So you could just pass some generics to Adapter, and all the required methods downwards would give you type hints. I'll try to report back here when it is available!

[ndom91]

Hey great work, what do we need to do to move this forward?

Are you interested in merging the changes in your fork back into this repo, @johannes-scharlach ?

[balazsorban44]

@ndom91 I wish that we wait with this until a proper Adapter interface is exposed from next-auth/adapters to type this correctly. I'll ping this thread when we have something

[balazsorban44]

@johannes-scharlach We now have an Adapter interface that can be of great help in this migration!

Have a look at the Prisma adapter which I just migrated, without a single as or any type of cheat. A single interface defined at the top, everything is typed downwards 🎉

https://github.com/nextauthjs/adapters/blob/fe59f6b4b006203b7c70f8664a54c148515aed05/packages/prisma/src/index.ts

This is a diff of all it took to type the Prisma adapter.

[willllhallll]

Only just seen this, great news!

I fear this PR may have become stale. If there is no word from @johannes-scharlach within the week, maybe myself and others could consider a new PR which takes into account the great new Adapter interface in #72 and also the sweeping error handling changes from #75.

I would be happy to make the first move on this.

CC @pbteja1998 @n44ps

[balazsorban44]

Sounds good to me. There are minor issues with the new Adapter interface still, but I have a solution in mind, and hopefully, they will be live within a week.

[pbteja1998]

@WillTheVideoMan @balazsorban44 The new adapter interface looks 💥. Can't wait to remove the local adapter code from all my projects and directly use this one.

Please let me know how I can help with this.

[n44ps]

Super nice, as I said to @WillTheVideoMan, I also have a few changes to make :

• Added the possibilities to set default values when creating User / Session / ..., important for custom models.
• accessToken uses the Tokens from Fauna to enable ABAC and custom roles while trying to query over Fauna.
• Updated the logic for Dates, using FQL which is for me easier to read than getTime(... + getTime())
• Added ttl to delete Sessions directly from Fauna if expired, because it was still here even if expired.

As soon as the goes forward, I will append those changes :)

[pbteja1998]

One small thing I want to add:
Rename verification_request_by_token index to verification_request_by_token_and_identifier.

[willllhallll]

Excellent to see so many collaborators here! I will await Monday 10th of May to open a new PR which includes these changes and the upcoming adapter interface fix.

@n44ps and @pbteja1998, let us then discuss on the new PR all the interesting and exciting changes and tweaks we can make to ensure the Fauna adapter is first-class!

See you all soon.

[johannes-scharlach]

All of these comments make a lot of sense. In my own case I'm using GraphQL to query faunadb and there is one problem with how the adapter is currently set up: An account has a userId as a string, rather than a user reference to the users collection.

It would be a breaking change, but I'd love to adapt it to this GraphQL schema

type User @collection(name: "users") {
  name: String
  email: String @unique(index: "user_by_email")
  image: String
  createdAt: Date
  updatedAt: Date
  accounts: [Account] @relation
  sessions: [Session] @relation
}

type Account @collection(name: "accounts") {
  user: User
  providerId: String @unique(index: "account_by_provider_account_id")
  providerType: String @unique(index: "account_by_provider_account_id")
  providerAccountId: String
  refreshToken: String
  accessToken: String
  createdAt: Date
  updatedAt: Date
}

type Session @collection(name: "sessions") {
  user: User
  expires: Date
  sessionToken: String @unique(index: "session_by_token")
  accessToken: String
  createdAt: Date
  updatedAt: Date
}

type VerificationRequest @collection(name: "verification_requests") {
  identifier: String
  token: String @unique(index: "verification_request_by_token")
  expires: Date
  createdAt: Date
  updatedAt: Date
}

type Query {
  userByEmail(email: String): User @index(name: "user_by_email")

  accountByProviderAccountId(
    providerId: String
    providerAccountId: String
  ): Account @index(name: "account_by_provider_account_id")

  sessionByToken(sessionToken: String): Session @index(name: "session_by_token")

  verificationRequestByToken(token: String): VerificationRequest
    @index(name: "verification_request_by_token")
}

Would this be the right time to also make such breaking changes? I'm concerned if I'm not able to use the adapter in my own application, my capacity to contribute will be close to 0. But I'm also happy for someone else to take over :)

[balazsorban44]

We are still at an early phase and am happy to accept breaking changes. Unless we could somehow make this an option?

An adapter can take a second parameter which is an options object. could that be somehow utilized to support different cases? We COULD create multiple adapters with different implementations for the same db, but it might be confusing for users.

I'm not using FaunaDB so I'll let those involved in this thread decide, I have no strong opinions here.

[willllhallll]

Just a ping to say that work has begun on the updated adapter, but perhaps it was more of a challenge than I had first anticipated. No matter how you interact with Fauna, through FQL or GraphQL, underneath it is fundamentally just an HTTP API which returns a certain shape which does not necessarily map directly to next-auth object shapes. So, to get them to play nicely, there is some interesting reshaping that must happen (see reshape). This ultimately means there must be also be subtly different type interfaces (see User vs a FaunaUser). Since Fauna is a little more loose than Prisma in the realm of schemas and structure, Fauna cannot itself provide types for the collections it queries. So all type definitions must be provided.

There are also some quirks regarding dates, since Fauna's returned Time objects (see type FaunaTime) must be transformed back and forth between JS dates.

Another quirk is that if a Fauna document does not exist, unlike Prisma that might return null, Fauna will throw. So the FQL structure become a little more complex (see fqlTemplates) to enable the object-or-null return pattern of methods such as getSession.

So, all in all, I may need just a couple extra days before a new PR can go ahead. I want to make sure that this is sensible and as DRY as possible, so I will take the extra time to make it good. Also need to finish writing up the tests. As the single index.ts file grows in size, I will also consider splitting it down.

You can peek the progress below:

https://github.com/WillTheVideoMan/adapters/blob/update-fauna-with-ts/packages/fauna/src/index.ts

Still lots of work to be done, but at least it has begun! I would greatly appreciate your thoughts at this stage.

[willllhallll]

@johannes-scharlach Regarding the GraphQL schema types you proposed, I think that could end up also becoming a larger breaking change within the core too, since especially in the Session type, there are direct references to the userId property e.g: https://github.com/nextauthjs/next-auth/blob/86baefdd9df9246669d4f788aaab4774605c256a/src/server/lib/callback-handler.js#L81-L83

However, I don't use GraphQL with Fauna, so I might be missing something big. To me, however, it feels like once the adapter is implemented it should be as opaque as possible to the core, so if one chooses to use GraphQL in their application it shouldn't matter if GraphQL or FQL is used in the adapter. It should just work.

Would appreciate your further thoughts.

[johannes-scharlach]

@WillTheVideoMan I still believe that userId should be exposed on the public types, once the reshape is done. This is about storing it as a ref rather than as a string.

E.g. inserting a new record for accounts would be

q.Create(q.Collection(collections.Account), {
  data: {
    user: q.Ref(q.Collection(collections.User), userId),
    ...accountData,
  },
})

instead of

q.Create(q.Collection(collections.Account), {
  data: {
    userId,
    ...accountData,
  },
})

which makes a huge difference for graphql because you can do

query {
  accountByProviderAccountId(providerId: "google", providerAccountId: "123") {
    user {
      _id
      email
    }
  }
}

all of a sudden, which are two things you cannot get in one query if the userId is saved as a string, rather than as a ref.

I also believe it's semantically correct to store relationships as refs rather than as ids.

My FQL knowledge isn't good enough to give more thorough examples right now, so I hope this makes sense :)

[willllhallll]

@johannes-scharlach My apologies, I fully misunderstood your previous comment! I see now that this is a Fauna-specific internal change, and that the reshaping to fit the next-auth types happens outside this scope.

From an FQL perspective, there is no change. From an adapter perspective, the only great change would be to extract the user id from the user field within the reshaper. This is probably easy, and can be typed with a new interface something like FaunaRef which would contain an array of values, the last of which would be the user id in the guise of that same user's ref e.g.

{
  ref: Ref(Collection("session"), "297938648561615367"),
  ts: 1620395418110000,
  data: {
    user: Ref(Collection("users"), "294622260513210880")
  }
}

Would become:

session: {
    id: 297938648561615367,
    userId: 294622260513210880
  }

Are there any other places you can see where this same pattern could be used? Totally agree on the change in semantics, and the clear benefit to all Fauna users, not just GraphQL. In this way, we keep it simple as we probably don't need a flag or separate Fauna-GraphQL adapter implementation either.

Thanks so much for bearing with my misunderstanding. Will put this to work straight away. Cheers!

[johannes-scharlach]

@WillTheVideoMan I only see it in the user-account relationship and the user-session relationship.

[willllhallll]

Apologies for the delay on this, work has caught up with me recently! You can keep tabs on the progress over on my branch. More to follow soon!

[bring-shrubbery]

@balazsorban44 Any idea on when will this be merged?

[n44ps]

Any news on this @WillTheVideoMan ? I would love to help :)

[willllhallll]

My open-source colleagues - I can only apologise. How quickly the real world of work catches up with you, and without even blinking, months go by...

Indeed, it has been a long time since I have had the opportunity to look at this, so it is likely that lots has changed since we last spoke about this Fauna implementation. Originally, I could justify spending time on this project as we used NextAuth and Fauna within a work project. However, that project has been put on hiatus in pursuit of other work (for context, I work at a developmental biology lab, so web development for us is not a fundamental part of our operations).

Given this, and my ongoing and ever-increasing work commitments, I must unfortunately say that - for the time being at least - I no-longer have the time to finish my adapter implementation, which is a great shame because a first-class Fauna Adapter for NextAuth would be superb.

I leave you then with my progress so far, my apologies for dropping off the face of the earth, and the best of luck. Thank you for your patience.

https://github.com/WillTheVideoMan/adapters/blob/update-fauna-with-ts/packages/fauna/src/index.ts

[balazsorban44]

There is a big refractor/simplification of the Adapter API on its way nextauthjs/next-auth#2361, so I'm going to close this for now.

@ndom91 and I will probably add TS support very soon. 😊

[balazsorban44]

Anyone still following, I have opened a new PR that also adds TS support: #182