feat(view): support searching package.json upward when package name is omitted#11696
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces ChangesDependency Migration and Feature Enhancement
Sequence Diagram(s)sequenceDiagram
participant CLI as pnpm CLI
participant Handler as view.handler
participant Finder as empathic/package
participant FS as node:fs
CLI->>Handler: invoke `pnpm view` without package arg
Handler->>Finder: search upward for nearest `package.json`
Finder->>Handler: return package.json path or nothing
Handler->>FS: read package.json
Handler->>Handler: parse & validate `name` -> set packageSpec or throw
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
deps/inspection/commands/test/view.ts (1)
228-244: ⚡ Quick winAdd an explicit parent-directory traversal assertion.
Line 230 places
package.jsonin the same directory ascwd, so this only validates direct lookup. Add a nestedcwdcase to verify upward search behavior.Proposed test adjustment
test('view: uses package.json name when no package name provided', async () => { const cwd = process.cwd() const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'view-test-')) const pkgJsonPath = path.join(tmpDir, 'package.json') + const nestedDir = path.join(tmpDir, 'a', 'b') try { fs.writeFileSync(pkgJsonPath, JSON.stringify({ name: 'is-negative' })) - process.chdir(tmpDir) + fs.mkdirSync(nestedDir, { recursive: true }) + process.chdir(nestedDir) const result = await view.handler(VIEW_OPTIONS as unknown as Config & ConfigContext, []) expect(typeof result).toBe('string') expect(result).toContain('is-negative')🤖 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 `@deps/inspection/commands/test/view.ts` around lines 228 - 244, The test "view: uses package.json name when no package name provided" currently places package.json in the same directory as cwd and so only validates direct lookup; update the test to also assert upward-search behavior by creating a nested directory (e.g., tmpDir/subdir), writing package.json to tmpDir, chdir into the nested subdir before calling view.handler (the handler function in view), then assert the returned string contains the package name; ensure the existing cleanup still restores cwd and removes tmp files and reuse VIEW_OPTIONS / Config & ConfigContext as before.
🤖 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 `@deps/inspection/commands/src/view/index.ts`:
- Around line 59-63: Wrap the JSON.parse of pkgFile in a try/catch and convert
any thrown SyntaxError (or other parse errors) into a PnpmError with code
'INVALID_PACKAGE_JSON'; after parsing, validate that pkgInfo.name is a string
(e.g., typeof pkgInfo.name === 'string') and throw the same
PnpmError('INVALID_PACKAGE_JSON', ...) when it's missing or not a string before
assigning packageSpec = pkgInfo.name so both malformed JSON and non-string name
values are handled consistently (references: pkgInfo, pkgFile, PnpmError,
packageSpec).
---
Nitpick comments:
In `@deps/inspection/commands/test/view.ts`:
- Around line 228-244: The test "view: uses package.json name when no package
name provided" currently places package.json in the same directory as cwd and so
only validates direct lookup; update the test to also assert upward-search
behavior by creating a nested directory (e.g., tmpDir/subdir), writing
package.json to tmpDir, chdir into the nested subdir before calling view.handler
(the handler function in view), then assert the returned string contains the
package name; ensure the existing cleanup still restores cwd and removes tmp
files and reuse VIEW_OPTIONS / Config & ConfigContext as before.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5a8912f6-9548-43fe-bf59-b1924c67112f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.changeset/view-package-json-upward-search.mddeps/inspection/commands/package.jsondeps/inspection/commands/src/view/index.tsdeps/inspection/commands/test/view.tspnpm-workspace.yamlworkspace/projects-filter/package.jsonworkspace/projects-filter/src/getChangedProjects.tsworkspace/root-finder/package.jsonworkspace/root-finder/src/index.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: ubuntu-latest / Node.js 24 / Test
- GitHub Check: Agent
- GitHub Check: Run benchmark on ubuntu-latest
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use Standard Style with modifications: trailing commas are used, functions are preferred over classes, functions are declared after they are used (hoisting is relied upon).
Functions should have no more than two or three arguments. If a function needs more parameters, use a single options object instead.
Maintain import order: (1) Standard libraries, (2) External dependencies (sorted alphabetically), (3) Relative imports.
Files:
deps/inspection/commands/test/view.tsworkspace/projects-filter/src/getChangedProjects.tsworkspace/root-finder/src/index.tsdeps/inspection/commands/src/view/index.ts
🧠 Learnings (2)
📚 Learning: 2026-05-05T23:03:04.286Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11479
File: __utils__/scripts/package.json:6-9
Timestamp: 2026-05-05T23:03:04.286Z
Learning: The pattern cross-env NODE_OPTIONS="$NODE_OPTIONS ..." in package.json scripts is an established convention in the pnpm/pnpm repository and is used across many packages (e.g., fs/hard-link-dir, worker, __utils__/scripts). Do not flag this as a cross-platform issue in individual files; if a change is needed, apply it as a repo-wide change in a separate PR. Scope this guidance to all package.json files in the repo; use the minimatch pattern '**/package.json' to identify relevant files and review changes at the repository level rather than per-file.
Applied to files:
workspace/projects-filter/package.jsondeps/inspection/commands/package.jsonworkspace/root-finder/package.json
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.
Applied to files:
deps/inspection/commands/test/view.tsworkspace/projects-filter/src/getChangedProjects.tsworkspace/root-finder/src/index.tsdeps/inspection/commands/src/view/index.ts
🔇 Additional comments (9)
.changeset/view-package-json-upward-search.md (1)
1-13: LGTM!pnpm-workspace.yaml (1)
183-183: LGTM!workspace/root-finder/package.json (1)
35-35: LGTM!workspace/projects-filter/package.json (1)
39-39: LGTM!workspace/root-finder/src/index.ts (1)
5-5: LGTM!Also applies to: 23-23
workspace/projects-filter/src/getChangedProjects.ts (1)
7-7: LGTM!Also applies to: 24-25
deps/inspection/commands/package.json (1)
61-61: LGTM!deps/inspection/commands/src/view/index.ts (1)
1-33: LGTM!Also applies to: 54-58, 64-66
deps/inspection/commands/test/view.ts (1)
1-4: LGTM!Also applies to: 246-262
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates pnpm view to mimic npm view more closely by allowing the package name to be omitted and then resolving it from the nearest package.json found by searching upward from the working directory. It also standardizes “find up” behavior across a couple of workspace utilities by replacing find-up with empathic.
Changes:
pnpm view: if no package name is provided, locate the nearestpackage.jsonupward and use itsname.- Replace
find-upwithempathicin workspace root-finding and projects filtering utilities. - Add tests and a changeset documenting the new
pnpm viewbehavior and dependency swap.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
workspace/root-finder/src/index.ts |
Switches workspace manifest upward-search from find-up to empathic/find. |
workspace/root-finder/package.json |
Replaces find-up dependency with empathic. |
workspace/projects-filter/src/getChangedProjects.ts |
Switches .git upward-search from find-up to empathic/find. |
workspace/projects-filter/package.json |
Replaces find-up dependency with empathic. |
pnpm-workspace.yaml |
Adds empathic to catalog and removes find-up from catalog. |
pnpm-lock.yaml |
Updates lockfile for the empathic addition and find-up@8 removal. |
deps/inspection/commands/src/view/index.ts |
Implements package-name omission by reading name from nearest package.json found upward. |
deps/inspection/commands/package.json |
Adds empathic dependency (for empathic/package). |
deps/inspection/commands/test/view.ts |
Adds tests covering package-name omission behavior and invalid package.json cases. |
.changeset/view-package-json-upward-search.md |
Declares a minor release for the new view behavior (and documents the dep swap). |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
deps/inspection/commands/src/view/index.ts:63
- When deriving the package name from the nearest package.json, JSON.parse/readFileSync errors will currently surface as raw Node/SyntaxError exceptions. Consider wrapping the read+parse in try/catch and throwing a PnpmError('INVALID_PACKAGE_JSON', ...) that includes the file path, and also validate that "name" is a non-empty string before assigning it to packageSpec.
const pkgFile = pkg.up()
if (pkgFile) {
const pkgInfo = JSON.parse(fs.readFileSync(pkgFile, 'utf8'))
if (!pkgInfo.name) {
throw new PnpmError('INVALID_PACKAGE_JSON', 'Invalid package.json. No "name" field.')
}
packageSpec = pkgInfo.name
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
deps/inspection/commands/src/view/index.ts:63
- Reading and parsing the discovered
package.jsonviafs.readFileSync+JSON.parsecan throw rawSyntaxError/IO errors, which will bypass pnpm’s usual error codes/messages for bad manifests. Also, the current check only tests truthiness; it won’t reject non-stringnamevalues.
Use an existing manifest reader (e.g. @pnpm/pkg-manifest.reader or readProjectManifestOnly) to parse/normalize and then validate that name is a non-empty string, wrapping failures in a PnpmError with a consistent code and file path.
const pkgInfo = JSON.parse(fs.readFileSync(pkgFile, 'utf8'))
if (!pkgInfo.name) {
throw new PnpmError('INVALID_PACKAGE_JSON', 'Invalid package.json. No "name" field.')
}
packageSpec = pkgInfo.name
deps/inspection/commands/src/view/index.ts:66
- The thrown
MISSING_PACKAGE_NAMEmessage still saysUsage: pnpm view <package-name>, buthelp()now documents that the package name is optional (and can be inferred from the nearest package.json). Please update the error message (or add a hint) so it matches the updated command usage/behavior.
} else {
throw new PnpmError('MISSING_PACKAGE_NAME', 'Package name is required. Usage: pnpm view <package-name>')
}
Contributor
Integrated-Benchmark Report (Linux)Each scenario has pacquet rows (direct install) and pnpr rows (the same client through the pnpr install accelerator), so pnpr@HEAD vs pacquet@HEAD is the pnpr-vs-direct ratio. Cold-store scenarios wipe the client store between runs (warm server); hot-store scenarios keep it warm. The pacquet@HEAD rows feed the pacquet Bencher testbed; the pnpr@HEAD rows feed the pnpr testbed. Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.79645697244,
"stddev": 0.09151047213947237,
"median": 4.76154443234,
"user": 2.3232240400000004,
"system": 3.749419600000001,
"min": 4.72398162134,
"max": 4.97353472234,
"times": [
4.79758276134,
4.81276151134,
4.94584465734,
4.76983838434,
4.72535902534,
4.73504591434,
4.72398162134,
4.72737064634,
4.97353472234,
4.75325048034
]
},
{
"command": "pacquet@main",
"mean": 4.768520172740001,
"stddev": 0.03805235985176815,
"median": 4.75975765834,
"user": 2.32074504,
"system": 3.7418074,
"min": 4.72350816334,
"max": 4.85878548634,
"times": [
4.78441611234,
4.75473961634,
4.74622695734,
4.74680533134,
4.85878548634,
4.73917967134,
4.79144987734,
4.77531481134,
4.72350816334,
4.76477570034
]
},
{
"command": "pnpr@HEAD",
"mean": 2.0243679253400004,
"stddev": 0.10841921840001591,
"median": 1.98061107484,
"user": 2.55260894,
"system": 3.2822381999999997,
"min": 1.9336836853400001,
"max": 2.30147135234,
"times": [
1.96033230734,
1.9598981103400002,
2.30147135234,
2.08811305634,
1.9775188643400001,
1.98370328534,
1.9336836853400001,
1.96101030234,
2.04827262134,
2.02967566834
]
},
{
"command": "pnpr@main",
"mean": 2.02501504734,
"stddev": 0.0435368159396114,
"median": 2.01362742234,
"user": 2.53574074,
"system": 3.2980364,
"min": 1.9538879113400003,
"max": 2.0888788323400003,
"times": [
2.01154557934,
2.06190412334,
2.08349236434,
2.03948377534,
2.0888788323400003,
2.00459161034,
1.9538879113400003,
1.9797726463400003,
2.01570926534,
2.01088436534
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6577674963600002,
"stddev": 0.031594509649220184,
"median": 0.6450403695600001,
"user": 0.3479935,
"system": 1.3097192199999999,
"min": 0.6360831680600001,
"max": 0.7400004680600001,
"times": [
0.7400004680600001,
0.6678912330600001,
0.6745211120600001,
0.6360860690600001,
0.6484109350600001,
0.6469636240600001,
0.6429473550600001,
0.6431171150600001,
0.6360831680600001,
0.6416538840600001
]
},
{
"command": "pacquet@main",
"mean": 0.6486954222600001,
"stddev": 0.01838080543333596,
"median": 0.6403374195600001,
"user": 0.344821,
"system": 1.30809672,
"min": 0.62899612006,
"max": 0.6834822710600001,
"times": [
0.6705394060600001,
0.6339601770600001,
0.6399225260600001,
0.6460091230600001,
0.62899612006,
0.6363100410600001,
0.6834822710600001,
0.6676605190600001,
0.6407523130600001,
0.63932172606
]
},
{
"command": "pnpr@HEAD",
"mean": 0.66595607786,
"stddev": 0.04117827678301331,
"median": 0.6536848670600002,
"user": 0.34746299999999997,
"system": 1.30799472,
"min": 0.6335087110600001,
"max": 0.7758429940600001,
"times": [
0.7758429940600001,
0.65937177706,
0.6631831990600001,
0.6335087110600001,
0.6859369250600001,
0.6477825690600001,
0.6530729880600001,
0.6466166190600001,
0.6542967460600001,
0.6399482500600001
]
},
{
"command": "pnpr@main",
"mean": 0.7106145981600002,
"stddev": 0.04455339063065789,
"median": 0.7086956165600001,
"user": 0.3477109,
"system": 1.3057344199999998,
"min": 0.6483647750600001,
"max": 0.8066424140600001,
"times": [
0.8066424140600001,
0.6960600710600001,
0.7289827290600001,
0.6483647750600001,
0.7213311620600001,
0.7365016270600001,
0.6934963760600001,
0.72858629906,
0.6720208510600001,
0.6741596770600001
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.1139616833600003,
"stddev": 0.03714015288128704,
"median": 2.1113083357600004,
"user": 3.3519568800000004,
"system": 2.9881123,
"min": 2.0644774062600004,
"max": 2.19705502526,
"times": [
2.13764747526,
2.1054337702600003,
2.11601533926,
2.0644774062600004,
2.1083568732600004,
2.1006310962600003,
2.06874725326,
2.11425979826,
2.19705502526,
2.12699279626
]
},
{
"command": "pacquet@main",
"mean": 2.0955745230600002,
"stddev": 0.029181950449622625,
"median": 2.10399078976,
"user": 3.37394238,
"system": 2.9512522,
"min": 2.0556099802600003,
"max": 2.13607773826,
"times": [
2.08316751226,
2.0588200902600002,
2.1095974232600003,
2.12693647726,
2.06183517326,
2.0983841562600003,
2.13607773826,
2.0556099802600003,
2.1102486232600004,
2.11506805626
]
},
{
"command": "pnpr@HEAD",
"mean": 2.11949856986,
"stddev": 0.029241414231693838,
"median": 2.1190073642600002,
"user": 3.3542757800000005,
"system": 3.0245532999999996,
"min": 2.0669081842600003,
"max": 2.15425185126,
"times": [
2.15425185126,
2.1335117512600004,
2.0669081842600003,
2.15160420826,
2.0991561032600004,
2.14779552726,
2.0965411312600004,
2.13658878726,
2.1041251772600003,
2.10450297726
]
},
{
"command": "pnpr@main",
"mean": 2.1072926023600003,
"stddev": 0.0154916682204988,
"median": 2.1077760847600002,
"user": 3.34211838,
"system": 3.0191242,
"min": 2.08489068726,
"max": 2.1354406002600004,
"times": [
2.10463259526,
2.0967087652600003,
2.1354406002600004,
2.08489068726,
2.08937225226,
2.1163397452600003,
2.11091957426,
2.09955885926,
2.1230565452600003,
2.1120063992600002
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.22907706022,
"stddev": 0.010132575735727518,
"median": 1.22831736192,
"user": 1.3471055199999997,
"system": 1.67793974,
"min": 1.21317291192,
"max": 1.2447027919200002,
"times": [
1.23424683792,
1.2274729769200001,
1.2250662109200001,
1.21317291192,
1.22916174692,
1.2170810219200001,
1.23047362692,
1.2254086939200002,
1.24398378292,
1.2447027919200002
]
},
{
"command": "pacquet@main",
"mean": 1.2570203759200003,
"stddev": 0.028051184541660334,
"median": 1.2517672234200001,
"user": 1.3582051199999998,
"system": 1.68012984,
"min": 1.21721573092,
"max": 1.31000424292,
"times": [
1.2541556359200001,
1.21721573092,
1.23631608692,
1.2838191889200001,
1.26670631292,
1.23149049792,
1.31000424292,
1.27934753192,
1.2417697199200002,
1.2493788109200001
]
},
{
"command": "pnpr@HEAD",
"mean": 1.2610756373200003,
"stddev": 0.040007694206206,
"median": 1.2561735814200001,
"user": 1.36437222,
"system": 1.6759078399999996,
"min": 1.20520915092,
"max": 1.3621602319200001,
"times": [
1.20520915092,
1.2553020439200002,
1.2342720009200001,
1.2540925979200002,
1.24908414992,
1.27108365792,
1.3621602319200001,
1.25704511892,
1.25958279692,
1.26292462392
]
},
{
"command": "pnpr@main",
"mean": 1.24269330462,
"stddev": 0.029992503251653427,
"median": 1.23051597642,
"user": 1.33087922,
"system": 1.68549814,
"min": 1.21912952292,
"max": 1.31823483592,
"times": [
1.21974783892,
1.2484577219200002,
1.2525144029200002,
1.23345462992,
1.2578863909200002,
1.21912952292,
1.31823483592,
1.22757732292,
1.22650672192,
1.2234236579200002
]
}
]
} |
Contributor
|
Bencher Report| Branch | pr/11696 |
| Testbed | pacquet |
🚨 1 Alert
| Benchmark | Measure Units | View | Benchmark Result (Result Δ%) | Upper Boundary (Limit %) |
|---|---|---|---|---|
| isolated-linker.fresh-restore.cold-cache.cold-store | Latency seconds (s) | 📈 plot 🚷 threshold 🚨 alert (🔔) | 4.80 s(+91.73%)Baseline: 2.50 s | 3.00 s (159.77%) |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result milliseconds (ms) (Result Δ%) | Upper Boundary milliseconds (ms) (Limit %) |
|---|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,113.96 ms(-7.78%)Baseline: 2,292.30 ms | 2,750.76 ms (76.85%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,229.08 ms(-16.07%)Baseline: 1,464.41 ms | 1,757.29 ms (69.94%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 4,796.46 ms(+91.73%)Baseline: 2,501.70 ms | 3,002.04 ms (159.77%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 657.77 ms(+1.34%)Baseline: 649.10 ms | 778.92 ms (84.45%) |
…port-omitted-package-name # Conflicts: # pnpm-lock.yaml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
deps/inspection/commands/test/view.ts (1)
311-329: ⚡ Quick winConsider extracting the temp-dir/chdir scaffolding into a helper.
Tests at lines 237–329 repeat the same
mkdtempSync→chdir→finally { chdir(cwd); rmSync(...) }boilerplate. A small helper would reduce duplication and ensure consistent cleanup.♻️ Example helper
async function withTempCwd ( setup: (dir: string) => void, run: () => Promise<void> ): Promise<void> { const cwd = process.cwd() const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'view-test-')) try { setup(tmpDir) await run() } finally { process.chdir(cwd) fs.rmSync(tmpDir, { recursive: true, force: true }) } }🤖 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 `@deps/inspection/commands/test/view.ts` around lines 311 - 329, Extract the repeated mkdtempSync/chdir/finally cleanup scaffolding used in the tests (e.g., the "view: resolves package.json from opts.dir when cwd differs" test) into a reusable helper (suggested name withTempCwd) that accepts a setup callback and an async run callback; implement it to capture original cwd, create the temp dir, call setup(tmpDir) then await run(), and always restore cwd and remove temp dirs in finally. Replace the duplicated blocks in tests that currently create tmpDir/otherDir, call process.chdir(otherDir), write package.json, invoke view.handler({ ...VIEW_OPTIONS, dir: tmpDir } as unknown as Config & ConfigContext, []), and cleanup with calls to this helper (ensure the helper supports creating/cleanup of any additional dirs like otherDir or letting setup create them). Ensure the helper is exported/imported where needed and preserves existing assertions and error handling.
🤖 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.
Nitpick comments:
In `@deps/inspection/commands/test/view.ts`:
- Around line 311-329: Extract the repeated mkdtempSync/chdir/finally cleanup
scaffolding used in the tests (e.g., the "view: resolves package.json from
opts.dir when cwd differs" test) into a reusable helper (suggested name
withTempCwd) that accepts a setup callback and an async run callback; implement
it to capture original cwd, create the temp dir, call setup(tmpDir) then await
run(), and always restore cwd and remove temp dirs in finally. Replace the
duplicated blocks in tests that currently create tmpDir/otherDir, call
process.chdir(otherDir), write package.json, invoke view.handler({
...VIEW_OPTIONS, dir: tmpDir } as unknown as Config & ConfigContext, []), and
cleanup with calls to this helper (ensure the helper supports creating/cleanup
of any additional dirs like otherDir or letting setup create them). Ensure the
helper is exported/imported where needed and preserves existing assertions and
error handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ca3bff90-46f3-4370-a26e-a7843f728382
📒 Files selected for processing (3)
deps/inspection/commands/package.jsondeps/inspection/commands/src/view/index.tsdeps/inspection/commands/test/view.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- deps/inspection/commands/package.json
- deps/inspection/commands/src/view/index.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Analyze (javascript)
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Follow Standard Style with trailing commas, preferring functions over classes, and declaring functions after they are used (relying on hoisting)
Use a single options object instead of multiple parameters when a function needs more than two or three arguments
Follow Import Order: Standard libraries first, then external dependencies (alphabetically), then relative imports
Write self-documenting code where function names, parameters, and types explain what a function does without requiring prose comments
Do not write comments that restate what the code already says; refactor via renaming, splitting helpers, or restructuring instead
Do not repeat documentation at call sites that already exists in JSDoc on the callee; update JSDoc once for all call sites to benefit
Use JSDoc only for a function's contract (preconditions, postconditions, edge cases, why the function exists), not for re-narrating the body
Do not record past implementation shape, refactor history, or 'the previous code did X' framing in code; use git log and git blame instead
Write comments only when: the reason for code is non-obvious (hidden invariant, workaround for known bug, deliberate exception), or the right name doesn't fit (temporary technical constraint)
Files:
deps/inspection/commands/test/view.ts
🧠 Learnings (1)
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.
Applied to files:
deps/inspection/commands/test/view.ts
🔇 Additional comments (3)
deps/inspection/commands/test/view.ts (3)
8-8: LGTM!
43-56: LGTM!
293-309: LGTM!
zkochan
requested changes
Jun 4, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
zkochan
approved these changes
Jun 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It should behave in accordance with
npm view.replace
find-up, refer to https://e18e.dev/docs/replacements/find-upSummary by CodeRabbit
New Features
pnpm viewnow searches upward for the nearest package.json when no package name is provided and uses its name; it errors if that file lacks a name.Tests
Chores