fix: search both Unix and Windows paths for npm global module#11
Conversation
npm global install layout differs by platform:
Unix: {prefix}/lib/node_modules/utoo/package.json
Windows: {prefix}/node_modules/utoo/package.json
getUtooVersion() now tries both candidates, fixing "package.json
could not be found or read" on Windows runners.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the getUtooVersion function by adding support for different npm global layouts on Unix and Windows, allowing it to correctly locate the package.json file. The review feedback suggests further improvements to the implementation, specifically addressing the dynamic import within a loop, the mixing of synchronous and asynchronous I/O operations, and the silent swallowing of errors, to enhance robustness and clarity.
| for (const candidate of candidates) { | ||
| try { | ||
| if (!existsSync(candidate)) continue; | ||
|
|
||
| // Read and parse package.json | ||
| const fs = await import("node:fs/promises"); | ||
| const content = await fs.readFile(packageJsonPath, "utf-8"); | ||
| const packageJson = JSON.parse(content); | ||
| const fs = await import("node:fs/promises"); | ||
| const content = await fs.readFile(candidate, "utf-8"); | ||
| const packageJson = JSON.parse(content); | ||
|
|
||
| return packageJson.version; | ||
| } catch (error) { | ||
| // If reading package.json fails, utoo might not be properly installed | ||
| return undefined; | ||
| if (packageJson.version) return packageJson.version; | ||
| } catch { | ||
| continue; | ||
| } | ||
| } |
There was a problem hiding this comment.
This implementation can be improved in a few ways for better robustness and clarity:
- Dynamic import in loop: The
import("node:fs/promises")is called inside the loop. It's more efficient to call it once outside. - Mixing sync/async IO: The code mixes a synchronous
existsSyncwith asynchronousreadFile. It's cleaner to use a fully asynchronous approach by removingexistsSyncand handling the 'file not found' error in thecatchblock. - Silent error swallowing: The empty
catchblock hides potentially useful error information (e.g., for malformed JSON or permission errors), making debugging harder.
Here is a suggested refactoring that addresses these points:
const fs = await import("node:fs/promises");
for (const candidate of candidates) {
try {
const content = await fs.readFile(candidate, "utf-8");
const packageJson = JSON.parse(content);
if (packageJson.version) {
return packageJson.version;
}
} catch (error: any) {
// It's expected that a candidate path might not exist (ENOENT).
// However, other errors like parsing or permission issues are worth reporting.
if (error.code !== 'ENOENT') {
warning(`Failed to process package.json candidate '${candidate}': ${error.message}`);
}
}
}1 participant
Summary
getUtooVersion()only searched{prefix}/lib/node_modules/utoo/package.json(Unix layout), but Windows npm uses{prefix}/node_modules/utoo/package.json(nolib/).This caused "Utoo was installed but package.json could not be found or read" on all Windows CI runners.
Now tries both candidate paths.
Test plan
utoopm-ci-x86_64-pc-windows-msvc) passes Setup Utoo step🤖 Generated with Claude Code