-
Notifications
You must be signed in to change notification settings - Fork 141
Feat: config env route #629
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughA new configuration router is introduced that exposes environment configuration values via a GET /config/env endpoint. The router is imported and integrated into the main router object, extending the public API surface with config-related endpoints. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/server/routers/config.ts (4)
6-6
: Make tags readonly.Minor type-safety/readability improvement; prevents accidental mutation and preserves tuple literal types.
-const tags = ['config']; +const tags = ['config'] as const;
11-18
: Optionally centralize the schema for reuse (server/client).Export a named EnvConfigSchema and reference it in output; makes typing reusable for clients and tests.
import { publicProcedure } from '@/server/orpc'; -const tags = ['config']; +const tags = ['config'] as const; + +export const EnvConfigSchema = z.object({ + name: z.string().min(1).optional(), + color: z.string(), + emoji: z.string().min(1).optional(), + isDemo: z.boolean(), + isDev: z.boolean(), +}); export default { env: publicProcedure() .route({ method: 'GET', path: '/config/env', tags }) - .output( - z.object({ - name: z.string().optional(), - color: z.string(), - emoji: z.string().optional(), - isDemo: z.boolean(), - isDev: z.boolean(), - }) - ) + .output(EnvConfigSchema)
20-28
: Confirm server runtime for import.meta.env.DEV; consider sourcing from the same env helper.If this code ever runs outside Vite’s transform (e.g., ts-node/tests), import.meta.env may be undefined. For consistency with envClient, consider reusing the same isDev source you already rely on there.
Example (only if available in your env helpers):
- isDev: import.meta.env.DEV, + isDev: envClient.__IS_DEV__ ?? import.meta.env.DEV,Or inject a small util exporting isDev used across server and env parsing.
8-29
: Optional: add caching hint/metadata.Values are static per build/env; if your framework supports per-route cache metadata or headers, set a reasonable max-age (e.g., 5–15 minutes) to cut noise.
Happy to wire this if you share the caching API used with orpc/your server.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/server/router.ts
(2 hunks)src/server/routers/config.ts
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/server/routers/config.ts (1)
src/env/client.ts (1)
envClient
(24-52)
⏰ 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). (4)
- GitHub Check: 🔬 Tests (24)
- GitHub Check: 🔬 Tests (lts/*)
- GitHub Check: Playwright E2E Tests
- GitHub Check: 🔬 Tests (22)
🔇 Additional comments (2)
src/server/router.ts (1)
5-5
: LGTM; route mounted correctly.Import and mount look consistent with existing pattern. Ensure any generated client/types are refreshed.
Quick checklist:
- Run typegen/client generation if applicable.
- Hit GET /config/env once in dev to confirm discovery and no double-prefixing.
Also applies to: 17-17
src/server/routers/config.ts (1)
10-10
: The review comment's concern is unfounded—there is no auto-prefixing.The ORPC framework does not automatically prefix routes based on parent keys. This is evident from the codebase patterns:
- book.ts (registered as
book: bookRouter
) uses paths like/books
and/users/{id}
without/book/
prefix- user.ts (registered as
user: userRouter
) uses paths like/users
without/user/
prefix- genre.ts (registered as
genre: genreRouter
) uses/genres
without/genre/
prefix- account.ts (registered as
account: accountRouter
) intentionally includes namespace:/account/submit-onboarding
- config.ts (registered as
config: configRouter
) follows the same pattern:/config/env
The hardcoded
/config/env
path is intentional and consistent with how account.ts structures its routes. The path will not become/config/config/env
because no auto-prefixing occurs.Likely an incorrect or invalid review comment.
Add a new server route to expose env config info
Summary by CodeRabbit