feat: support node:sqlite#3869
Conversation
|
@himself65 is attempting to deploy a commit to the better-auth Team on Vercel. A member of the Team first needs to authorize it. |
better-auth
@better-auth/cli
@better-auth/expo
@better-auth/sso
@better-auth/stripe
commit: |
There was a problem hiding this comment.
cubic analysis
5 issues found across 12 files • Review in cubic
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| isAutoIncrementing: col.name === autoIncrementCol, | ||
| hasDefaultValue: col.dflt_value != null, | ||
| })), | ||
| isView: true, |
There was a problem hiding this comment.
Every table is incorrectly flagged as a view (isView: true) instead of being detected dynamically or defaulting to false.
Prompt for AI agents
Address the following comment on packages/better-auth/src/adapters/kysely-adapter/node-sqlite-dlalect.ts at line 255:
<comment>Every table is incorrectly flagged as a view (isView: true) instead of being detected dynamically or defaulting to false.</comment>
<file context>
@@ -0,0 +1,300 @@
+/**
+ * @see {@link https://nodejs.org/api/sqlite.html} - Node.js SQLite API documentation
+ */
+import {
+ Kysely,
+ CompiledQuery,
+ DEFAULT_MIGRATION_LOCK_TABLE,
+ DEFAULT_MIGRATION_TABLE,
+ sql,
</file context>
| isView: true, | |
| isView: false, |
| if ("fileControl" in db) { | ||
| return "sqlite"; | ||
| } | ||
| if ("open" in db && "close" in db && "prepare" in db) { |
There was a problem hiding this comment.
Relying on the presence of an "open" method to detect a node:sqlite Database instance is unreliable – the DatabaseSync object returned by node:sqlite exposes close and prepare, but not open. Consequently, this condition will almost never be satisfied for real database objects, so the adapter will not be selected even when node:sqlite is actually used.
Prompt for AI agents
Address the following comment on packages/better-auth/src/adapters/kysely-adapter/dialect.ts at line 47:
<comment>Relying on the presence of an "open" method to detect a node:sqlite Database instance is unreliable – the DatabaseSync object returned by node:sqlite exposes `close` and `prepare`, but *not* `open`. Consequently, this condition will almost never be satisfied for real database objects, so the adapter will not be selected even when node:sqlite is actually used.</comment>
<file context>
@@ -44,6 +44,9 @@ function getDatabaseType(
if ("fileControl" in db) {
return "sqlite";
}
+ if ("open" in db && "close" in db && "prepare" in db) {
+ return "sqlite";
+ }
</file context>
packages/better-auth/src/adapters/kysely-adapter/test/normal/node-sqlite-dialect.test.ts
Outdated
Show resolved
Hide resolved
| }); | ||
| }); | ||
|
|
||
| describe("better-auth adapter integration", async () => { |
There was a problem hiding this comment.
Async function passed to describe can cause unpredictable test ordering and race conditions; describe callbacks should be synchronous
Prompt for AI agents
Address the following comment on packages/better-auth/src/adapters/kysely-adapter/test/normal/node-sqlite-dialect.test.ts at line 196:
<comment>Async function passed to describe can cause unpredictable test ordering and race conditions; describe callbacks should be synchronous</comment>
<file context>
@@ -0,0 +1,260 @@
+import { DatabaseSync } from "node:sqlite";
+import { describe, it, expect, beforeAll, afterAll } from "vitest";
+import { Kysely, sql } from "kysely";
+import { NodeSqliteDialect } from "../../node-sqlite-dlalect";
+import { kyselyAdapter } from "../../kysely-adapter";
+import { runAdapterTest } from "../../../test";
+import { getMigrations } from "../../../../db/get-migration";
+import type { BetterAuthOptions } from "../../../../types";
+import merge from "deepmerge";
</file context>
|
lets go |
b8bbd11 to
cd93f65
Compare
e699806 to
00315e8
Compare
# Conflicts: # examples/browser-extension-example/package.json # examples/nextjs-mcp/package.json # examples/tanstack-example/package.json # pnpm-lock.yaml
…imself65/2025/08/07/fix
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Related: nodejs/node@d1eabcb Related: nodejs/node#59405 Related: better-auth/better-auth#3869 Related: oven-sh/bun#22109 Nowadays, there are tons of database packages, like sqlite3, sqlite, better-sqlite, bun:sqlite... Checking the difference from them is pretty hard. instanceof is not good, since the developer will still need to import the module, which is costly. I think we should provide a symbol to distinguish different SQLite classes, at least nodejs could make the first step. --------- Signed-off-by: Alex Yang <himself65@outlook.com> Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
Related: nodejs/node@d1eabcb Related: nodejs/node#59405 Related: better-auth/better-auth#3869 Related: oven-sh/bun#22109 Nowadays, there are tons of database packages, like sqlite3, sqlite, better-sqlite, bun:sqlite... Checking the difference from them is pretty hard. instanceof is not good, since the developer will still need to import the module, which is costly. I think we should provide a symbol to distinguish different SQLite classes, at least nodejs could make the first step. --------- Signed-off-by: Alex Yang <himself65@outlook.com> Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
I'm a Node.js SQLite WG member, so I'm happy to integrate this into better-auth. Even though it's not stable yet but it's okay to do sth like testing and small demo
Document: https://nodejs.org/docs/v22.18.0/api/sqlite.html#sqlite
Summary by cubic
Added support for the experimental Node.js built-in SQLite driver (
node:sqlite) in Better Auth, allowing users to connect using DatabaseSync. Updated documentation and added tests for the new dialect.node:sqlite.