fix(pnpm): write pnpmOverrides to pnpm-workspace.yaml#336
Open
unstubbable wants to merge 1 commit into
Open
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. |
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.
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.