Skip to content

Read optionalDependencies from pnpm lockfiles - #1766

Draft
spatten wants to merge 1 commit into
masterfrom
pnpm-optional-dependencies
Draft

Read optionalDependencies from pnpm lockfiles#1766
spatten wants to merge 1 commit into
masterfrom
pnpm-optional-dependencies

Conversation

@spatten

@spatten spatten commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Overview

The pnpm lockfile parser never read optionalDependencies, at any level. An importer's optional dependencies were not direct, and a package's optional dependencies produced no edges. The packages still appeared in the result, because every entry in the lockfile's packages section is added to the graph whether or not anything points at it, so the only visible symptom was shape: sharp declared by the project showed up as a transitive dependency, and fsevents had no edge from chokidar. The pnpm docs already say optional dependencies are included; this makes the graph match that.

The parser now reads optionalDependencies in importers as direct production dependencies, and in packages entries (v5 to v8) and snapshots (v9) as edges. buildGraphCore treats an importer's optional dependencies exactly like its dependencies.

This is a prerequisite for #1763, which scopes a pnpm result to selected workspace members by pruning what the selection cannot reach. Without these edges, pruning would silently drop every optional package and anything only reachable through one. Unscoped analysis is unaffected by that pruning, which is why the gap was harmless until now.

Acceptance criteria

For a pnpm project whose package.json lists sharp under optionalDependencies and depends on chokidar, fossa analyze reports sharp as a direct dependency and fsevents as a dependency of chokidar, in both v6 and v9 lockfiles. Nothing that was reported before disappears.

Testing plan

  1. cabal test unit-tests --test-options=--match=Pnpm runs the two new fixtures, one v6 and one v9, alongside every existing pnpm fixture, none of which declare optional dependencies, so their expectations are unchanged.
  2. On a real project, compare fossa analyze --output before and after: the vertex set is the same, sharp moves from transitive to direct, and chokidar gains an edge to fsevents.

Risks

Optional dependencies are labeled production. The npm v3 lockfile strategy already reads a package's optionalDependencies as ordinary edges, so this matches it, but the label is a choice.

Metrics

None.

References

  • Found while reviewing #1763, which should be rebased onto this once it merges so that its link following also reads optional dependencies.

Checklist

  • I added tests for this PR's change (or explained in the PR description why tests don't make sense).
  • If this PR introduced a user-visible change, I added documentation into docs/.
  • If this PR added docs, I added links as appropriate to the user manual's ToC in docs/README.ms and gave consideration to how discoverable or not my documentation is.
  • If this change is externally visible, I updated Changelog.md. If this PR did not mark a release, I added my changes into an ## Unreleased section at the top.
  • If I made changes to .fossa.yml or fossa-deps.{json.yml}, I updated docs/references/files/*.schema.json AND I have updated example files used by fossa init command. You may also need to update these if you have added/removed new dependency type (e.g. pip) or analysis target type (e.g. poetry).
  • If I made changes to a subcommand's options, I updated docs/references/subcommands/<subcommand>.md.

🤖 Generated with Claude Code

https://claude.ai/code/session_0122XfZKg7Yidpc1HEEvBSJD

The parser never read optionalDependencies at any level, so an
importer's optional dependencies were not direct and a package's
optional dependencies produced no edges. The packages still appeared,
because every `packages` entry becomes a graph node whether or not
anything points at it, so the only symptom was shape: a project's own
`sharp` reported as transitive, `fsevents` with no edge from chokidar.

Importers' optionalDependencies are now direct production dependencies,
and packages' (v5 to v8) and snapshots' (v9) optionalDependencies are
edges like any other. This is a prerequisite for #1763, whose scoped
results prune whatever the selection cannot reach.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0122XfZKg7Yidpc1HEEvBSJD
@spatten

spatten commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 1dd71ec5-311a-415a-8553-fc230082bcba

📥 Commits

Reviewing files that changed from the base of the PR and between 00580ae and 8ec1357.

⛔ Files ignored due to path filters (1)
  • test/Pnpm/testdata/pnpm-9-optional-deps/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • Changelog.md
  • src/Strategy/Node/Pnpm/PnpmLock.hs
  • src/Strategy/Node/Pnpm/Types.hs
  • test/Pnpm/PnpmLockSpec.hs
  • test/Pnpm/testdata/pnpm-lock-v6-optional.yaml

Included review availability: 2 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.


Walkthrough

Pnpm lockfile parsing now includes optional dependencies from snapshots, project maps, package data, and non-workspace roots. Project-level optional dependencies are stored as direct dependencies and processed as production dependencies. Package-level optional dependencies are merged into package dependency edges. New pnpm v6 and v9 fixtures verify direct chokidar and sharp dependencies and the chokidar edges to readdirp and fsevents.

Merge Risk: ⚪ Minimal · up to 8ec13

The PR extends pnpm graph parsing to include optional dependencies, with no actionable merge-blocking risk identified; it is merge-ready after normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: adding support for reading optionalDependencies from pnpm lockfiles.
Description check ✅ Passed The description includes all required sections, explains the implementation and user impact, provides concrete testing steps, identifies risks, and documents references and checklist status.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (5 skipped: 5 unsupported.)


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

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