fix: read version from package.json instead of hardcoded string#18
Conversation
The --version flag was always printing "1.0.0" because the version
string in src/index.ts was never updated alongside package.json bumps.
Replaced it with a runtime require("../package.json").version so future
releases automatically reflect the correct version.
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request updates the CLI to dynamically load its version from package.json rather than using a hardcoded value. Feedback on the changes suggests moving the require statement below all ES import statements to maintain standard import ordering and prevent potential hoisting issues.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // eslint-disable-next-line @typescript-eslint/no-require-imports | ||
| const { version } = require("../package.json") as { version: string }; |
There was a problem hiding this comment.
Mixing require statements with ES import statements violates standard import ordering practices (such as the ESLint import/first rule).
In TypeScript/JavaScript, import statements are hoisted to the top of the module scope during compilation, whereas require is executed at runtime in the order it appears. This means the import statements below this line will actually execute before this require statement.
To maintain clean code structure and avoid potential linting or formatting issues, please move this require statement below all the import statements (after the import of registerLogoutCommand on line 19).
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | 0 |
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
The PR successfully addresses the goal of dynamically reading the project version from package.json, ensuring the --version flag remains synchronized with the release version. Codacy analysis indicates the changes are up to standards.
However, there are two primary concerns: the use of require for JSON loading in a file otherwise using ES Module imports (which can cause runtime issues and hoisting confusion), and a total reliance on manual testing. There is no automated verification that the relative path resolution will function correctly once the project is compiled to the dist folder, which is a high-risk area for this type of change.
About this PR
- The PR currently lacks automated tests to verify the version output and relative path resolution. Given that pathing often breaks between source and build directories, it is recommended to add a unit or integration test that executes the CLI with the
--versionflag.
Test suggestions
- Verify that executing the CLI with the
--versionflag returns the exact version string present in the project's package.json - Verify that the build process does not nest the output (e.g., dist/src/index.js) which would break the relative require path
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify that executing the CLI with the `--version` flag returns the exact version string present in the project's package.json
2. Verify that the build process does not nest the output (e.g., dist/src/index.js) which would break the relative require path
Low confidence findings
- The use of
requirefor a JSON file may cause issues if the project transitions to a strict ES Module (ESM) setup in the future. While currently suppressed by ESLint, this inconsistency should be addressed to avoid future compatibility issues.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| // eslint-disable-next-line @typescript-eslint/no-require-imports | ||
| const { version } = require("../package.json") as { version: string }; |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: Mixing require and import is inconsistent and will crash in native ES Module environments. Furthermore, because import statements are hoisted, they will always execute before this require call.
A more modern and type-safe approach is to use TypeScript's native import syntax. Try the following:
- Ensure
resolveJsonModuleis enabled intsconfig.json. - Replace the
requirecall with a standard import:import { version } from '../package.json';. - Move the import to the top of the file and remove the ESLint override.
Summary
codacy --versionwas always printing1.0.0because the version string insrc/index.tswas hardcoded and never updated with each releaserequire("../package.json").versionso it stays in sync automaticallyTest plan
npm run build && node dist/index.js --versionshould print1.3.1(after changeset version bump) or1.3.0on the current branchdist/index.jsat root of dist, not nested underdist/src/)🤖 Generated with Claude Code