fix: resolve package.json path correctly in bundled build#88
Conversation
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Multi-path version resolution src/utils/version-check.ts |
getInstalledVersion() exported and updated to iterate two candidate package.json paths (production build and development layout), parse each, return the first valid string version, and fall back to '0.0.0' if none succeed. |
Tests for version resolution src/__tests__/version-check.test.ts |
Adds tests that mock node:fs.readFileSync, cover production vs development path selection, malformed JSON, missing/invalid version, and fallback behavior; ensures per-test mock reset for isolation. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- KDM-cli/kdm-cli#29: Modifies the same
version-check.tsutility and affects installed-version extraction used by update notifications.
Poem
Two package paths now take their chance,
read, parse, and gracefully advance—
when versions hide or files are shy,
a fallback hums:0.0.0nearby. ✨
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately describes the main change: exporting and updating getInstalledVersion() to resolve package.json from multiple paths in bundled builds. |
| Linked Issues check | ✅ Passed | The implementation directly addresses issue #47 by probing multiple package.json paths to correctly resolve the version in both production and development contexts, fixing the 0.0.0 display issue. |
| Out of Scope Changes check | ✅ Passed | All changes are strictly scoped to version resolution: exporting getInstalledVersion(), adding multi-path logic, and comprehensive test coverage—no unrelated modifications present. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
📋 Issue Planner
Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
View plan for ticket: #47
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Pull request overview
Fixes the update notification’s “Current Version : v0.0.0” bug by making runtime version detection locate the correct package.json in both bundled (dist) and source (src) execution contexts.
Changes:
- Updated
getInstalledVersion()to probe multiple candidatepackage.jsonlocations relative to the runtime module path. - Falls back to
0.0.0only after exhausting all candidate paths.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const paths = [ | ||
| join(__dirname, '..', 'package.json'), // production: dist/../package.json | ||
| join(__dirname, '..', '..', 'package.json') // development: src/utils/../../package.json | ||
| ]; | ||
| for (const pkgPath of paths) { | ||
| try { | ||
| const pkg = JSON.parse(readFileSync(pkgPath, 'utf-8')); | ||
| if (pkg.version) return pkg.version; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/version-check.ts`:
- Around line 16-23: The loop that reads package.json files currently returns
pkg.version without type-checking; update the code in the for-loop that reads
pkgPath/JSON.parse so that you only return pkg.version if typeof pkg.version ===
'string' (otherwise continue to next path), ensuring compareSemver (which calls
.replace()) always receives a string; do not coerce non-string values—skip them
and try the next path or fall back to the existing default behavior.
- Around line 11-25: Add Vitest tests that exercise every branch of
getInstalledVersion(): mock node:fs.readFileSync with vi.mock('node:fs') and
return fixture contents for the "production" path (valid package.json with
version) and the "development" path (valid package.json with version) to assert
correct version is returned; add mocks that return malformed JSON to assert the
try/catch advances to the next path; add mocks that return valid JSON without a
version field to assert it continues searching; and add mocks that throw for
both paths to assert the function falls back to '0.0.0'. Reference
getInstalledVersion() in src/utils/version-check.ts and use fixtures/return
values keyed to the two join(__dirname, ...) paths to ensure each branch is
covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 72ddbb32-1dee-4528-ba57-cc845dee9d05
📒 Files selected for processing (1)
src/utils/version-check.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/__tests__/version-check.test.ts`:
- Around line 113-115: The mocked readFileSync implementation uses
path.endsWith(...) assuming POSIX separators; normalize the incoming path before
the suffix checks to make tests OS-portable by converting backslashes to forward
slashes or using path.normalize()/path.posix methods. Update the
vi.mocked(readFileSync).mockImplementation callback (and the other similar mock
implementations at the other locations) to coerce the path (e.g., const p =
String(path).replace(/\\\\/g, '/')) and then use p.endsWith('src/package.json')
(and the other suffixes) so the tests pass on Windows.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3ab71436-a6c8-48ee-8283-6b9b5c91f84c
📒 Files selected for processing (2)
src/__tests__/version-check.test.tssrc/utils/version-check.ts
| vi.mocked(readFileSync).mockImplementation((path) => { | ||
| if (typeof path === 'string' && path.endsWith('src/package.json')) { | ||
| return JSON.stringify({ version: '2.3.4' }); |
There was a problem hiding this comment.
Normalize mocked file paths before suffix checks.
Line 114/125/136/150 style checks assume / separators, so these tests can fail on Windows (\). Normalize path before endsWith() to make tests portable.
💡 Suggested fix
+const normalizePath = (p: unknown) => String(p).replace(/\\/g, '/');
it('should resolve and return the version if package.json in production path is valid', () => {
vi.mocked(readFileSync).mockImplementation((path) => {
- if (typeof path === 'string' && path.endsWith('src/package.json')) {
+ const normalized = normalizePath(path);
+ if (normalized.endsWith('src/package.json')) {
return JSON.stringify({ version: '2.3.4' });
}
throw new Error('File not found');
});
expect(getInstalledVersion()).toBe('2.3.4');
});Also applies to: 125-126, 136-140, 150-154
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/__tests__/version-check.test.ts` around lines 113 - 115, The mocked
readFileSync implementation uses path.endsWith(...) assuming POSIX separators;
normalize the incoming path before the suffix checks to make tests OS-portable
by converting backslashes to forward slashes or using
path.normalize()/path.posix methods. Update the
vi.mocked(readFileSync).mockImplementation callback (and the other similar mock
implementations at the other locations) to coerce the path (e.g., const p =
String(path).replace(/\\\\/g, '/')) and then use p.endsWith('src/package.json')
(and the other suffixes) so the tests pass on Windows.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. An unexpected error occurred while generating fixes: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
Resolves #47
This PR resolves the issue where the update notification displays
Current Version : v0.0.0instead of the actual installed version (e.g.,1.2.2).getInstalledVersioninsidesrc/utils/version-check.tsto probe multiple relative paths:dist/../package.json(production/bundled build)src/utils/../../package.json(development/testing)Summary by CodeRabbit