Skip to content

Comments

refactor: extract walkAncestors, findNearestFile#41

Merged
9romise merged 1 commit intomainfrom
refactor/resolve
Feb 12, 2026
Merged

refactor: extract walkAncestors, findNearestFile#41
9romise merged 1 commit intomainfrom
refactor/resolve

Conversation

@9romise
Copy link
Member

@9romise 9romise commented Feb 12, 2026

No description provided.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 12, 2026

📝 Walkthrough

Walkthrough

This pull request refactors the package resolution logic across the codebase. The changes introduce two new utility functions, walkAncestors and findNearestFile, in the resolve module to replace the previous resolvePackageRelativePath method. A new resolvePackageJson function is added to handle package manifest parsing. The open-file-in-npmx command is updated to use these new utilities with additional error handling. The PackageManifest interface is modified to remove the name property. Mock filesystem utilities are enhanced to support file existence checks, and test suites are updated accordingly. Additionally, a dependency specification in the playground package configuration is updated.

Possibly related PRs

🚥 Pre-merge checks | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The pull request has no description provided by the author, making it impossible to assess whether intent is clearly communicated. Add a pull request description explaining the refactoring rationale, the new utilities being extracted, and how they improve the codebase.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/resolve

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
src/utils/resolve.ts (1)

46-56: Consider logging or narrowing the catch block for easier debugging.

The empty catch {} silently swallows all errors, including unexpected ones (e.g., JSON parse errors on malformed content, permission issues). While returning undefined is appropriate for missing/invalid files, distinguishing between "file not found" and "unexpected error" could aid debugging.

♻️ Optional: Add minimal logging for unexpected errors
 export async function resolvePackageJson(pkgJsonUri: Uri): Promise<PackageManifest | undefined> {
   try {
     const content = await workspace.fs.readFile(pkgJsonUri)
     const manifest = JSON.parse(new TextDecoder().decode(content)) as PackageManifest

     if (!manifest || !manifest.name || !manifest.version)
       return

     return manifest
-  } catch {}
+  } catch {
+    // File missing, unreadable, or malformed JSON - caller handles undefined
+  }
 }
src/commands/open-file-in-npmx.ts (1)

35-35: Consider clarifying the relative path derivation.

The current logic is correct but relies on the implicit equivalence: pkgJsonUri.path.lastIndexOf('/') + 1 equals the length of the package directory plus one. Using Uri.joinPath or explicitly computing the directory path would make the intent clearer.

♻️ Optional: Make the intent explicit
-  const relativePath = uri.path.slice(pkgJsonUri.path.lastIndexOf('/') + 1)
+  const pkgDir = Uri.joinPath(pkgJsonUri, '..')
+  const relativePath = uri.path.slice(pkgDir.path.length + 1)

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


Comment @coderabbitai help to get the list of available commands and usage tips.

@9romise 9romise merged commit 0b7c4af into main Feb 12, 2026
9 checks passed
@9romise 9romise deleted the refactor/resolve branch February 12, 2026 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant