search

nextauthjs / next-auth

Authentication for the Web.
https://authjs.dev
ISC License
24.9k stars 3.52k forks source link

@auth/kysely-adapter incompatibilities/pain points with kysely-libsql dialect #9039

Open danidb opened 1 year ago

danidb commented 1 year ago

Adapter type

@auth/kysely-adapter

Environment

System: OS: macOS 14.0 CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz Memory: 32.53 MB / 16.00 GB Shell: 5.9 - /bin/zsh Binaries: Node: 18.18.0 - ~/.nvm/versions/node/v18.18.0/bin/node npm: 10.2.0 - ~/.nvm/versions/node/v18.18.0/bin/npm pnpm: 8.6.10 - ~/Library/pnpm/pnpm Browsers: Chrome: 118.0.5993.117 Safari: 17.0 Safari Technology Preview: 17.4

Reproduction URL

https://github.com/danidb/next-auth/tree/kysely-adapter-libsql/packages/adapter-kysely

This fork+branch includes tests that exercise the adapter with the kysely-libsql (Turso) dialect. See below.

Describe the issue

TLDR (1); The Kysely adapter seems to be incompatible with the kysely-libsql dialect

With the kysely-libsql dialect, the return of .executeTakeFirst, .executeTakeFirstOrThrow is immutable (writable: false). This causes many operations (like createUser) to fail here:

https://github.com/nextauthjs/next-auth/blob/50ff32c4b113c2f2554351a19d3b05641dfd5d9c/packages/adapter-kysely/src/index.ts#L96

In the fork/branch linked as repro, I've expanded the tests to exercise the adapter with the kysely-libsql dialect, most of the new test cases will fail.

The built-in Kysely dialects return mutable objects from .executeTakeFirst/.executeTakeFirstOrThrow.

Despite (possible) first impressions, this doesn't seem like a problem with Kysely or the kysely-libsql dialect.

My understanding is that Kysely makes minimal assumptions (or demands) of the structure of query results. It's all up to the dialect. I asked about this chez Kysely, referencing the issue I've described above: https://www.answeroverflow.com/m/1169663688505049258. It seems like 'adapting' the adapter (pun intended) would be most appropriate.

I ended up handling this by just patching @auth/adapter-kysely and can contribute a quick-fix, but...

TLDR (2); It might be worth restructuring the adapter implementation for different Kysely dialects (similar to Drizzle for sqlite vs. mysql vs. postgres) to address this and some other pain points. If useful/wanted, will gladly contribute a PR.

The schema/migration used to setup the Kysely adapter is heavily dependent on the dialect.

8453 seems analogous to the situation I'm describing here but with another dialect.

For sqlite and libsql, the strategy for choosing IDs is also worth bringing up. For sqlite/libsql, my understanding is that this would need to be set up in the adapter implementation, not in the schema/migration. The tests for the adapter assume sequential integer ids for sqlite, the default, which might not always be appropriate. (e.g. I ended up patching-in cuids as an alternative.) The Drizzle adapter, for example, uses crypto.randomUUID to create user IDs

Given all of the above, it might be worth splitting up the Kysely adapter implementation for some dialects. These could either be internal to the one adpater (e.g. mysql vs sqlite vs postgres in the Drizzle adapter, probably the best approach) or be separate adapters altogether.

Will gladly contribute a PR to help develop the adapter further/address the(se) issue(s) and get it working with kysely-libsql, with whatever strategy seems most appropriate, if it would be useful/is wanted.

PS. Thank you all. Next-Auth has saved us a lot of time over the past few years. Cheers.

How to reproduce

Note that the (modified) test script will build a libsql Docker image (from https://github.com/tursodatabase/libsql ) the first time you run it. Compiling libsql can take a while. The mysql image might also take a while to start. It might be necessary to change the wait/sleep time in the test script.

Expected behavior

An adapter should work with Kysely + the kysely-libsql dialect, with minimal a fuss/no patches. (See above). Will gladly contribute to address this.

aradhya-tiwari commented 4 months ago

any update ?

Edit by maintainer bot: Comment was automatically minimized because it was considered unhelpful. (If you think this was by mistake, let us know). Please only comment if it adds context to the issue. If you want to express that you have the same problem, use the upvote 👍 on the issue description or subscribe to the issue for updates. Thanks!