refactor: extract walkAncestors, findNearestFile

playground/package.json

@@ -8,6 +8,6 @@
     "array-includes": "",
     "axios": "",
     "is-number": "",
-    "lodash": "^2.4.2"
+    "lodash": "catalog:"
   }
 }

src/commands/open-file-in-npmx.ts

@@ -1,6 +1,7 @@
+import { PACKAGE_JSON_BASENAME } from '#constants'
 import { logger } from '#state'
 import { npmxFileUrl } from '#utils/links'
-import { resolvePackageRelativePath } from '#utils/resolve'
+import { findNearestFile, resolvePackageJson } from '#utils/resolve'
 import { useActiveTextEditor } from 'reactive-vscode'
 import { env, Uri, window } from 'vscode'
 
@@ -23,14 +24,15 @@ export async function openFileInNpmx(fileUri?: Uri) {
   }
 
   // Find the associated package manifest and the relative path to the given file.
-  const result = await resolvePackageRelativePath(uri)
-  if (!result) {
+  const pkgJsonUri = await findNearestFile(PACKAGE_JSON_BASENAME, uri, (u) => u.path.endsWith('/node_modules'))
+  const manifest = pkgJsonUri ? await resolvePackageJson(pkgJsonUri) : undefined
+  if (!pkgJsonUri || !manifest) {
     logger.warn(`Could not resolve npmx url: ${uri.toString()}`)
     window.showWarningMessage(`npmx: Could not find package.json for ${uri.toString()}`)
     return
   }
-  const { manifest, relativePath } = result
 
+  const relativePath = uri.path.slice(pkgJsonUri.path.lastIndexOf('/') + 1)
   // Use line number only if the user is actively looking at the relevant file
   const openingActiveFile = !fileUri || fileUri.toString() === textEditor.value?.document.uri.toString()
 

src/utils/resolve.ts

@@ -1,87 +1,56 @@
 import { Uri, workspace } from 'vscode'
 
-/**
- * Resolves the relative path of a file within its package.
- *
- * @param uri The URI of the file to resolve.
- * @returns A promise that resolves to the package manifest and relative path,
- *     or `undefined` if not found.
- */
-export async function resolvePackageRelativePath(uri: Uri): Promise<{ manifest: PackageManifest, relativePath: string } | undefined> {
-  const result = await findPackageJson(uri)
-  if (!result)
-    return undefined
+export function* walkAncestors(start: Uri, shouldStop?: (uri: Uri) => boolean): Generator<Uri, void, void> {
+  let current = start
+  while (true) {
+    yield current
 
-  const { uri: pkgUri, manifest } = result
-  const relativePath = uri.path.slice(pkgUri.path.lastIndexOf('/') + 1)
+    if (shouldStop?.(current))
+      return
+
+    const parent = Uri.joinPath(current, '..')
+    if (parent.toString() === current.toString())
+      return
+
+    current = parent
+  }
+}
 
-  return { manifest, relativePath }
+export async function findNearestFile(filename: string, start: Uri, shouldStop?: (uri: Uri) => boolean): Promise<Uri | undefined> {
+  for (const dir of walkAncestors(start, shouldStop)) {
+    const fileUri = Uri.joinPath(dir, filename)
+    try {
+      await workspace.fs.stat(fileUri)
+      return fileUri
+    } catch {
+      continue
+    }
+  }
 }
 
 /** A parsed `package.json` manifest file. */
 interface PackageManifest {
   /** Package name. */
   name: string
-
   /** Package version specifier. */
   version: string
 }
 
 /**
- * Finds the nearest package.json file by searching upwards from the given URI.
+ * Reads and parses a `package.json` file.
  *
- * @param file The URI to start the search from.
- * @returns The URI and parsed content of the package.json, or `undefined` if
- *     not found.
+ * @param pkgJsonUri The URI of the `package.json` file.
+ * @returns A promise that resolves to the parsed manifest,
+ *     or `undefined` if the file is invalid or missing required fields.
  */
-async function findPackageJson(file: Uri): Promise<{ uri: Uri, manifest: PackageManifest } | undefined> {
-  // Start from the directory, so we don't look for
-  // `node_modules/foo/bar.js/package.json`
-  const startDir = Uri.joinPath(file, '..')
-
-  for (const dir of walkAncestors(startDir)) {
-    const pkgUri = Uri.joinPath(dir, 'package.json')
-
-    let pkg: Partial<PackageManifest> | undefined
-    try {
-      const content = await workspace.fs.readFile(pkgUri)
-      pkg = JSON.parse(new TextDecoder().decode(content)) as Partial<PackageManifest>
-    } catch {
-      continue
-    }
-
-    if (isValidManifest(pkg)) {
-      return {
-        uri: pkgUri,
-        manifest: pkg,
-      }
-    }
-  }
-
-  return undefined
-}
+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
 
-function* walkAncestors(start: Uri): Generator<Uri, void, void> {
-  let currentUri = start
-  while (true) {
-    yield currentUri
-
-    if (currentUri.path.endsWith('/node_modules'))
+    if (!manifest || !manifest.name || !manifest.version)
       return
 
-    const parentUri = Uri.joinPath(currentUri, '..')
-    if (parentUri.toString() === currentUri.toString())
-      return
-
-    currentUri = parentUri
-  }
-}
-
-/**
- * Check for valid package manifest, as it might be a manifest which just
- * configures a setting without really being a package (such as
- * `{sideEffects: false}`).
- */
-function isValidManifest(json: Partial<PackageManifest>): json is PackageManifest {
-  return Boolean(json && json.name && json.version)
+    return manifest
+  } catch {}
 }

tests/__mocks__/filesystem.ts

@@ -16,6 +16,14 @@ export function mockFileSystem(files: Record<string, string>) {
     }
   }
 
+  vi.mocked(workspace.fs.stat).mockImplementation(async (uri) => {
+    const path = uri.path
+    if (files[path] === undefined) {
+      throw new Error(`File not found: ${uri.toString()}`)
+    }
+    return {} as any
+  })
+
   vi.mocked(workspace.fs.readFile).mockImplementation(async (uri) => {
     const path = uri.path
     const content = files[path]

tests/resolve.test.ts

@@ -1,130 +1,77 @@
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { Uri } from 'vscode'
-import { resolvePackageRelativePath } from '../src/utils/resolve'
+import { findNearestFile, walkAncestors } from '../src/utils/resolve'
 import { mockFileSystem } from './__mocks__/filesystem'
 
-describe('resolvePackageRelativePath', () => {
-  beforeEach(() => {
-    vi.resetAllMocks()
+describe('walkAncestors', () => {
+  it('should yield all ancestor directories', () => {
+    const uri = Uri.file('/a/b/c/file.js')
+    const ancestors = [...walkAncestors(uri)]
+    expect(ancestors.map((u) => u.path)).toEqual([
+      '/a/b/c/file.js',
+      '/a/b/c',
+      '/a/b',
+      '/a',
+      '/',
+    ])
   })
 
-  it('should resolve simple package file', async () => {
-    mockFileSystem({
-      '/root/node_modules/pkg/package.json': JSON.stringify({
-        name: 'pkg',
-        version: '1.0.0',
-      }),
-    })
-
-    const uri = Uri.file('/root/node_modules/pkg/src/index.js')
-    const result = await resolvePackageRelativePath(uri)
-
-    expect(result).toBeDefined()
-
-    const { manifest, relativePath } = result!
-    expect(manifest).toEqual({ name: 'pkg', version: '1.0.0' })
-    expect(relativePath).toBe('src/index.js')
+  it('should stop when shouldStop returns true', () => {
+    const uri = Uri.file('/a/b/c/file.js')
+    const ancestors = [...walkAncestors(uri, (u) => u.path === '/a/b')]
+    expect(ancestors.map((u) => u.path)).toEqual([
+      '/a/b/c/file.js',
+      '/a/b/c',
+      '/a/b',
+    ])
   })
 
-  it('should handle bundled dependencies', async () => {
-    mockFileSystem({
-      '/root/node_modules/parent/package.json': JSON.stringify({
-        name: 'parent',
-        version: '1.0.0',
-      }),
-      '/root/node_modules/parent/node_modules/child/package.json': JSON.stringify({
-        name: 'child',
-        version: '2.0.0',
-      }),
-    })
-
-    const uri = Uri.file('/root/node_modules/parent/node_modules/child/index.js')
-    const result = await resolvePackageRelativePath(uri)
-
-    expect(result).toBeDefined()
+  it('should handle root URI', () => {
+    const uri = Uri.file('/')
+    const ancestors = [...walkAncestors(uri)]
+    expect(ancestors.map((u) => u.path)).toEqual(['/'])
+  })
+})
 
-    const { manifest, relativePath } = result!
-    expect(manifest).toEqual({ name: 'child', version: '2.0.0' })
-    expect(relativePath).toBe('index.js')
+describe('findNearestFile', () => {
+  beforeEach(() => {
+    vi.resetAllMocks()
   })
 
-  it('should handle pnpm structure', async () => {
+  it('should find a file in a parent directory', async () => {
     mockFileSystem({
-      '/root/.pnpm/pkg@1.0.0/node_modules/pkg/package.json': JSON.stringify({
-        name: 'pkg',
-        version: '1.0.0',
-      }),
+      '/a/b/target.txt': '',
     })
 
-    const uri = Uri.file('/root/.pnpm/pkg@1.0.0/node_modules/pkg/src/index.js')
-    const result = await resolvePackageRelativePath(uri)
-
+    const result = await findNearestFile('target.txt', Uri.file('/a/b/c/d'))
     expect(result).toBeDefined()
-
-    const { manifest, relativePath } = result!
-    expect(manifest).toEqual({ name: 'pkg', version: '1.0.0' })
-    expect(relativePath).toBe('src/index.js')
+    expect(result!.path).toBe('/a/b/target.txt')
   })
 
-  it('should handle scoped packages', async () => {
+  it('should return the closest match', async () => {
     mockFileSystem({
-      '/root/node_modules/@scope/pkg/package.json': JSON.stringify({
-        name: '@scope/pkg',
-        version: '1.0.0',
-      }),
+      '/a/target.txt': '',
+      '/a/b/c/target.txt': '',
     })
 
-    const uri = Uri.file('/root/node_modules/@scope/pkg/src/index.js')
-    const result = await resolvePackageRelativePath(uri)
-
+    const result = await findNearestFile('target.txt', Uri.file('/a/b/c/d'))
     expect(result).toBeDefined()
-
-    const { manifest, relativePath } = result!
-    expect(manifest).toEqual({ name: '@scope/pkg', version: '1.0.0' })
-    expect(relativePath).toBe('src/index.js')
+    expect(result!.path).toBe('/a/b/c/target.txt')
   })
 
-  it('should return undefined if no package.json found', async () => {
+  it('should return undefined when file is not found', async () => {
     mockFileSystem({})
 
-    const uri = Uri.file('/root/no-pkg/file.js')
-    const result = await resolvePackageRelativePath(uri)
+    const result = await findNearestFile('target.txt', Uri.file('/a/b/c'))
     expect(result).toBeUndefined()
   })
 
-  it('should return undefined even when a package.json exists outside the node_modules directory', async () => {
+  it('should respect shouldStop', async () => {
     mockFileSystem({
-      '/root/package.json': JSON.stringify({
-        name: 'root',
-        version: '1.0.0',
-      }),
+      '/a/target.txt': '',
     })
 
-    const uri = Uri.file('/root/node_modules/pkg/file.js')
-    const result = await resolvePackageRelativePath(uri)
+    const result = await findNearestFile('target.txt', Uri.file('/a/b/c'), (u) => u.path === '/a/b')
     expect(result).toBeUndefined()
   })
-
-  it('should skip invalid manifests', async () => {
-    mockFileSystem({
-      '/root/node_modules/pkg/package.json': JSON.stringify({
-        name: 'pkg',
-        version: '1.0.0',
-      }),
-
-      // Context: Side effects is often configured per-directory as the only key
-      // in a `package.json`, but it does not actually represent a real package.
-      '/root/node_modules/pkg/src/package.json': JSON.stringify({
-        sideEffects: false,
-      }),
-    })
-
-    const uri = Uri.file('/root/node_modules/pkg/src/index.js')
-    const result = await resolvePackageRelativePath(uri)
-    expect(result).toBeDefined()
-
-    const { manifest, relativePath } = result!
-    expect(manifest).toEqual({ name: 'pkg', version: '1.0.0' })
-    expect(relativePath).toBe('src/index.js')
-  })
 })