fix: drizzle env loading #772
Conversation
|
@moreorover is attempting to deploy a commit to the Better T Stack Team on Vercel. A member of the Team first needs to authorize it. |
dosubot
bot
added
size:M
WalkthroughThree Drizzle config templates now enforce presence of required env vars at runtime and remove empty-string fallbacks for Changes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 0
🧹 Nitpick comments (2)
apps/cli/templates/db/drizzle/sqlite/drizzle.config.ts.hbs (1)
2-6: Excellent replacement of dotenv with project env module!The conditional import logic correctly selects between web and server env modules based on the backend type. Since this is a Handlebars template, the conditional will be evaluated during project scaffolding, generating a single import statement in the final file.
Based on learnings, Bun automatically loads .env files, so removing dotenv is the right approach.
Minor: Inconsistent indentation
The import statements have 4-space indentation, which will generate imports with leading whitespace in the final file. The postgres template (lines 3-7) has no indentation. Consider removing the indentation for consistency:
🔎 Optional formatting fix
\{{#if (eq backend "self")}} - import { env } from "@\{{projectName}}/env/web"; +import { env } from "@\{{projectName}}/env/web"; \{{else}} - import { env } from "@\{{projectName}}/env/server"; +import { env } from "@\{{projectName}}/env/server"; \{{/if}}apps/cli/templates/db/drizzle/mysql/drizzle.config.ts.hbs (1)
3-7: Correct approach with minor formatting inconsistency.The conditional import logic properly replaces dotenv usage with the project's env module. However, the import statements have 4-space indentation, which differs from the postgres template and will generate imports with leading whitespace.
🔎 Optional formatting fix for consistency
\{{#if (eq backend "self")}} - import { env } from "@\{{projectName}}/env/web"; +import { env } from "@\{{projectName}}/env/web"; \{{else}} - import { env } from "@\{{projectName}}/env/server"; +import { env } from "@\{{projectName}}/env/server"; \{{/if}}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/cli/templates/db/drizzle/mysql/drizzle.config.ts.hbsapps/cli/templates/db/drizzle/postgres/drizzle.config.ts.hbsapps/cli/templates/db/drizzle/sqlite/drizzle.config.ts.hbs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.hbs
📄 CodeRabbit inference engine (.cursor/rules/better-t-stack-repo.mdc)
**/*.hbs: In Handlebars templates, avoid generic if/else blocks. Write explicit conditions, such as: use if (eq orm "prisma") for Prisma, and else if (eq orm "drizzle") for Drizzle
Escape the '{{' in Handlebars templates like '{{'
Files:
apps/cli/templates/db/drizzle/sqlite/drizzle.config.ts.hbsapps/cli/templates/db/drizzle/postgres/drizzle.config.ts.hbsapps/cli/templates/db/drizzle/mysql/drizzle.config.ts.hbs
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-12-03T07:48:26.419Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Bun automatically loads .env files, so don't use the dotenv package
📚 Learning: 2025-12-03T07:48:26.419Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-12-03T07:48:26.419Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `bun:sqlite` module for SQLite instead of `better-sqlite3`
Applied to files:
apps/cli/templates/db/drizzle/sqlite/drizzle.config.ts.hbs
📚 Learning: 2025-12-03T07:48:26.419Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-12-03T07:48:26.419Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Bun automatically loads .env files, so don't use the dotenv package
Applied to files:
apps/cli/templates/db/drizzle/sqlite/drizzle.config.ts.hbsapps/cli/templates/db/drizzle/postgres/drizzle.config.ts.hbsapps/cli/templates/db/drizzle/mysql/drizzle.config.ts.hbs
📚 Learning: 2025-12-03T07:48:26.419Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-12-03T07:48:26.419Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `Bun.sql` for Postgres instead of `pg` or `postgres.js`
Applied to files:
apps/cli/templates/db/drizzle/postgres/drizzle.config.ts.hbs
🔇 Additional comments (4)
apps/cli/templates/db/drizzle/postgres/drizzle.config.ts.hbs (2)
3-7: LGTM!The conditional import logic is clean and correctly implements the backend-based selection between web and server env modules. The formatting is also consistent with no leading whitespace on the import statements.
Based on learnings, this aligns with the project's approach of relying on Bun's automatic .env loading rather than using the dotenv package.
14-14: Verify that DATABASE_URL is validated as required.The change from
process.env.DATABASE_URL || ""toenv.DATABASE_URLis an improvement for type safety. However, the old code provided a fallback empty string, while the new code expects the variable to always be defined. Ensure that the env module validates DATABASE_URL as a required non-empty string.You can use the verification script from the sqlite file review to check the env module's validation schema.
apps/cli/templates/db/drizzle/mysql/drizzle.config.ts.hbs (1)
14-14: Verify DATABASE_URL is validated as required.The change from
process.env.DATABASE_URL || ""toenv.DATABASE_URLimproves type safety and code clarity. Ensure that the env module validates this variable as a required non-empty string to prevent runtime errors.Use the verification script from the sqlite file review to check the env validation schema.
apps/cli/templates/db/drizzle/sqlite/drizzle.config.ts.hbs (1)
18-20: The env module properly validates DATABASE_URL and DATABASE_AUTH_TOKEN as required non-empty strings.The template's use of
env.DATABASE_URLandenv.DATABASE_AUTH_TOKENis correct. The generated env module (inpackages/env/src/server.ts) validates both variables asz.string().min(1)when using Turso, ensuring they are required and cannot be empty. The type-safe access toenvproperties is well-supported by this validation, eliminating the need for fallback values.
|
You tried running the "bun db:push" with this setup? It didnt work for me. |
yeah, you right, that is a problem, let me have a look a bit more into this |
|
had a bit of a read on how turbo manages environment variables https://turborepo.com/docs/crafting-your-repository/using-environment-variables https://turborepo.com/docs/reference/configuration#env I will make example shortly and report back |
|
If you can find a easy solution then its cool but the env used in drizzle config or prisma config are build time env and not required in runtime so it is fine. |
This reverts commit 2420ebf.
dosubot
bot
added
size:S
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/cli/templates/db/drizzle/postgres/drizzle.config.ts.hbs (1)
1-10: PR objective not met: dotenv should be replaced with the project's env package.According to issue #765, these lines should be replaced with the project's env package. The requested implementation was:
- Replace
import dotenv from "dotenv"withimport { env } from "@{project-name}/env/server"- Remove the
dotenv.config()call and conditional path logicThe current changes only add runtime validation but don't address the core objective of switching to the env package.
🔎 Proposed fix based on issue #765 requirements
-import { defineConfig } from "drizzle-kit"; -import dotenv from "dotenv"; - -dotenv.config({ - {{#if (eq backend "self")}} - path: "../../apps/web/.env", - {{else}} - path: "../../apps/server/.env", - {{/if}} -}); +import { defineConfig } from "drizzle-kit"; +import { env } from "@\{{projectName}}/env/server";Note: Adjust the import path based on your project's package naming convention.
apps/cli/templates/db/drizzle/sqlite/drizzle.config.ts.hbs (1)
1-10: PR objective not met: dotenv should be replaced with the project's env package.According to issue #765, these lines should be replaced with the project's env package. The requested implementation was:
- Replace
import dotenv from "dotenv"withimport { env } from "@{project-name}/env/server"- Remove the
dotenv.config()call and conditional path logicThe current changes only add runtime validation but don't address the core objective of switching to the env package.
🔎 Proposed fix based on issue #765 requirements
-import { defineConfig } from "drizzle-kit"; -import dotenv from "dotenv"; - -dotenv.config({ - {{#if (eq backend "self")}} - path: "../../apps/web/.env", - {{else}} - path: "../../apps/server/.env", - {{/if}} -}); +import { defineConfig } from "drizzle-kit"; +import { env } from "@\{{projectName}}/env/server";Note: Adjust the import path based on your project's package naming convention.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/cli/templates/db/drizzle/mysql/drizzle.config.ts.hbsapps/cli/templates/db/drizzle/postgres/drizzle.config.ts.hbsapps/cli/templates/db/drizzle/sqlite/drizzle.config.ts.hbs
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/cli/templates/db/drizzle/mysql/drizzle.config.ts.hbs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.hbs
📄 CodeRabbit inference engine (.cursor/rules/better-t-stack-repo.mdc)
**/*.hbs: In Handlebars templates, avoid generic if/else blocks. Write explicit conditions, such as: use if (eq orm "prisma") for Prisma, and else if (eq orm "drizzle") for Drizzle
Escape the '{{' in Handlebars templates like '{{'
Files:
apps/cli/templates/db/drizzle/sqlite/drizzle.config.ts.hbsapps/cli/templates/db/drizzle/postgres/drizzle.config.ts.hbs
🔇 Additional comments (1)
apps/cli/templates/db/drizzle/postgres/drizzle.config.ts.hbs (1)
1-23: The template implementation appears correct. The conditional logic usingbackendvariable to select.envfiles is appropriate for distinguishing backend architecture (self-hosted vs. separate server), not ORM selection. Since this file is in a drizzle-specific directory, the guideline requiring explicit ORM conditions applies only to multi-ORM templates.However, the core concern—whether
bun db:pushfunctions correctly—requires manual runtime testing with an actual database environment, which cannot be verified through code inspection alone. The template includes proper DATABASE_URL validation and follows the correct drizzle-kit command pattern. Verify functionality by runningbun db:pushin a local environment with a properly configured database.
|
I can not find a simple solution yet. But I still feel like turbo should be able to handle environment variables in this case as well. In the mean time I suggest these changes. It will clearly say if referenced .env file does not contain required variable so it a bit more developer friendly. |
2 participants
closes #765
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.