feat: read env with default value #1
Conversation
|
Warning Rate limit exceeded@fengmk2 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 38 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis pull request introduces several configuration and workflow files along with new source code. An ESLint configuration, TypeScript settings, and package metadata file have been added to establish linting, TypeScript configurations, and package details. Three GitHub Actions workflows manage continuous integration, package publishing, and release processes across multiple operating systems with secret management. The documentation has been enhanced with additional badges, usage guidance, and licensing details. A new TypeScript module exports an Changes
Sequence Diagram(s)sequenceDiagram
participant Repo as Repository
participant CI as GitHub Workflow
participant Node as Node Environment
Repo->>CI: Trigger on push/PR
CI->>CI: Checkout code
CI->>CI: Enable Corepack
CI->>CI: Setup Node.js (v20)
CI->>CI: Install dependencies (npm install)
CI->>CI: Build project (npm run prepublishOnly)
CI->>CI: Publish package (npx pkg-pr-new publish)
sequenceDiagram
participant C as Client
participant E as env() Function
C->>E: Call env("KEY", type, default)
E->>E: Retrieve process.env["KEY"]
alt Value exists
E->>E: Trim & convert value based on type
alt Conversion valid
E-->>C: Return converted value
else Conversion invalid
E-->>C: Throw TypeError
end
else No value exists
E-->>C: Return default value
end
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
commit: |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
![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 (8)
.github/workflows/release.yml (1)
3-6: Review the Trigger Configuration
The workflow is set to trigger on a push to the master branch. This is acceptable if your branching strategy is fixed to "master." Just ensure that this aligns with your repository’s branch naming conventions and consider whether additional events (such as tags or specific paths) might be beneficial for release automation..github/workflows/pkg.pr.new.yml (2)
23-23: Step Naming for Publish CommandFor better transparency in the workflow logs, consider adding a descriptive name for the publish step. Currently, the step only includes a run command. A named step would aid in debugging and readability.
Suggested diff:
- - run: npx pkg-pr-new publish + - name: Publish Package + run: npx pkg-pr-new publish
1-24: Consideration on Environment Variable HandlingSince the PR objectives mention "read env with default value," verify that any functionality requiring environment variable defaults is addressed—either by the newly added TypeScript module or optionally by setting an env block in this workflow if needed. For example, if you need to supply default values to certain variables during publish, you might add:
env: MY_VARIABLE: ${{ env.MY_VARIABLE || 'default_value' }}Otherwise, confirm that such logic is encapsulated within your application code.
src/index.ts (1)
12-46: Consider adding input validation and improving error messages.The implementation is solid but could benefit from some improvements:
- The key parameter should be validated to ensure it's not empty
- Error messages could be more descriptive for better debugging
Consider applying these changes:
export function env<V extends ValueType, D extends DefaultValue>(key: string, valueType: V, defaultValue?: D): EnvReturn<V, D> { + if (!key) { + throw new TypeError('Environment variable key cannot be empty'); + } let value = process.env[key]; if (typeof value === 'string') { value = value.trim(); } if (!value) { return defaultValue as EnvReturn<V, D>; } if (valueType === 'string') { return value as EnvReturn<V, D>; } if (valueType === 'boolean') { let booleanValue = false; if (value === 'true' || value === '1') { booleanValue = true; } else if (value === 'false' || value === '0') { booleanValue = false; } else { - throw new TypeError(`Invalid boolean value: ${value} on process.env.${key}`); + throw new TypeError(`Invalid boolean value for process.env.${key}: expected 'true', 'false', '1', or '0', but got '${value}'`); } return booleanValue as EnvReturn<V, D>; } if (valueType === 'number') { const numberValue = Number(value); if (isNaN(numberValue)) { - throw new TypeError(`Invalid number value: ${value} on process.env.${key}`); + throw new TypeError(`Invalid number value for process.env.${key}: '${value}' cannot be converted to a number`); } return numberValue as EnvReturn<V, D>; } - throw new TypeError(`Invalid value type: ${valueType}`); + throw new TypeError(`Invalid value type: '${valueType}'. Supported types are: 'string', 'boolean', 'number'`); }test/index.test.ts (1)
1-102: Add test cases for improved coverage.The test suite is comprehensive but could benefit from additional test cases:
- Test empty key validation
- Test whitespace-only values for number/boolean types
Consider adding these test cases:
+test('should throw error if key is empty', () => { + assert.throws(() => env('', 'string', 'default'), /Environment variable key cannot be empty/); +}); + +test('should handle whitespace values for number/boolean', () => { + mock(process.env, 'TEST_ENV_NUMBER', ' 123 '); + assert.equal(env('TEST_ENV_NUMBER', 'number', 0), 123); + + mock(process.env, 'TEST_ENV_BOOLEAN', ' true '); + assert.equal(env('TEST_ENV_BOOLEAN', 'boolean', false), true); +});tsconfig.json (1)
1-11: Consider adding sourceMap and declaration options.The TypeScript configuration is good but could be enhanced for better development experience:
Consider adding these compiler options:
{ "extends": "@eggjs/tsconfig", "compilerOptions": { "strict": true, "noImplicitAny": true, "target": "ES2022", "module": "NodeNext", - "moduleResolution": "NodeNext" + "moduleResolution": "NodeNext", + "sourceMap": true, + "declaration": true } }.github/workflows/ci.yml (1)
1-18: Consider OS-specific Node.js versions and caching.The CI workflow is well-structured but could be optimized:
Consider these improvements:
jobs: Job: name: Node.js uses: node-modules/github-actions/.github/workflows/node-test.yml@master with: - os: 'ubuntu-latest, macos-latest, windows-latest' - version: '20, 22' + os: 'ubuntu-latest, macos-latest' + version: '20, 22' + cache: 'npm' + strategy: + matrix: + include: + - os: windows-latest + node-version: '20' secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}This change:
- Adds npm caching for faster builds
- Limits Windows testing to Node.js 20 to avoid potential compatibility issues
README.md (1)
20-29: API Documentation is Detailed
The API section clearly documents the function signature, parameters, and usage. One minor suggestion: consider ensuring consistent punctuation for the parameter descriptions (e.g. ending each with a period) for improved readability.🧰 Tools
🪛 LanguageTool
[uncategorized] ~26-~26: Loose punctuation mark.
Context: ...ber | boolean) #### Parameters -key: The environment variable key to read. -...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~27-~27: Loose punctuation mark.
Context: ...vironment variable key to read. -type: The type of the value to be returned. -...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~28-~28: Loose punctuation mark.
Context: ...e value to be returned. -defaultValue: The default value to return if the envi...(UNLIKELY_OPENING_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.eslintrc(1 hunks).github/workflows/ci.yml(1 hunks).github/workflows/pkg.pr.new.yml(1 hunks).github/workflows/release.yml(1 hunks).gitignore(1 hunks)LICENSE(1 hunks)README.md(1 hunks)package.json(1 hunks)src/index.ts(1 hunks)test/index.test.ts(1 hunks)tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- .gitignore
- .eslintrc
- LICENSE
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~3-~3: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 864 characters long)
Context: ...//www.npmjs.com/package/read-env-value) 
(EN_EXCESSIVE_EXCLAMATION)
[uncategorized] ~26-~26: Loose punctuation mark.
Context: ...ber | boolean) #### Parameters - key: The environment variable key to read. -...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~27-~27: Loose punctuation mark.
Context: ...vironment variable key to read. - type: The type of the value to be returned. -...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~28-~28: Loose punctuation mark.
Context: ...e value to be returned. - defaultValue: The default value to return if the envi...
(UNLIKELY_OPENING_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Node.js / Test (windows-latest, 22)
- GitHub Check: Node.js / Test (windows-latest, 20)
🔇 Additional comments (22)
.github/workflows/release.yml (3)
1-2: Workflow Name Declaration Looks Correct
The workflow is clearly named "Release," which makes it easy to identify in the GitHub Actions dashboard.
7-13: Pin the Reusable Workflow Version for Stability
The job uses a reusable workflow with the reference "node-modules/github-actions/.github/workflows/node-release.yml@master." While functional, using the "master" branch ref can introduce unexpected changes if the reusable workflow is updated. It is recommended to pin to a specific version tag or commit SHA for consistency and stability. For example:- uses: node-modules/github-actions/.github/workflows/node-release.yml@master + uses: node-modules/github-actions/.github/workflows/[email protected] # Replace with the correct, stable version
12-13: Secrets Configuration Looks Correct
The secrets "NPM_TOKEN" and "GIT_TOKEN" are being passed properly to the workflow. Ensure that these secrets are set in your repository’s settings and that they have the required permissions..github/workflows/pkg.pr.new.yml (6)
1-2: Workflow Name and Trigger Event ConfigurationThe workflow name ("Publish Any Commit") and trigger events ([push, pull_request]) are clear and correctly specified. If your PR objective related to "read env with default value" should impact workflow-level environment handling, confirm whether an env block is needed here.
4-7: Job Configuration is CorrectThe "jobs" and "build" configuration using ubuntu-latest is configured correctly for a typical Node.js build environment.
9-11: Checkout Step is Properly ConfiguredThe checkout step using actions/checkout@v4 is implemented correctly, ensuring that the repository is available for subsequent steps.
12-16: Corepack Enable and Node SetupThe command to enable Corepack and the use of actions/setup-node@v4 with node-version set to 20 are well implemented. This ensures that the correct Node.js version and package management tool are available.
17-19: Dependency InstallationThe installation step ("npm install") is straightforward and correctly placed before building the project.
20-22: Build Step VerificationRunning "npm run prepublishOnly --if-present" is good practice to conditionally execute the build script if it exists. This step is correctly specified.
src/index.ts (1)
1-10: LGTM! Well-designed type system.The type definitions are well-structured and provide excellent type safety:
ValueTypecorrectly constrains the supported typesDefaultValueallows for undefined valuesTypeMapelegantly handles type inference based on default valuesREADME.md (6)
1-8: Badge and Header Section Looks Good
The updated header and badge section clearly provide quick access to project metrics. All links appear to be correctly formatted.🧰 Tools
🪛 LanguageTool
[style] ~3-~3: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 864 characters long)
Context: ...//www.npmjs.com/package/read-env-value)
[
9-10: Clear Project Title
The title “Read env with default value” succinctly describes the package’s purpose.
11-18: Usage Example is Concise and Informative
The “Usage” section includes a clear code snippet demonstrating how to import and use theenvfunction with a default value. This helps new users quickly understand how to work with the API.
30-32: Return Value Description is Sufficient
The “Returns” section briefly explains the function’s output. If possible, you might note the resulting type for absolute clarity, though the current description is acceptable.
34-40: Example Section Reinforces Usage
The duplicate example usage in the “Example” section reiterates how to call the function with a default value. This is helpful and clear—great job!
42-51: License and Contributors Sections are Well Documented
Including the MIT license link and a contributors badge enriches the documentation. These additions improve transparency and community engagement.package.json (6)
1-21: Project Metadata is Well Defined
The metadata including name, version, description, repository, keywords, author, license, bugs, homepage, and engines are clearly specified and align with the project’s goals.
22-35: DevDependencies are Explicitly Pinned
All development dependencies are specified with exact version ranges. This is good for ensuring reproducible builds. It might be worthwhile to occasionally verify that these versions remain compatible as the project and ecosystem evolve.
36-44: Scripts Section is Appropriately Configured
The defined scripts for linting, testing, and publishing are clear. Please double-check the custom commands (such asattw --pack) to ensure they work as intended in your development environment.
45-51: Module Type and Tshy Configuration are Correct
Setting the package type to “module” along with thetshyexports configuration indicates proper support for ES modules. This looks good and aligns with modern Node.js practices.
52-64: Exports Field Provides Dual Entry Points
The “exports” field correctly specifies both ESM and CommonJS entry points with their respective type definitions. This dual configuration enhances compatibility across different consumer environments.
65-71: Files, Main, and Module Entries are Accurate
Listing the “dist” and “src” directories in the files array—along with clear definitions for “types”, “main”, and “module”—ensures that the published package includes all necessary assets.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
README.md (2)
3-10: Badge Section Enhancement:
The added badges provide an effective at-a-glance overview of key project metrics (NPM version, downloads, license, code coverage, CI status, etc.), which is excellent for establishing credibility. Note that the markdown syntax here uses exclamation marks heavily; while common for badge images, please verify that this usage aligns with your style guidelines.🧰 Tools
🪛 LanguageTool
[style] ~3-~3: Using many exclamation marks might seem excessive (in this case: 5 exclamation marks for a text that’s 1214 characters long)
Context: ...//www.npmjs.com/package/read-env-value)
[
28-30: Parameter List Punctuation:
The bullet points describing the parameters are clear, but a consistent punctuation style (such as ending each description with a period) might enhance readability.🧰 Tools
🪛 LanguageTool
[uncategorized] ~28-~28: Loose punctuation mark.
Context: ...ber | boolean) #### Parameters -key: The environment variable key to read. -...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~29-~29: Loose punctuation mark.
Context: ...vironment variable key to read. -type: The type of the value to be returned. -...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~30-~30: Loose punctuation mark.
Context: ...e value to be returned. -defaultValue: The default value to return if the envi...(UNLIKELY_OPENING_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~3-~3: Using many exclamation marks might seem excessive (in this case: 5 exclamation marks for a text that’s 1214 characters long)
Context: ...//www.npmjs.com/package/read-env-value) 
(EN_EXCESSIVE_EXCLAMATION)
[uncategorized] ~28-~28: Loose punctuation mark.
Context: ...ber | boolean) #### Parameters - key: The environment variable key to read. -...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~29-~29: Loose punctuation mark.
Context: ...vironment variable key to read. - type: The type of the value to be returned. -...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~30-~30: Loose punctuation mark.
Context: ...e value to be returned. - defaultValue: The default value to return if the envi...
(UNLIKELY_OPENING_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Node.js / Test (windows-latest, 22)
🔇 Additional comments (4)
README.md (4)
11-12: Project Title Clarity:
The title "read-env-value" and the subtitle "Read env with default value" clearly communicate the package’s purpose. This straightforward approach improves first impressions.
13-21: Usage Section Demonstration:
The newly added "Usage" section is clear and concise. The TypeScript snippet effectively illustrates how to import and utilize theenvfunction with a default value, making it easy for users to get started.
22-43: API Documentation Detail:
The API section is well-organized and provides a detailed explanation of theenvfunction’s signature, parameters, return value, and usage example. This level of detail is very beneficial for users.
Consider a minor review of the formatting for consistency—for example, ensuring that list items and their punctuation are uniform throughout.
[nitpick, refactor_suggestion_good_to_have]🧰 Tools
🪛 LanguageTool
[uncategorized] ~28-~28: Loose punctuation mark.
Context: ...ber | boolean) #### Parameters -key: The environment variable key to read. -...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~29-~29: Loose punctuation mark.
Context: ...vironment variable key to read. -type: The type of the value to be returned. -...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~30-~30: Loose punctuation mark.
Context: ...e value to be returned. -defaultValue: The default value to return if the envi...(UNLIKELY_OPENING_PUNCTUATION)
44-53: License and Contributors Presentation:
The License section along with the Contributors badge is neatly presented and includes all necessary links. This enhances the document’s transparency and adheres to open-source best practices.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores