Conversation
|
| Name | Type |
|---|---|
| @arkenv/bun-plugin | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
github-actions
bot
added
docs
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughAdds NODE_ENV to the bun-plugin environment schema and type filtering so NODE_ENV is validated at startup and appears with full type safety when included in the schema; updates examples, docs, and dependency versions accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
commit: |
Contributor
📦 Bundle Size Report
✅ All size limits passed! |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bun-plugin/src/types.ts (1)
4-50:⚠️ Potential issue | 🟡 MinorUpdate the JSDoc to document
NODE_ENVinclusion.The type was changed to include
"NODE_ENV"viaAllowedKeys, but the JSDoc block still only documents prefix-based filtering. Specifically:
- Line 9:
"filters it to only include variables matching the Bun prefix (defaults to 'BUN_PUBLIC_')"— omits theNODE_ENVexception.- Neither
@exampleblock demonstratesNODE_ENVin a schema or inprocess.env.📝 Suggested JSDoc patch
* This type extracts the inferred type from the schema (result of `type()` from arkenv), - * filters it to only include variables matching the Bun prefix (defaults to "BUN_PUBLIC_"), - * and makes them available on `process.env`. + * filters it to only include variables matching the Bun prefix (defaults to "BUN_PUBLIC_") + * and `NODE_ENV` (when present in the schema), and makes them available on `process.env`.And in the first
@example, addNODE_ENVto show it is passed through:* export const Env = type({ * BUN_PUBLIC_API_URL: 'string', * BUN_PUBLIC_DEBUG: 'boolean', + * NODE_ENV: "'development' | 'production' | 'test'", // Also typed in ProcessEnvAugmented * PORT: 'number.port', // Server-only, won't be in ProcessEnvAugmented * });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bun-plugin/src/types.ts` around lines 4 - 50, Update the JSDoc for ProcessEnvAugmented to mention that in addition to filtering by the Prefix (default "BUN_PUBLIC_"), the NODE_ENV key is always included (via AllowedKeys/FilterByPrefix). Modify the narrative sentence that currently says it "filters it to only include variables matching the Bun prefix" to explicitly state "filters by the Bun prefix and always includes NODE_ENV". Also add NODE_ENV to the first `@example` schema and/or to the second example's ProcessEnvAugmented usage so the docs demonstrate that NODE_ENV is passed through (references: ProcessEnvAugmented and FilterByPrefix).
🧹 Nitpick comments (1)
packages/bun-plugin/src/types.ts (1)
47-50: Consider exposingAllowedKeysas a type parameter for user customizability.
"NODE_ENV"is hardcoded, so users who use a custom prefix and want different extra keys included have no escape hatch. SinceFilterByPrefixalready accepts a genericAllowedKeys,ProcessEnvAugmentedcould forward a third parameter with a sensible default:♻️ Optional refactor
export type ProcessEnvAugmented< TSchema extends type.Any, Prefix extends string = "BUN_PUBLIC_", - > = FilterByPrefix<InferType<TSchema>, Prefix, "NODE_ENV">; + ExtraKeys extends string = "NODE_ENV", +> = FilterByPrefix<InferType<TSchema>, Prefix, ExtraKeys>;This is fully backward-compatible (the default is
"NODE_ENV") and lets advanced users add or replace the allowed extra keys.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bun-plugin/src/types.ts` around lines 47 - 50, ProcessEnvAugmented currently hardcodes "NODE_ENV" as the extra allowed key; make it customizable by adding a third generic type parameter like AllowedKeys extends string = "NODE_ENV" to the ProcessEnvAugmented declaration and forward that to FilterByPrefix (i.e., change ProcessEnvAugmented< TSchema extends type.Any, Prefix extends string = "BUN_PUBLIC_", AllowedKeys extends string = "NODE_ENV" > = FilterByPrefix<InferType<TSchema>, Prefix, AllowedKeys>), keeping the default so existing behavior is preserved while allowing users to supply custom extra keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/few-lights-ask.md:
- Line 2: The changeset incorrectly marks the release as "patch" even though
ProcessEnvAugmented now adds NODE_ENV to the public TypeScript API; update the
changeset entry for "@arkenv/bun-plugin" from "patch" to "minor" so the
semantic-release reflects the additive API change and consumers see a minor
version bump.
---
Outside diff comments:
In `@packages/bun-plugin/src/types.ts`:
- Around line 4-50: Update the JSDoc for ProcessEnvAugmented to mention that in
addition to filtering by the Prefix (default "BUN_PUBLIC_"), the NODE_ENV key is
always included (via AllowedKeys/FilterByPrefix). Modify the narrative sentence
that currently says it "filters it to only include variables matching the Bun
prefix" to explicitly state "filters by the Bun prefix and always includes
NODE_ENV". Also add NODE_ENV to the first `@example` schema and/or to the second
example's ProcessEnvAugmented usage so the docs demonstrate that NODE_ENV is
passed through (references: ProcessEnvAugmented and FilterByPrefix).
---
Nitpick comments:
In `@packages/bun-plugin/src/types.ts`:
- Around line 47-50: ProcessEnvAugmented currently hardcodes "NODE_ENV" as the
extra allowed key; make it customizable by adding a third generic type parameter
like AllowedKeys extends string = "NODE_ENV" to the ProcessEnvAugmented
declaration and forward that to FilterByPrefix (i.e., change
ProcessEnvAugmented< TSchema extends type.Any, Prefix extends string =
"BUN_PUBLIC_", AllowedKeys extends string = "NODE_ENV" > =
FilterByPrefix<InferType<TSchema>, Prefix, AllowedKeys>), keeping the default so
existing behavior is preserved while allowing users to supply custom extra keys.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
examples/basic-js/package-lock.jsonis excluded by!**/package-lock.jsonexamples/basic/package-lock.jsonis excluded by!**/package-lock.jsonexamples/with-bun-react/bun.lockis excluded by!**/*.lockexamples/with-bun/bun.lockis excluded by!**/*.lockexamples/with-solid-start/package-lock.jsonis excluded by!**/package-lock.jsonexamples/with-standard-schema/package-lock.jsonis excluded by!**/package-lock.jsonexamples/with-vite-react/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
.changeset/few-lights-ask.mdapps/playgrounds/bun-react/.env.developmentapps/playgrounds/bun-react/src/env.tsapps/www/content/docs/bun-plugin/index.mdxapps/www/content/docs/bun-plugin/typing-process-env.mdxexamples/basic-js/package.jsonexamples/basic/package.jsonexamples/with-bun-react/.env.developmentexamples/with-bun-react/package.jsonexamples/with-bun-react/src/env.tsexamples/with-bun/package.jsonexamples/with-solid-start/package.jsonexamples/with-standard-schema/package.jsonexamples/with-vite-react/package.jsonpackages/bun-plugin/src/types.tspackages/internal/types/src/filter-by-prefix.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Natural continuation of #804 - that was the runtime change, this PR is the typing change.
Summary by CodeRabbit
New Features
Documentation
Chores