migrate to prisma 7.0 #10
Conversation
WalkthroughPrisma upgraded to v7; added LibSQL and MariaDB adapters; generator provider changed to "prisma-client"; datasource URL lines removed from schemas; added prisma.config.ts for environment-aware schema/datasource selection; adapter-backed Prisma clients and CI/test DB/workflow adjustments introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as CI/Test Runner
participant Env as Environment
participant Config as prisma.config.ts
participant Lib as src/lib/prisma.ts
participant PrismaCLI as Prisma CLI
Runner->>Env: read CI, DB_FILE, DATABASE_URL, SHADOW_DATABASE_URL
Env-->>Config: provide env values
alt CI == "true"
Note right of Config `#D6EAF8`: E2E mode (file DB)
Config->>Lib: select schema "prisma-e2e/schema.prisma"
Lib->>Lib: getPrismaE2E() → Prisma client(adapter: PrismaLibSql(file:DB_FILE))
else
Note right of Config `#F9E79F`: Dev/Prod mode (env DB)
Config->>Lib: select schema "prisma-main/schema.prisma"
Lib->>Lib: getPrismaMain() → Prisma client(adapter: PrismaMariaDb(DATABASE_URL))
end
Runner->>PrismaCLI: run `CI=true prisma generate` & `CI=true prisma db push --schema=...`
PrismaCLI-->>Runner: generated client & applied schema
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.json(2 hunks)prisma-e2e/schema.prisma(0 hunks)prisma-main/schema.prisma(0 hunks)prisma.config.ts(1 hunks)
💤 Files with no reviewable changes (2)
- prisma-e2e/schema.prisma
- prisma-main/schema.prisma
🧰 Additional context used
🧬 Code graph analysis (1)
prisma.config.ts (1)
e2e/setup-e2e-db.js (1)
path(5-5)
🪛 GitHub Actions: Test & Deploy
prisma.config.ts
[warning] 1-1: Code style issues found by Prettier. Run 'npx prettier --write .' to fix.
🔇 Additional comments (1)
prisma.config.ts (1)
1-3: The import fromprisma/configis valid. The Prisma Config reference documents defineConfig usage and options, including the env helper, confirming that bothdefineConfigandenvare correct APIs for Prisma 7.0.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/test/global-setup.ts (1)
26-31: Confirm that implicit Prisma client generation in global test setup is desiredDropping
--skip-generatemeans every global test setup will now also run Prisma client generation for the temp schema, which adds work and may be redundant if the client is already generated elsewhere (and could differ slightly from your main config if the schema diverges over time). It’s worth double‑checking whether you actually need generation in this path; if not, consider re‑introducing--skip-generatehere or removing any separateprisma generatestep so you don’t pay for generation twice.e2e/setup-e2e-db.js (1)
22-31: Avoid double Prisma client generation in E2E setupWith
--skip-generateremoved,prisma db pushwill now also runprisma generate, but you already callprisma generatejust above. That means the client is generated twice on every E2E setup. Consider either dropping the explicitprisma generatecall or restoring--skip-generateondb push, depending on which step you want to be the single source of truth.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
e2e/setup-e2e-db.js(1 hunks)src/test/global-setup.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/global-setup.ts (1)
src/test/constants.ts (1)
TEMP_SCHEMA_PATH(4-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-tests
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
prisma.config.ts (1)
6-6: Inconsistent CI environment checks remain unfixed.This issue was previously flagged but remains unresolved. Line 6 uses
process.env.CI === 'true'(strict string equality) while line 14 usesprocess.env.CI(truthy check). This inconsistency will cause incorrect behavior whenCIis set to values like'1','yes', or other truthy non-'true'strings, resulting in a datasource/schema mismatch.Apply this diff to use consistent checking:
export default defineConfig({ datasource: - process.env.CI === 'true' + process.env.CI ? { url: process.env.DB_FILE ? process.env.DB_FILE : 'file:./prisma-main/test.db' } : { url: env('DATABASE_URL'), shadowDatabaseUrl: env('SHADOW_DATABASE_URL') }, schema: path.join(process.env.CI ? 'prisma-e2e' : 'prisma-main', 'schema.prisma') });Also applies to: 14-14
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
e2e/setup-e2e-db.js(1 hunks)prisma.config.ts(1 hunks)src/app/reset-password/ResetPasswordForm.test.tsx(0 hunks)src/lib/prisma.ts(2 hunks)src/test/constants.ts(1 hunks)src/test/global-setup.ts(2 hunks)
💤 Files with no reviewable changes (1)
- src/app/reset-password/ResetPasswordForm.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- e2e/setup-e2e-db.js
- src/test/global-setup.ts
🧰 Additional context used
🧬 Code graph analysis (2)
prisma.config.ts (1)
e2e/setup-e2e-db.js (1)
path(5-5)
src/test/constants.ts (1)
e2e/setup-e2e-db.js (1)
path(5-5)
🔇 Additional comments (2)
src/test/constants.ts (1)
4-5: LGTM!Good refactor to extract the database file path as a reusable constant. This improves maintainability and allows the path to be referenced in other files.
src/lib/prisma.ts (1)
50-51: Import paths are correct for Prisma v7's custom generator configuration.The paths
../../prisma-main/types/clientand../../prisma-e2e/types/clientare correct. Both schema files define custom generator output paths:
prisma-main/schema.prisma:output = "../prisma-main/types"prisma-e2e/schema.prisma:output = "../prisma-e2e/types"Prisma v7 generates types into a
client/subdirectory within the specified output directory, so the import paths correctly target these generated locations. This is a valid monorepo setup with multiple Prisma schemas—the paths match the generator configuration exactly.
prisma.config.ts
Outdated
| datasource: | ||
| process.env.CI === 'true' | ||
| ? { | ||
| url: process.env.DB_FILE ? process.env.DB_FILE : 'file:./prisma-main/test.db' |
There was a problem hiding this comment.
Add file: prefix when using DB_FILE environment variable.
Based on src/test/constants.ts, DB_FILE is defined as 'prisma-e2e/template.db' without the file: protocol prefix. When this value is used directly as a Prisma datasource URL, it will be invalid. The URL should be formatted as file:./prisma-e2e/template.db for SQLite.
Apply this diff to add the proper prefix:
- url: process.env.DB_FILE ? process.env.DB_FILE : 'file:./prisma-e2e/test.db'
+ url: process.env.DB_FILE ? `file:./${process.env.DB_FILE}` : 'file:./prisma-e2e/test.db'Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
prisma.config.ts around line 8: the DB_FILE env value may be
'prisma-e2e/template.db' (no protocol) which produces an invalid Prisma SQLite
URL; update the conditional to ensure the value used for url always includes the
'file:' protocol (and a './' relative prefix when appropriate) by checking if
process.env.DB_FILE starts with 'file:' and, if not, prepending 'file:./' to it;
keep the existing default 'file:./prisma-main/test.db' for the fallback.
Critical: Default datasource path mismatches selected schema.
When CI === 'true' but DB_FILE is not set, the code defaults to 'file:./prisma-main/test.db'. However, line 14 would select the prisma-e2e schema in this scenario. This creates a dangerous configuration where the datasource points to a prisma-main database but uses the prisma-e2e schema definition, which will cause runtime errors.
Apply this diff to fix the default path:
- url: process.env.DB_FILE ? process.env.DB_FILE : 'file:./prisma-main/test.db'
+ url: process.env.DB_FILE ? process.env.DB_FILE : 'file:./prisma-e2e/test.db'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| url: process.env.DB_FILE ? process.env.DB_FILE : 'file:./prisma-main/test.db' | |
| url: process.env.DB_FILE ? process.env.DB_FILE : 'file:./prisma-e2e/test.db' |
🤖 Prompt for AI Agents
In prisma.config.ts around lines 8 to 14, the datasource default URL currently
falls back to 'file:./prisma-main/test.db' which mismatches the schema chosen
when CI === 'true' (prisma-e2e), causing dangerous cross-schema use; update the
fallback logic so when CI === 'true' and DB_FILE is unset the URL defaults to
'file:./prisma-e2e/test.db' (or otherwise compute the default path based on the
selected schema), ensuring the datasource file path always matches the selected
schema.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/test/setup-db.ts (1)
11-11: Consider removing redundant DATABASE_URL environment variable at line 11.The environment variable is set but never used—all file system operations between line 11 and PrismaClient instantiation (lines 50-56) operate independently. Once PrismaClient is created with explicit datasources configuration, the env var becomes unused for the test flow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/setup-db.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/setup-db.ts (2)
scripts/migration.js (1)
prisma(284-284)src/test/constants.ts (1)
TEST_DB_PATH(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e-tests
- GitHub Check: unit-integration-tests
🔇 Additional comments (1)
src/test/setup-db.ts (1)
3-3: LGTM! Import path updated for Prisma 7.0.The import path change from
'../../prisma-e2e/types'to'../../prisma-e2e/types/client'aligns with the Prisma 7.0 migration and updated client type exports.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/lib/prisma.ts (2)
50-52: Imports and type re-export match Prisma 7 client layout; docs slightly staleSwitching to
../../prisma-main/types/clientand../../prisma-e2e/types/client(and re‑exporting from.../types/client) is consistent with the new Prisma client output. The top‑of‑file JSDoc still references../../prisma-main/types/../../prisma-e2e/types, which could be updated to mention.../types/clientto avoid confusion for future readers.If you rely on the Prisma 7 generator layout, please confirm the generated paths are indeed
types/clientin your build artifacts.Also applies to: 62-62
91-99: Unify Prisma client typing for the global singleton and hardenPrismaLibSqladapter creationTwo related points here:
- Typing of the global singleton vs.
getPrismareturn type
getPrismanow returnsmainClient | e2eClientbased onprocess.env.CI, butglobalForPrisma.prismais annotated asReturnType<typeof getPrismaMain>only.const prisma = globalForPrisma.prisma || getPrisma();therefore has a union type, and assigningprismaback toglobalForPrisma.prismacan become a TypeScript compatibility issue once e2eClient is involved.Consider widening the global type to match
getPrismaand making the property optional:-const globalForPrisma = global as unknown as { - prisma: ReturnType<typeof getPrismaMain>; -}; +type AppPrismaClient = ReturnType<typeof getPrisma>; + +const globalForPrisma = global as unknown as { + prisma?: AppPrismaClient; +}; @@ -const prisma = globalForPrisma.prisma || getPrisma(); +const prisma: AppPrismaClient = globalForPrisma.prisma ?? getPrisma();This keeps the singleton pattern but aligns the types with the actual runtime behavior.
- E2E adapter creation and
DATABASE_URLhandling
PrismaLibSqlis instantiated at module load withurl: process.env.DATABASE_URL ?? ''. That:
- Constructs the adapter even when
CIis false and you never use the e2e client.- Silently falls back to an empty URL if
DATABASE_URLis misconfigured, which can hide setup errors.It’s safer to build the adapter lazily and fail fast when
DATABASE_URLis missing:-import { PrismaLibSql } from '@prisma/adapter-libsql'; - -const adapter = new PrismaLibSql({ - url: process.env.DATABASE_URL ?? '' -}); - -export const getPrismaE2E = () => new e2eClient({ adapter }); +import { PrismaLibSql } from '@prisma/adapter-libsql'; + +export const getPrismaE2E = () => { + const url = process.env.DATABASE_URL; + if (!url) { + throw new Error('DATABASE_URL must be set when using getPrismaE2E'); + } + const adapter = new PrismaLibSql({ url }); + return new e2eClient({ adapter }); +};This confines adapter creation to the CI/e2e path and avoids silently using an invalid URL.
To confirm there are no type issues, you can run
tsc --noEmitornext buildand check for errors aroundglobalForPrisma.prismaandauthPrismaAdapter(prisma, ...). Also verify that your CI/e2e setup always setsDATABASE_URLbefore importing@/lib/prisma.Also applies to: 108-108, 116-118, 140-142
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
package.json(2 hunks)src/lib/prisma.ts(3 hunks)src/test/setup-db.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/setup-db.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e-tests
- GitHub Check: unit-integration-tests
🔇 Additional comments (1)
package.json (1)
40-41: Prisma packages are correctly placed and versions are aligned
@prisma/clientand@prisma/adapter-libsqlare independenciesfor runtime use, andprismais indevDependenciesfor CLI/migrations. All three packages track^7.0.0, confirming version alignment.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
prisma.config.ts (1)
5-15: Unify CI detection and fix datasource/schema & path mismatch.There are still a few coupled issues here:
Inconsistent CI check logic
- Lines 7–10 use
process.env.CI === 'true'.- Line 15 uses
process.env.CI ? ... : ....This can select the
prisma-e2eschema while using the non‑CI datasource (or vice versa) whenCIis set to values like'1','TRUE', etc. Define a singleisCIboolean and reuse it for bothdatasourceandschema.Default CI datasource path mismatches schema
- When
CI === 'true'andDB_FILEis unset, the datasource url falls back to'file:./prisma-main/test.db', but line 15 picks theprisma-e2eschema for truthyCI. That’s a dangerous mismatch.DB_FILE may be missing
file:prefix
- If
DB_FILEis set to a bare path (for exampleprisma-e2e/test.db), passing it directly will produce an invalid SQLite URL. Normalizing it is safer.You can address all three with something like:
-import 'dotenv/config'; -import path from 'node:path'; -import { defineConfig, env } from 'prisma/config'; - -export default defineConfig({ - datasource: - process.env.CI === 'true' - ? { - url: process.env.DB_FILE ? process.env.DB_FILE : 'file:./prisma-main/test.db' - } - : { - url: env('DATABASE_URL'), - shadowDatabaseUrl: env('SHADOW_DATABASE_URL') - }, - schema: path.join(process.env.CI ? 'prisma-e2e' : 'prisma-main', 'schema.prisma') -}); +import 'dotenv/config'; +import path from 'node:path'; +import { defineConfig, env } from 'prisma/config'; + +const isCI = ['1', 'true', 'yes'].includes(String(process.env.CI ?? '').toLowerCase()); + +const dbFileEnv = process.env.DB_FILE; +const dbFileUrl = + dbFileEnv && dbFileEnv.startsWith('file:') + ? dbFileEnv + : dbFileEnv + ? `file:./${dbFileEnv}` + : 'file:./prisma-e2e/test.db'; + +export default defineConfig({ + datasource: isCI + ? { + url: dbFileUrl, + } + : { + url: env('DATABASE_URL'), + shadowDatabaseUrl: env('SHADOW_DATABASE_URL'), + }, + schema: path.join(isCI ? 'prisma-e2e' : 'prisma-main', 'schema.prisma'), +});This keeps schema and datasource in lockstep and ensures the SQLite URL is valid in CI.
Confirm current Prisma 7 docs for `prisma.config.(ts|js)` regarding `datasource` URL formats (especially SQLite `file:` URLs) and recommended CI environment handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.json(2 hunks)prisma.config.ts(1 hunks)src/lib/prisma.ts(4 hunks)src/test/setup-db.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/setup-db.ts
🧰 Additional context used
🧬 Code graph analysis (2)
prisma.config.ts (1)
e2e/setup-e2e-db.js (1)
path(5-5)
src/lib/prisma.ts (1)
src/test/constants.ts (1)
TEST_DB_PATH(6-6)
🪛 GitHub Actions: Test & Deploy
src/lib/prisma.ts
[error] 105-105: Parsing error: ',' expected.
🔇 Additional comments (1)
package.json (1)
40-43: Dependency block aligns with adapter-based Prisma 7 setup.The added adapter packages and the Prisma client/CLI version bumps are consistent with the new adapter‑based client instantiation elsewhere in the PR. I don’t see issues in this manifest segment.
Also applies to: 85-85
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/lib/prisma.ts (1)
73-83: Fix eagerDATABASE_URLparsing and MariaDB adapter host/port handlingThis still has two issues previously flagged in review comments:
Module-scope
new URL(process.env.DATABASE_URL as string)
IfDATABASE_URLis unset (typical in CI when only SQLite/e2e is used), importing this module will throw beforegetPrismaE2Ecan be used. URL parsing should be moved insidegetPrismaMainwith an explicit check forprocess.env.DATABASE_URL.Misuse of
url.hostand unsafeparseInt(url.port)
url.hostincludes the port (e.g."localhost:3306"), so passing it ashostis incorrect. Also, when the URL omits a port,url.portis"", andparseInt(url.port)becomesNaN, which is not a valid adapter port.A safer pattern is:
- Lazily parse
DATABASE_URLinsidegetPrismaMain.- Use
url.hostnamefor the host.- Derive a numeric port only when present, and omit the field otherwise.
For example:
-import { PrismaMariaDb } from '@prisma/adapter-mariadb'; - -const url = new URL(process.env.DATABASE_URL as string); - -const getPrismaMain = () => - new mainClient({ - adapter: new PrismaMariaDb({ - host: url.host, - port: parseInt(url.port) - }) - }); +import { PrismaMariaDb } from '@prisma/adapter-mariadb'; + +const getPrismaMain = () => { + const databaseUrl = process.env.DATABASE_URL; + if (!databaseUrl) { + throw new Error('DATABASE_URL is not set'); + } + + const url = new URL(databaseUrl); + const port = url.port ? Number(url.port) : undefined; + + return new mainClient({ + adapter: new PrismaMariaDb({ + host: url.hostname, + ...(port !== undefined ? { port } : {}), + }), + }); +};This prevents CI/test imports from crashing and sends the adapter a clean hostname and numeric port.
Please double-check the Prisma MariaDB adapter docs for any additional required options (database name, user, password) and adjust the adapter config accordingly.
🧹 Nitpick comments (3)
e2e/setup-e2e-db.js (1)
3-15: E2E DB bootstrap with CI-based schema selection looks good (minor cleanup possible)The header clearly explains the flow, and forcing
CI: truefor bothprisma generateandprisma db pushmatches the prisma.config-based schema selection for the SQLite e2e schema. Only nit:dbUrlis now unused, so you can drop it to avoid confusion.Also applies to: 37-41, 44-48
src/lib/prisma.ts (2)
92-100: Consider avoiding a test → app dependency ingetPrismaE2E
getPrismaE2EimportsTEST_DB_PATHfrom@/test/constants, which makes the main app library depend on a test-only module. This is workable since the constants file is simple, but it inverts the usual layering and could complicate production bundling if test code ever pulls in heavier dev-only dependencies.If possible, consider either:
- Moving
getPrismaE2Eand its constants into a test-only module (e.g. undersrc/test), or- Passing the DB path into
getPrismaE2Efrom test code instead of importing test constants here.
52-53: Keep Prisma generator output paths aligned with these type importsBoth the client imports and the
export *line assume that Prisma generates types at../../prisma-main/types/clientand../../prisma-e2e/types/client. As you evolveprisma-main/prisma-e2eor change generatoroutputpaths, remember these imports need to stay in lockstep or you’ll get runtime import errors.You might centralize these paths (e.g. via a small shared config module) so schema/generator changes can’t silently desync from this file.
Also applies to: 64-65
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
e2e/setup-e2e-db.js(2 hunks)prisma-e2e/schema.prisma(1 hunks)prisma-main/schema.prisma(1 hunks)prisma.config.ts(1 hunks)src/lib/prisma.ts(5 hunks)src/test/global-setup.ts(1 hunks)src/test/setup-db.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- prisma-main/schema.prisma
- src/test/setup-db.ts
- prisma.config.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/test/global-setup.ts (1)
src/test/constants.ts (3)
TEMPLATE_DB_PATH(5-5)SCHEMA_PATH(3-3)DB_FILE(4-4)
src/lib/prisma.ts (1)
src/test/constants.ts (1)
TEST_DB_PATH(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit-integration-tests
- GitHub Check: e2e-tests
🔇 Additional comments (2)
prisma-e2e/schema.prisma (1)
1-14: Schema documentation and Prisma 7 generator update look consistentThe CI/e2e-specific comments and the switch to
provider = "prisma-client"with a SQLite datasource (URL delegated toprisma.config.ts) align with the new Prisma 7 + adapter-based setup. Just keep the “structurally identical to prisma-main/schema.prisma” note in sync if the main schema evolves.Also applies to: 16-22
src/test/global-setup.ts (1)
11-20: Template DB creation flow viaDB_FILEenv is consistent and robustThe documented 3-step process (delete existing template,
prisma db pushwithDB_FILE, then assertTEMPLATE_DB_PATHexists) is clear and matches the new constants setup. This should give predictable failures ifprisma db pushmisconfigures the path, and teardown covers the main SQLite artifacts cleanly.If you want extra safety, you can verify end-to-end by running tests once with
DB_FILEintentionally mis-set to confirm the existence check fails as expected.Also applies to: 29-38, 40-45
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/lib/prisma.ts (1)
73-83: Fix MariaDB adapter config: move URL parsing intogetPrismaMainand usehostname+ full credentialsThe current MariaDB adapter wiring has a few problems:
const url = new URL((process.env.DATABASE_URL as string) ?? 'mysql://localhost:3306');runs at module load, even in CI where only SQLite is needed, and silently falls back to a dummy URL instead of surfacing a missingDATABASE_URL.host: url.hostpasses"hostname:port"instead of just the hostname; Prisma’s examples use a plain hostname and numeric port. (prisma.io)- Only
hostandportare passed; the official adapter README shows specifying user/password/database (or, alternatively, using a parsed connection string). (glama.ai)Recommend:
- Parse
DATABASE_URLinsidegetPrismaMain.- Throw a clear error if
DATABASE_URLis missing whengetPrismaMainis actually invoked.- Use
url.hostnameand a safely-parsed port, and ideally derive user/password/database from the URL.Example:
-import { PrismaMariaDb } from '@prisma/adapter-mariadb'; - -const url = new URL((process.env.DATABASE_URL as string) ?? 'mysql://localhost:3306'); - -const getPrismaMain = () => - new mainClient({ - adapter: new PrismaMariaDb({ - host: url.host, - port: parseInt(url.port) - }) - }); +import { PrismaMariaDb } from '@prisma/adapter-mariadb'; + +const getPrismaMain = () => { + const databaseUrl = process.env.DATABASE_URL; + if (!databaseUrl) { + throw new Error('DATABASE_URL is not set (required for main MySQL/MariaDB Prisma client)'); + } + + const url = new URL(databaseUrl); + const port = url.port ? Number(url.port) : undefined; + const username = url.username || undefined; + const password = url.password || undefined; + const database = url.pathname ? url.pathname.slice(1) || undefined : undefined; + + return new mainClient({ + adapter: new PrismaMariaDb({ + host: url.hostname, + ...(port ? { port } : {}), + ...(username ? { user: decodeURIComponent(username) } : {}), + ...(password ? { password: decodeURIComponent(password) } : {}), + ...(database ? { database } : {}), + }), + }); +};This matches the adapter’s documented configuration style and avoids hidden defaults or malformed
hostvalues.Confirm that for `@prisma/adapter-mariadb` v7 / Prisma ORM 7, the recommended configuration pattern still uses either `{ host, port, user, password, database }` or a parsed `DATABASE_URL`, and that `url.hostname` (not `url.host`) is the correct value to pass as the `host` option.
🧹 Nitpick comments (1)
src/lib/prisma.ts (1)
111-112: Environment-based client selection and singleton typing are solid
getPrismaswitching onprocess.env.CIcorrectly routes between main and E2E clients per the docs.- Typing
globalForPrisma.prismaasReturnType<typeof getPrisma>is a clean way to keep the singleton’s type in sync with the factory.If you ever introduce more environments (e.g. local dev SQLite), consider abstracting the env switch to a small helper, but it’s not necessary now.
Also applies to: 120-121
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
prisma.config.ts(1 hunks)src/lib/prisma.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- prisma.config.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/prisma.ts (1)
src/test/constants.ts (1)
DB_FILE(4-4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit-integration-tests
- GitHub Check: e2e-tests
🔇 Additional comments (4)
src/lib/prisma.ts (4)
8-17: Docs correctly describe new multi-schema + adapter setupDocumentation now accurately reflects prisma-main/prisma-e2e split, adapter usage, and the “wrong import” example path. No issues here.
Also applies to: 28-29
52-53: Type re-exports and client imports look consistentImporting
PrismaClientfromprisma-main/prisma-e2eand re-exporting all types fromprisma-maincleanly centralizes typing behind this module, matching the comments’ intent. Looks good.Also applies to: 64-64
169-172: Auth adapter wiring matches runtime DB selectionConfiguring
authPrismaAdapteronce withprovider: process.env.CI ? 'sqlite' : 'mysql'keeps Better Auth in sync with whichever Prisma client is active. This is consistent with the rest of the module.
87-90: E2E SQLite client via LibSQL adapter is correctly configuredThe code pattern using
PrismaLibSql({ url: \file:./${DB_FILE}` })is confirmed to work with@prisma/adapter-libsqlin Prisma ORM 7 for local/embedded SQLite databases without requiring anauthToken`.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/lib/prisma.ts (1)
73-82: Fix MariaDB adapter config: usehostname, safe port handling, and a clearDATABASE_URLcheckRight now:
host: url.hostincludes both hostname and port (for exampledb:3306), which is not a valid hostname and can break DNS resolution.parseInt(url.port)will beNaNwhen the URL omits an explicit port, and thatNaNis passed straight to the adapter.- If
DATABASE_URLis missing,new URL(process.env.DATABASE_URL as string)throws a generic “Invalid URL” instead of a clear configuration error.Prisma’s own MySQL/MariaDB adapter examples show passing a plain hostname and numeric port into
PrismaMariaDb’s options.(prisma.io)Suggest tightening this function as follows:
-const getPrismaMain = () => { - const url = new URL(process.env.DATABASE_URL as string); - const adapter = new PrismaMariaDb({ - host: url.host, - port: parseInt(url.port) - }); - return new mainClient({ adapter }); -} +const getPrismaMain = () => { + const databaseUrl = process.env.DATABASE_URL; + if (!databaseUrl) { + throw new Error('DATABASE_URL is not set'); + } + + const url = new URL(databaseUrl); + const port = url.port ? Number.parseInt(url.port, 10) : undefined; + + const adapter = new PrismaMariaDb({ + host: url.hostname, + ...(port && !Number.isNaN(port) ? { port } : {}), + }); + + return new mainClient({ adapter }); +};This avoids malformed hosts, skips passing a bogus port, and fails fast with a clear message if
DATABASE_URLis misconfigured.
🧹 Nitpick comments (1)
src/lib/prisma.ts (1)
118-120: Consider usingglobalThisand an optionalprismafield for the singletonThe current singleton wiring works, but it hard-casts
globaland treatsprismaas always-present in the type:const globalForPrisma = global as unknown as { prisma: ReturnType<typeof getPrisma>; }; const prisma = globalForPrisma.prisma || getPrisma();For slightly safer and more portable code (Node + edge / different runtimes), you could:
- Use
globalThisinstead ofglobal.- Make
prismaoptional in the type and use??for the fallback.Example:
-const globalForPrisma = global as unknown as { - prisma: ReturnType<typeof getPrisma>; -}; +const globalForPrisma = globalThis as unknown as { + prisma?: ReturnType<typeof getPrisma>; +}; @@ -const prisma = globalForPrisma.prisma || getPrisma(); +const prisma = globalForPrisma.prisma ?? getPrisma();This keeps runtime behavior the same while better reflecting the actual lifecycle of the cached instance.
Also applies to: 142-144
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/prisma.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/prisma.ts (1)
src/test/constants.ts (1)
DB_FILE(4-4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit-integration-tests
- GitHub Check: e2e-tests
🔇 Additional comments (3)
src/lib/prisma.ts (3)
8-29: Docs accurately describe new adapter-based dual-schema setupThe updated module doc (dependencies list, CI SQLite vs prod MySQL/MariaDB explanation, and “correct vs wrong” import examples) matches the new adapter-backed architecture and reinforces the intended public API (
@/lib/prismaonly). No changes needed here.
52-65: Import/re-export wiring for Prisma clients and types looks consistentUsing separate generated clients for
prisma-mainandprisma-e2ewhile re-exporting all types fromprisma-mainkeeps runtime concerns (engine choice) decoupled from the public typing surface, which is exactly what your docs above promise. This setup is coherent and should work well as long as both schemas remain structurally identical.
91-99: LibSQL E2E client wiring matches Prisma’s file-based SQLite pattern
getPrismaE2Eusingnew PrismaLibSql({ url: process.env.DB_FILE ?? \file:./${DB_FILE}` })aligns with the documented pattern for file-backed SQLite/LibSQL adapters (for example,url: "file:./prisma/dev.db").([prisma.io](https://www.prisma.io/docs/concepts/database-connectors/sqlite?utm_source=openai)) TheDB_FILE` constant keeps the default in sync with test helpers, and the env override gives CI flexibility. No changes needed here.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/lib/prisma.ts (1)
73-82: Fix MariaDB adapter host/port parsing and add a clearerDATABASE_URLvalidation.
getPrismaMainstill has two issues:
host: url.hostpasses"hostname:port"toPrismaMariaDb, while Prisma docs showhostshould be just the hostname, withportas a separate numeric field. (prisma.io)parseInt(url.port)will beNaNif the URL omits an explicit port, which can break otherwise validDATABASE_URLvalues.You already moved URL parsing inside
getPrismaMain, so CI no longer crashes when using the SQLite client. To finish this off, I’d recommend:
- Validate
DATABASE_URLexplicitly for a clearer error.- Use
url.hostnameand only passportwhen it exists and parses cleanly.Example diff:
-const getPrismaMain = () => { - const url = new URL(process.env.DATABASE_URL as string); - const adapter = new PrismaMariaDb({ - host: url.host, - port: parseInt(url.port) - }); - return new mainClient({ adapter }); -}; +const getPrismaMain = () => { + const databaseUrl = process.env.DATABASE_URL; + if (!databaseUrl) { + throw new Error('DATABASE_URL is not set'); + } + + const url = new URL(databaseUrl); + const port = url.port ? parseInt(url.port, 10) : undefined; + + const adapter = new PrismaMariaDb({ + host: url.hostname, + ...(port ? { port } : {}), + }); + + return new mainClient({ adapter }); +};This aligns with the PrismaMariaDb examples (hostname + numeric port) and avoids NaN/URL edge cases. (prisma.io)
Please double‑check this against the current
@prisma/adapter-mariadbdocs you’re targeting to ensure the option shape remains correct.
🧹 Nitpick comments (2)
src/lib/prisma.ts (1)
91-99: LibSQL adapter wiring looks correct; consider decoupling test constants from app lib.Functionally this is good:
PrismaLibSqlis instantiated with a{ url }options object, which matches the current adapter API. (prisma.io)- Defaulting to
file:./${DB_FILE}keeps the runtime path in sync with the test constants.Two minor suggestions:
Decouple from
@/test/constants: Importing fromsrc/testinsidesrc/libcouples production code to test-only modules. Consider movingDB_FILE(and related path constants) into a shared non-test module (e.g.,src/lib/db-constants) and importing from there in bothprisma.tsand test utilities.Make
DB_FILEsemantics explicit: In a follow-up, it might be worth a short comment indicating thatprocess.env.DB_FILEis expected to be a full LibSQL/SQLite URL (includingfile:), while the constant is a relative path, to avoid accidental use of a bare filename in env.If you update the adapter options in line with future
@prisma/adapter-libsqlreleases, please re-verify thaturl: file:./...remains the recommended pattern for local SQLite databases.src/test/constants.ts (1)
4-7: DB path constants look good; consider deriving the journal path fromDB_FILE.Centralizing
DB_FILEandTEMPLATE_DB_FILEand then buildingTEMPLATE_DB_PATH/TEST_DB_PATHfrom them is a nice cleanup and keeps filesystem paths in sync across tests and setup scripts.As a minor follow-up, you could also derive the journal path from
DB_FILEto avoid one remaining hard-coded filename, e.g.:export const TEST_DB_JOURNAL_PATH = path.resolve( __dirname, `../../${DB_FILE}-journal` );Not required, but it would make renaming
DB_FILEsafer in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/test.yml(5 hunks)e2e/setup-e2e-db.js(2 hunks)src/lib/prisma.ts(5 hunks)src/test/constants.ts(1 hunks)src/test/global-setup.ts(1 hunks)src/test/setup-db.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/setup-db.ts
- e2e/setup-e2e-db.js
🧰 Additional context used
🧬 Code graph analysis (3)
src/test/global-setup.ts (1)
src/test/constants.ts (3)
TEMPLATE_DB_PATH(6-6)SCHEMA_PATH(3-3)TEMPLATE_DB_FILE(5-5)
src/test/constants.ts (1)
e2e/setup-e2e-db.js (1)
path(19-19)
src/lib/prisma.ts (1)
src/test/constants.ts (1)
DB_FILE(4-4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e-tests
- GitHub Check: unit-integration-tests
🔇 Additional comments (9)
.github/workflows/test.yml (4)
5-7: Branch filter restricts workflow to master.Good move to limit test & deploy runs to the master branch, reducing unnecessary CI load on feature branches.
203-203: Deploy condition correctly prevents fork PR deployments.The condition properly gates deployment: prevents fork PRs while allowing pushes to master and manual workflow triggers. The branch filter on the push event (line 6) adds another layer of protection.
216-243: Production/preview deployment separation is well-structured.The workflow cleanly separates production deployments (master pushes with
--prodflag and alias setup) from preview deployments (pull requests with standard build). Each path has appropriate conditions and the preview build captures the URL for downstream use.
245-288: PR comment automation is functional; bot detection is reasonable.The workflow correctly uses
context.issue.numberfor pull request context and properly references the preview URL from the preceding deploy step. The bot comment detection (searching for 'Bot' user type and 'Preview Deployment' text) is practical, though could be more precise by checking for the 'github-actions' bot specifically. This is an acceptable balance between simplicity and accuracy for this use case.src/lib/prisma.ts (3)
8-29: Doc updates for dual-schema Prisma setup look consistent.The JSDoc accurately reflects the new main/e2e schema layout, adapter usage, and the recommended import pattern via
@/lib/prisma. No issues here.
52-53: Importing schema-specific clients and re‑exporting types is solid.Using
PrismaClientaliases fromprisma-main/prisma-e2eand re‑exporting everything fromprisma-maingives a single canonical type surface while still allowing engine-specific clients behind the scenes. This is a clean abstraction.Also applies to: 64-64
118-120: Singleton typing forglobalForPrismais acceptable for this pattern.Using
ReturnType<typeof getPrisma>for the globalprismainstance keeps the union of main/e2e clients in sync with the factory. Given the explicit type assertion onglobal, this is consistent with common Prisma singleton patterns.src/test/global-setup.ts (2)
11-20: Global setup documentation matches the new template DB flow.The JSDoc clearly explains the three-step template DB process and aligns with the current implementation, which helps future maintainers understand why the schema push happens once up front.
40-42: Template DB existence check and logging look good.Verifying
TEMPLATE_DB_PATHafter theprisma db pushand throwing if it’s missing gives a clear failure mode, and the success log is helpful for diagnosing setup issues.Also applies to: 44-44
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/test/global-setup.ts (1)
11-20: Fix comments to referenceDB_FILE(notTEMPLATE_DB_FILE) and describe actual behavior.The implementation uses the
DB_FILEenvironment variable:env: { ...process.env, DB_FILE: `file:./${TEMPLATE_DB_FILE}` }but both the doc block (step 2) and the inline comments still talk about a
TEMPLATE_DB_FILEenv var. That’s misleading for future maintainers.Suggested wording:
- * 2. Runs prisma db push with TEMPLATE_DB_FILE env to create template database + * 2. Runs prisma db push with DB_FILE env pointing to TEMPLATE_DB_FILE to create the template database @@ - // Use TEMPLATE_DB_FILE environment variable for database path - // prisma.config.ts reads TEMPLATE_DB_FILE when CI=true to determine database location + // Use DB_FILE environment variable for the database path (pointing at TEMPLATE_DB_FILE) + // prisma.config.ts reads DB_FILE in CI/test mode to determine the SQLite database locationOptionally, you might also switch
stdio: 'pipe'to'inherit'duringprisma db pushwhen you’re debugging, to get CLI output in the test logs, but the current setting is fine for CI.Also applies to: 29-38
🧹 Nitpick comments (3)
e2e/setup-e2e-db.js (1)
21-48: Optional: explicitly wireDB_FILEfor Prisma CLI instead of relying on the config fallback.Right now these
prisma generate/prisma db pushcalls depend on the CI branch inprisma.config.tsdefaulting tofile:./prisma-e2e/test.db. To make the script self‑contained and future‑proof (and reuse the computeddbFile), you could passDB_FILEexplicitly:-const dbFile = path.join(__dirname, '../prisma-e2e/test.db'); +const dbFile = path.join(__dirname, '../prisma-e2e/test.db'); +const dbUrl = `file:${dbFile}`; @@ -execSync('npx prisma generate --schema=prisma-e2e/schema.prisma', { - stdio: 'inherit', - env: { ...process.env, CI: true } -}); +execSync('npx prisma generate --schema=prisma-e2e/schema.prisma', { + stdio: 'inherit', + env: { ...process.env, CI: true, DB_FILE: dbUrl } +}); @@ -execSync('npx prisma db push --schema=prisma-e2e/schema.prisma', { - stdio: 'inherit', - env: { ...process.env, CI: true } -}); +execSync('npx prisma db push --schema=prisma-e2e/schema.prisma', { + stdio: 'inherit', + env: { ...process.env, CI: true, DB_FILE: dbUrl } +});This keeps the DB URL used by the CLI aligned with the file you just cleaned up, even if the Prisma config’s default ever changes.
src/test/setup-db.ts (1)
3-4: Test DB setup is consistent withgetPrismaE2E; consider tightening error handling incleanDatabase.The flow of:
- setting
process.env.DB_FILE = \file:${DB_FILE}``,- copying
TEMPLATE_DB_PATH→TEST_DB_PATH,- then creating the client via
getPrismaE2E()keeps the adapter URL and the physical test DB file in sync, which is exactly what you want for fast, file‑based tests.
One small optional improvement:
cleanDatabase()wraps all deletes in a singletryand swallows any error. If a real connection/config issue occurs, tests might silently reuse stale data. You could, for example, rethrow non‑“table not found” / non‑schema errors or at least log them atconsole.errorwhile still ignoring missing‑table cases.Also applies to: 21-22, 60-62
prisma.config.ts (1)
1-11: ClarifyCI/DB_FILEexpectations in the config comment to match current behavior.The runtime behavior here is:
- Any truthy
process.env.CIselects theprisma-e2eschema/migrations.DB_FILEis treated as a complete SQLite URL (e.g.file:./prisma-e2e/test.db), with a sane default if it’s unset.The header comment still reads as if
CImust literally be"true"and doesn’t spell out thatDB_FILEis a full URL, not just a path. To avoid future confusion/misconfiguration, you might tweak the doc like:- * - CI=true: Uses SQLite schema (prisma-e2e/schema.prisma) for testing + * - CI set: Uses SQLite schema (prisma-e2e/schema.prisma) for testing @@ - * Database URLs are configured via environment variables (DATABASE_URL) or - * DB_FILE for test environments. + * Database URLs are configured via environment variables: + * - DATABASE_URL / SHADOW_DATABASE_URL for MySQL (dev/prod) + * - DB_FILE (full SQLite URL, e.g. "file:./prisma-e2e/test.db") for CI/testNo functional changes needed; this just documents the current invariants more precisely so future scripts don’t start passing a bare path into
DB_FILE.Also applies to: 15-25
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/test.yml(5 hunks)e2e/setup-e2e-db.js(2 hunks)prisma.config.ts(1 hunks)src/test/constants.ts(1 hunks)src/test/global-setup.ts(1 hunks)src/test/setup-db.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/constants.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/test/setup-db.ts (2)
src/lib/prisma.ts (1)
getPrismaE2E(94-99)src/test/constants.ts (1)
DB_FILE(4-4)
src/test/global-setup.ts (1)
src/test/constants.ts (3)
TEMPLATE_DB_PATH(6-6)SCHEMA_PATH(3-3)TEMPLATE_DB_FILE(5-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit-integration-tests
- GitHub Check: e2e-tests
🔇 Additional comments (2)
.github/workflows/test.yml (2)
141-147: E2EDB_FILEwiring looks consistent with Prisma/test setup.Using
DB_FILE: 'file:./prisma-e2e/test.db'here aligns with the CI branch inprisma.config.tsand thegetPrismaE2Efallback, so the Playwright run should hit the same SQLite file as your Vitest setup. No issues from a workflow perspective.
202-288: Deploy job split between production and preview is structured well.The
ifon the job plus the per‑stepifconditions cleanly separate:
- production pulls/builds/deploys only on
pushtomaster- preview pulls/builds/deploys only for
pull_request- PR comment with preview URL is updated or created idempotently.
This is a solid pattern for visibility without over‑deploying.
Summary by CodeRabbit
Chores
Refactor
Tests / E2E
CI
✏️ Tip: You can customize this high-level summary in your review settings.