fix env vars string bug #24
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an environment variables string bug and enhances the test coverage for environment parsing. Key changes include:
- Adding comprehensive tests for different environment variable formats and scenarios in test/utils/secrets.test.ts.
- Updating the parseEnv function in src/utils/secrets.ts to correctly strip quotation marks.
- Bumping version numbers and adding Jest configuration in src/index.ts and package.json.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/utils/secrets.test.ts | Introduces tests to cover various env variable scenarios. |
| src/utils/secrets.ts | Fixes env var string parsing by stripping quotes from values. |
| src/index.ts | Updates CLI version from v1.0.8 to v1.0.9. |
| package.json | Updates version, adds Jest config, and bumps related dev dependencies. |
| } | ||
|
|
||
| // Strip quotation marks from value if present (both single and double quotes) | ||
| if (value.length > 1 && |
There was a problem hiding this comment.
It's not a simple problem and needs more consideration. It's dangerous to strip quotation marks blindly. Consider the two cases
- One wrote the quotations but he doesn't mean to
- One wrote the quotations and he means to do so
When no striping, 1 doesn't work but 2 works. A simple stripping would enable case 1, but not 2. So, in either way we don't support the both cases.
It turns out this is common and well considered problem. You can read the rule used by dotenv package to learn how it's handled.
In this case, I strongly suggest that instead writing it on our own, we can use the original dotenv package's parse feature to handle it properly
No description provided.