Implement env helpers and refactor config #400
Conversation
📝 WalkthroughWalkthroughThe changes refactor environment variable access throughout the codebase by replacing generic accessor functions with new, type-specific functions for strings, numbers, booleans, CSV arrays, and filters. This standardizes and clarifies environment variable parsing, reduces manual type conversion, and updates all configuration and logging modules to use the new typed accessors. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant env
participant Logger
participant Filter
App->>env: string("VAR_NAME", default)
App->>env: number("VAR_NAME", default)
App->>env: boolean("VAR_NAME", default)
App->>env: csv("VAR_NAME", default)
App->>env: filter("VAR_NAME", defaultJson, Logger)
env->>Logger: (for filter only) log parsing
env->>Filter: (for filter only) createFilter
env-->>App: Typed value (string, number, boolean, array, or filter)
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
yarn install v1.22.22 ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (4)
src/lib/env.ts (3)
42-49: Consider more flexible boolean parsing.The current implementation only accepts exactly
'true'as true. Consider supporting common boolean representations for better usability.export function boolean( envVarName: string, defaultValue = false, ): boolean { const value = process.env[envVarName]; if (value === undefined || value.trim() === '') return defaultValue; - return value === 'true'; + return ['true', '1', 'yes', 'on'].includes(value.toLowerCase()); }
65-69: Consider trimming whitespace in CSV parsing.The current implementation doesn't trim whitespace around comma-separated values, which could lead to unexpected behavior with spaces.
export function csv(envVarName: string, defaultValue = ''): string[] { const value = process.env[envVarName]; const str = value !== undefined && value.trim() !== '' ? value : defaultValue; - return str === '' ? [] : str.split(','); + return str === '' ? [] : str.split(',').map(s => s.trim()); }
76-84: Optimize JSON parsing in filter function.The function parses JSON twice (line 82 and 83), which is redundant and potentially inefficient.
export function filter( envVarName: string, defaultJson: string, logger: Logger, ): ItemFilter { const raw = string(envVarName, defaultJson); - const canonical = canonicalize(JSON.parse(raw)); - return createFilter(JSON.parse(canonical), logger); + const parsed = JSON.parse(raw); + const canonical = canonicalize(parsed); + return createFilter(parsed, logger); }src/config.ts (1)
64-67: Use numeric default instead of string template.The numeric environment variable is using a string template literal as the default, which requires unnecessary string-to-number conversion.
export const REDIS_CACHE_TTL_SECONDS = env.number( 'REDIS_CACHE_TTL_SECONDS', - `${60 * 60 * 8}`, // 8 hours by default + 60 * 60 * 8, // 8 hours by default );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/config.ts(18 hunks)src/lib/env.ts(2 hunks)src/log.ts(1 hunks)src/tracing.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/lib/env.ts (2)
src/types.d.ts (1)
ItemFilter(719-721)src/filters.ts (1)
createFilter(278-380)
🔇 Additional comments (8)
src/lib/env.ts (3)
22-25: LGTM: Clean function rename with preserved logic.The rename from
varOrDefaulttostringimproves clarity about the return type while maintaining the same logic.
27-30: LGTM: Clean function rename with preserved logic.The rename from
varOrUndefinedtostringOrUndefinedimproves clarity about the return type while maintaining the same logic.
32-40: LGTM: Clean function rename with preserved logic.The rename from
varOrRandomtostringOrRandomimproves clarity while maintaining the same random generation logic.src/log.ts (1)
22-26: LGTM: Consistent adoption of typed environment accessors.The conversion to typed environment variable functions improves code clarity and type safety while preserving existing behavior.
src/tracing.ts (1)
32-46: LGTM: Improved type safety with numeric environment variables.The conversion from manual string-to-number coercion (
+env.varOrDefault) to typedenv.numbercalls improves clarity and eliminates potential parsing issues.src/config.ts (3)
32-32: LGTM: Consistent adoption of typed environment accessors.The conversion of configuration constants to use typed environment variable functions significantly improves code clarity and type safety throughout the configuration layer.
Also applies to: 56-59, 74-77, 80-80
130-130: LGTM: Effective use of CSV parsing helper.The new
env.csvfunction eliminates manual string splitting and provides consistent CSV parsing across the configuration.Also applies to: 133-136, 143-143, 204-207, 210-213, 567-567
259-263: LGTM: Streamlined filter creation.The new
env.filterfunction effectively replaces manual JSON parsing and filter creation, reducing code duplication and improving consistency.Also applies to: 272-276, 573-577, 583-587
|
Closing this in favor of #401. |
Summary
string,stringOrUndefined,stringOrRandom,boolean,number,numberOrUndefined,csv,filter)Testing
yarn test(fails: command not found)