feat: shared env#756
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR introduces a centralized environment loading system by creating a new ChangesShared environment loading migration
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/tests/src/index.ts (1)
178-197: ⚡ Quick winComment contradicts fallback logic.
Lines 179–181 state "the
parseEnvfallback under Bun (which lacks it..." — the pronoun "it" appears to refer toprocess.loadEnvFile, yet the sentence positionsparseEnvas the fallback for Bun. If Bun lacksparseEnv(as the parenthetical implies), the fallback branch will fail. Clarify whether Bun is expected to provideparseEnv, or revise the comment and implementation to match the actual capability matrix.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tests/src/index.ts` around lines 178 - 197, The comment in loadDotEnvTest is misleading about which runtime provides which helper; update the comment to correctly state that Node provides process.loadEnvFile while Bun auto-loads .env but does not provide process.loadEnvFile, so we fallback to parseEnv+readFileSync when process.loadEnvFile is absent; also adjust wording so it doesn't imply Bun lacks parseEnv — reference process.loadEnvFile, parseEnv, readFileSync, and the loadDotEnvTest function to ensure the comment matches the implemented branching logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@boilerplates/shared-env/bati.config.ts`:
- Around line 3-8: Add an `if` gate to the defineConfig call so the config only
applies when the project meta indicates the boilerplate is relevant; update the
exported defineConfig({ enforce: "pre" }) to include an if property (e.g. if:
(meta) => Boolean(meta?.selectedBoilerplates?.includes('shared-env')) or a
similar predicate that checks meta for the presence of this boilerplate),
keeping the existing enforce: "pre" and using the defineConfig wrapper.
In `@boilerplates/shared-env/files/server/load.ts`:
- Around line 19-29: The empty catch is swallowing all errors when loading the
env; change it to catch the error object (e.g., catch (err)) and only ignore
missing-file errors (err.code === 'ENOENT'), rethrow any other errors so
parse/permission failures surface; apply this around the block that calls
process.loadEnvFile, parseEnv, and readFileSync so failures in
process.loadEnvFile, parseEnv, or readFileSync are not silently suppressed.
In `@boilerplates/shared-env/package.json`:
- Around line 23-30: The package.json exports for the subpaths "./server/env"
and "./server/load" only expose type entries and omit runtime targets, which
breaks side-effect/runtime imports; update those export entries in package.json
to include appropriate "import" and/or "require" (or "default") runtime entry
points (pointing at the built JS files in dist, e.g., dist/server/env.* and
dist/server/load.*) alongside the existing "types" entries so runtime module
resolution succeeds when code imports "./server/env" or "./server/load".
---
Nitpick comments:
In `@packages/tests/src/index.ts`:
- Around line 178-197: The comment in loadDotEnvTest is misleading about which
runtime provides which helper; update the comment to correctly state that Node
provides process.loadEnvFile while Bun auto-loads .env but does not provide
process.loadEnvFile, so we fallback to parseEnv+readFileSync when
process.loadEnvFile is absent; also adjust wording so it doesn't imply Bun lacks
parseEnv — reference process.loadEnvFile, parseEnv, readFileSync, and the
loadDotEnvTest function to ensure the comment matches the implemented branching
logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 33962076-654a-4708-a8ae-5fb49bd607c4
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (40)
boilerplates/authjs/files/server/authjs-handler.tsboilerplates/authjs/package.jsonboilerplates/better-auth/files/$package.json.tsboilerplates/better-auth/files/database/better-auth/migrate.tsboilerplates/better-auth/files/server/better-auth.tsboilerplates/better-auth/package.jsonboilerplates/drizzle/files/$package.json.tsboilerplates/drizzle/files/drizzle.config.tsboilerplates/drizzle/package.jsonboilerplates/elysia/files/$package.json.tsboilerplates/elysia/files/+server.tsboilerplates/elysia/package.jsonboilerplates/express/files/$package.json.tsboilerplates/express/files/+server.tsboilerplates/express/package.jsonboilerplates/fastify/files/$package.json.tsboilerplates/fastify/files/+server.tsboilerplates/fastify/package.jsonboilerplates/hono/files/$package.json.tsboilerplates/hono/files/+server.tsboilerplates/hono/package.jsonboilerplates/kysely/files/$package.json.tsboilerplates/kysely/files/database/kysely/db.tsboilerplates/kysely/package.jsonboilerplates/postgres/files/$package.json.tsboilerplates/postgres/files/database/postgres/schema/todos.tsboilerplates/postgres/package.jsonboilerplates/shared-env/bati.config.tsboilerplates/shared-env/bati.d.tsboilerplates/shared-env/files/server/env.tsboilerplates/shared-env/files/server/load.tsboilerplates/shared-env/package.jsonboilerplates/shared-env/tsconfig.jsonboilerplates/sqlite/files/$package.json.tsboilerplates/sqlite/files/database/sqlite/schema/todos.tsboilerplates/sqlite/package.jsonpackages/tests-utils/src/prepare.tspackages/tests/package.jsonpackages/tests/src/common.tspackages/tests/src/index.ts
💤 Files with no reviewable changes (2)
- boilerplates/better-auth/files/$package.json.ts
- packages/tests/package.json
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| 🔵 In progress View logs |
bati | d1cf5dd | Jun 09 2026, 03:56 PM |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/tests/transform-ts.spec.ts (1)
585-614: 💤 Low valueTest coverage is solid for the main use case.
The test suite validates the core behavior: imports gated by falsy BATI conditions are removed from both the emitted code and
context.imports, preventing incorrectinclude-if-importedtracking.Consider adding a test case for the
"remove-comments-only"variant with imports to ensureisRemovedByConditionhandles it correctly:describe("imports with remove-comments-only are tracked", () => { const code = `//# BATI.has("react") || "remove-comments-only" import "`@batijs/shared-env/server/load`"; export const x = 1;`; test("kept in graph when condition is remove-comments-only", () => { const { code: output, context } = transform(code, "test.ts", { BATI: new BatiSet([], features, "pnpm"), BATI_TEST: false, }); expect(output).toContain(`import "./server/load";`); expect([...context.imports]).toContain("./server/load"); }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/tests/transform-ts.spec.ts` around lines 585 - 614, Add a new test case mirroring the existing "imports stripped by a BATI condition are not tracked" suite but using the `//# BATI.has("react") || "remove-comments-only"` condition to verify the "remove-comments-only" branch keeps the import in both emitted code and `context.imports`; use the same `transform(...)` call and `BatiSet` setup (with an empty feature set and `BATI_TEST: false`) and assert that `output` contains `import "./server/load";` and `[...context.imports]` contains `"./server/load"`, ensuring `isRemovedByCondition` handles the remove-comments-only variant.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/core/tests/transform-ts.spec.ts`:
- Around line 585-614: Add a new test case mirroring the existing "imports
stripped by a BATI condition are not tracked" suite but using the `//#
BATI.has("react") || "remove-comments-only"` condition to verify the
"remove-comments-only" branch keeps the import in both emitted code and
`context.imports`; use the same `transform(...)` call and `BatiSet` setup (with
an empty feature set and `BATI_TEST: false`) and assert that `output` contains
`import "./server/load";` and `[...context.imports]` contains `"./server/load"`,
ensuring `isRemovedByCondition` handles the remove-comments-only variant.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a630219f-f9cd-47c3-8511-d245bf444b3d
📒 Files selected for processing (11)
boilerplates/authjs/files/server/authjs-handler.tsboilerplates/better-auth/files/server/better-auth.tsboilerplates/elysia/files/$package.json.tsboilerplates/shared-env/bati.config.tsboilerplates/shared-env/files/server/env.tsboilerplates/shared-env/files/server/load.tsboilerplates/shared-env/tsconfig.jsonpackages/core/src/parse/linters/linter-ts.tspackages/core/src/parse/linters/linter-vue.tspackages/core/src/parse/linters/visitor-imports.tspackages/core/tests/transform-ts.spec.ts
💤 Files with no reviewable changes (3)
- boilerplates/shared-env/bati.config.ts
- boilerplates/shared-env/files/server/load.ts
- boilerplates/shared-env/files/server/env.ts
✅ Files skipped from review due to trivial changes (1)
- boilerplates/shared-env/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (2)
- boilerplates/elysia/files/$package.json.ts
- boilerplates/authjs/files/server/authjs-handler.ts
Summary by CodeRabbit
Refactor
Chores
dotenvdev dependency and added a shared environment package.