fix(pnpm): write pnpmOverrides to pnpm-workspace.yaml - #336
Closed
unstubbable wants to merge 1 commit into
Closed
unstubbable wants to merge 1 commit into
unstubbable wants to merge 1 commit into
Conversation
The `fix` command reads `overrides` from `pnpm-workspace.yaml` but never writes the corrected versions back, so any mismatch that `lint` flagged was reported as fixed (`✓`) while the file was actually left untouched. The read side was added in 72ce5ef, which pointed the default `pnpmOverrides` dependency type at `pnpm-workspace.yaml` with `source: PnpmWorkspace`. The matching write path in `copy_expected_specifier_yaml` was never extended, though: it returns early for any instance without a catalog name, so the `versionsByName` workspace instances (i.e. `overrides`) fall through and nothing is written. An `overrides` block is a flat `name: version` map with the same shape as the default `catalog:` block, so the single-block logic now lives in reusable `build_block_insert_patch` and `ensure_yaml_block` helpers, extracted from `build_insert_patch` and `ensure_catalog_block` (which the default catalog delegates to). Non-catalog `versionsByName` workspace instances route through a new `insert_into_yaml_block`, the single-block sibling of `insert_catalog_definition`. The block key comes from the dependency type's `path` (e.g. `/overrides`), so the same path also covers any user `customType` with `source: PnpmWorkspace`. Reads and lints already detected these mismatches, so apart from the now-working write path the behaviour is unchanged. A new regression test, `pnpm_fix_updates_overrides_in_yaml`, asserts the override is rewritten in `pnpm-workspace.yaml` and the file is marked dirty.
Owner
|
Damn, nice catch. I checked that catalogs were written back to this file, but I must've totally overlooked overrides, thanks a lot. |
Owner
|
Released in 15.3.2, thanks a lot. |
greydragon888
added a commit
to greydragon888/real-router
that referenced
this pull request
Sep 12, 2026
15.3.2 carries JamieMason/syncpack#336: `syncpack fix` read `overrides` from pnpm-workspace.yaml but never wrote the corrected versions back, so a mismatch lint had flagged was reported as fixed while the file stayed untouched. 15.3.3 adds only cargo dependency updates on top. ⚠ The PR page says CLOSED, which is misleading: the maintainer squashed it, so GitHub never linked it as merged. 15.3.2's release notes carry the fix by name — "**pnpm:** write pnpmOverrides to pnpm-workspace.yaml (ef7639e)" — and his comment on the PR says "Released in 15.3.2". Measured, the bug was unreachable here TWICE over, which is why this is hygiene rather than a fix: 1. Our "Ignore pnpm overrides" group suppresses the whole pnpmOverrides dependency type. With a real mismatch planted (@types/node 24.13.2 in overrides against the pinned 24.13.3) syncpack reports "✓ No issues found"; delete the group and the same tree reports SameRangeMismatch naming both declarations. So `fix` never has an override correction to write. 2. Even without the group, that shape is not writable: `syncpack fix` answers "Some issues remain which cannot be fixed automatically". A two-sided SameRangeMismatch has no expected version to write, so the repaired code path is not the one our config would reach anyway. Still worth taking: the failure shape is a tool printing success while changing nothing, and 53 override entries sit behind that one config line, several of them security floors. Validation is the finding set, not the exit code: `syncpack lint` green before and after, and `syncpack list` identical at 138 lines. The eight native binaries pin to 15.3.3 exactly and the tool reports 15.3.3 — no wrapper/binary drift. lint:deps, lint:dedupe and lint:audit "No issues found" are green. Claude-Session: https://claude.ai/code/session_01Ws4eQ4YAetztqb4HPg86Sj
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
2 participants
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.
The
fixcommand readsoverridesfrompnpm-workspace.yamlbut never writes the corrected versions back, so any mismatch thatlintflagged was reported as fixed (✓) while the file was actually left untouched.The read side was added in 72ce5ef, which pointed the default
pnpmOverridesdependency type atpnpm-workspace.yamlwithsource: PnpmWorkspace. The matching write path incopy_expected_specifier_yamlwas never extended, though: it returns early for any instance without a catalog name, so theversionsByNameworkspace instances (i.e.overrides) fall through and nothing is written.An
overridesblock is a flatname: versionmap with the same shape as the defaultcatalog:block, so the single-block logic now lives in reusablebuild_block_insert_patchandensure_yaml_blockhelpers, extracted frombuild_insert_patchandensure_catalog_block(which the default catalog delegates to). Non-catalogversionsByNameworkspace instances route through a newinsert_into_yaml_block, the single-block sibling ofinsert_catalog_definition. The block key comes from the dependency type'spath(e.g./overrides), so the same path also covers any usercustomTypewithsource: PnpmWorkspace.Reads and lints already detected these mismatches, so apart from the now-working write path the behaviour is unchanged. A new regression test,
pnpm_fix_updates_overrides_in_yaml, asserts the override is rewritten inpnpm-workspace.yamland the file is marked dirty.Tip
Best reviewed with hidden whitespace changes because of the shared helper extraction.