feat(managers/npm): support pnpm catalogs #33376

fpapado · 2025-01-02T17:31:10Z

Changes

This PR adds support for pnpm catalogs, extracting dependencies from / updating dependencies in pnpm-workspace.yaml files.

I took @rarkins' suggestion in #30079 (comment), and added this as part of the npm manager. I tried to keep the functionality contained in pnpm files inside the relevant modules, to make it easier to split out in the future, if needed.

I took care of the following things:

pnpm-workspace.yaml files are picked up by default; their dependencies are extracted, and any updates are applied
Updates should preserve formatting as much as possible (tests back this up, but YAML is complicated)
pnpm-workspace.yaml files can be renamed and specified in fileMatch. The npm manager tries to parse a workspace YAML file first, before falling back to pakcage.json-like parsing.
The pnpmShrinkwrap file gets picked up, to use for updating. I believe I implemented that correctly, but I might have missed something.
Locked catalog dependencies are resolved from the lockfile. This should allow range replacements and other more advanced use-cases.

These questions have been resolved; I leave the text here for posterity

Open question 1: ways of picking up `pnpm-workspace.yaml` for extraction

I have placed pnpm-workspace.yaml in the fileMatch defaultConfig of the npm manager, and added an extra branch to extractAllPackageFiles.

An alternative would be to do this work in postExtract, by looking up sibling/parent pnpm-workspace.yaml file, and doing similar processing. This avoids the need for changing the config, but might have downsides that I cannot think of. What do you think?

Open question 2: `depType` and allowing granular updates to named catalogs

In the current version, I gave catalog-related upgrades a depType of pnpm.catalog, just for namespacing. However, I am wondering whether we could go more granular with this, such as pnpm.catalog.<catalogName>

One of the pnpm examples for catalogs, is allowing incremental updates, by defining named catalogs:

catalogs:
  # Can be referenced through "catalog:react17"
  react17:
    react: ^17.0.2
    react-dom: ^17.0.2

  # Can be referenced through "catalog:react18"
  react18:
    react: ^18.2.0
    react-dom: ^18.2.0

In such a scenario, I imagine users wanting to keep react 17 and 18 each in their range (allowing for bugfixes), without bumping the major. To do that, we would need to expose the catalog name to the user for matching, such as {matchDepTypes: ['pnpm.catalog.react17']}.

Have you had similar scenarios in the past? I noticed that pnpm.overrides kind of works like this, when using the pkg-a>pkg-b syntax (reference, example), but based on the depName. This could probably be done in a follow-up PR, but it's good to think about 💭

Context

Closes #30079

pnpm catalogs are described in the pnpm docs

This is my first Renovate contribution, I'm open to any and all ideas 😊

Documentation (please check one with an [x])

I have updated the documentation, or
[] No documentation update is required

I made a small documentation addition for the readme.md of the npm manager, in order to mention pnpm.catalogs.<name> as a depType.

How I've tested my work (please select one)

I have verified these changes via:

Code inspection only, or
Newly added/modified unit tests, or
No unit tests but ran on a real repository, or
Both unit tests + ran on a real repository

I added unit tests for the extraction and updates, the latter being a similar set to the package.json update tests. I will double-check the code coverage on CI, I think I missed something when running coverage locally.

I did a fair bit of repository testing at https://github.com/fpapado/renovate-testing-catalogs/pulls, to understand how the npm extraction/update phases work for package.json files.

lib/modules/manager/npm/update/dependency/pnpm.ts

lib/util/yaml.ts

fpapado · 2025-01-05T09:32:14Z

Ah, I noticed now in my test repo that the lockfile does not get updated after a standalone catalog dependency upgrade. I missed that codepath when trying to keep the functionality minimal. I'll add it to my todo 🗒️

Update: this has since been fixed 🎉

fpapado · 2025-01-09T08:13:32Z

@secustor and @viceice, apologies for the ping, do you have any pointers for the open questions here? If you think the PR is not in a reviewable state, I'm happy to implement with my gut feeling and re-request a review later down the line. I'm asking because I don't want to waste your time if something ends up being unworkable or unidiomatic, but I also don't want this to be stuck in limbo 😌

viceice · 2025-01-09T19:40:17Z

we usually don't review draft PRs, will try to have a look tomorrow

secustor

I had it on my plate, but haven't come around to finish it.

Open question 1: ways of picking up pnpm-workspace.yaml for extraction

pnpm-workspace.yaml should be part of fileMatch.
The expectation is that users will have use cases which deviating filenames e.g. because they are overwriting this name in a config.
Ideally we detect the fact that it is a workspace file via schema or other marker and are not relying on it. That way we are not bound to the filename and it "magically" works.

Open question 2: depType and allowing granular updates to named catalogs

Having pnpm.catalog.foo is preferable as with string pattern matching users can match all catalogs if needed.
Can you add an example of this in the readme.md that way it will show up in the manager docs.

lib/modules/manager/npm/extract/pnpm.ts

lib/modules/manager/npm/update/dependency/pnpm.spec.ts

lib/modules/manager/npm/update/dependency/pnpm.ts

lib/modules/manager/npm/update/dependency/common.ts

rarkins · 2025-01-10T12:38:30Z

@secuster can't we discover the pnpm-workspace.yaml file automatically instead of needing it in fileMatch?

secustor · 2025-01-10T13:10:54Z

@secuster can't we discover the pnpm-workspace.yaml file automatically instead of needing it in fileMatch?

If we are fine with only supporting this file name, we can drop it out of fileMatch.
There are requests regarding the possiblitly to change the file name and workarounds

rarkins · 2025-01-10T13:18:15Z

Ideally the name should be referenced from package.json. If not, I agree that we need to support it in fileMatch, but it's undesirable

lib/modules/manager/npm/extract/types.ts

lib/modules/manager/npm/update/dependency/pnpm.spec.ts

lib/modules/manager/npm/post-update/index.ts

fpapado · 2025-01-14T20:27:50Z

Thank you all kindly for your earler reviews! They helped clarify the direction quite a bit. I've made relevant changes now, tidied up the tests, implemented lockfile updating and locked version detection, and the coverage should be ok too.

I'll put this up for a review now. Let me know if you spot anything that can be improved, and I'll get to it 🔧

lib/modules/manager/npm/__fixtures__/lockfile-parsing/pnpm-lock-v9.yaml

lib/modules/manager/npm/extract/index.ts

lib/modules/manager/npm/extract/pnpm.spec.ts

lib/modules/manager/npm/extract/pnpm.ts

secustor

Sry, for the back and forth.

lib/modules/manager/npm/post-update/index.ts

lib/modules/manager/npm/extract/pnpm.spec.ts

secustor

LGTM, but there is a conflict with main

It turns out that including managerData in one of the extraction deps, would cause the top-level (packageFile) managerData to be overwritten for the corresponding upgrade. In practice, this meant that {managerData: {pnpmShrinkWrap: ''}} was not found, and no lockfile update was triggered.

Adding pnpm-workspace.yaml to the default matchFiles means that a few of the assertions were failing.

…ockfile This ensures that the lockfile update command will see the updated workspace file, and will update correctly. There was a hardcoded package.json check, that was previously precluding that.

Co-authored-by: Sebastian Poxhofer <[email protected]>

This required a few more changes in the rest of the file, in order to mock fs and use the utils consistently.

This introduces a tryParsePnpmWorkspaceYaml function, which parses a string as YAML, and validates it via a schema, returning the result, and whether parsing succeeded. This allows the npm manager extraction call-site to only parse the content once, and pass on the parsed contents (if any) for extraction.

Previously, this was being tested _incidentally_, due to not mocking the filesystem calls.

fpapado · 2025-01-28T09:05:25Z

LGTM, but there is a conflict with main

Oh right, there was a conflict in the npm README around depTypes. Rebased and resolved it now ✅

secustor

Please also no force pushes, your branch will automatically get rebased and squashed anyway

Thanks for your contribution!

renovate-release · 2025-01-28T10:48:13Z

🎉 This PR is included in version 39.138.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

fpapado commented Jan 2, 2025

View reviewed changes

lib/modules/manager/npm/update/dependency/pnpm.ts Outdated Show resolved Hide resolved

fpapado commented Jan 2, 2025

View reviewed changes

lib/util/yaml.ts Show resolved Hide resolved

fpapado marked this pull request as ready for review January 2, 2025 17:47

secustor self-requested a review January 2, 2025 21:18

viceice self-requested a review January 4, 2025 16:11

rarkins marked this pull request as draft January 8, 2025 12:27

secustor reviewed Jan 9, 2025

View reviewed changes

viceice reviewed Jan 11, 2025

View reviewed changes

lib/modules/manager/npm/extract/types.ts Outdated Show resolved Hide resolved

lib/modules/manager/npm/update/dependency/pnpm.spec.ts Outdated Show resolved Hide resolved

fpapado commented Jan 14, 2025

View reviewed changes

lib/modules/manager/npm/post-update/index.ts Show resolved Hide resolved

fpapado marked this pull request as ready for review January 14, 2025 20:27

fpapado requested review from secustor and viceice January 14, 2025 20:27

secustor reviewed Jan 17, 2025

View reviewed changes

fpapado requested a review from secustor January 19, 2025 18:12

secustor reviewed Jan 22, 2025

View reviewed changes

lib/modules/manager/npm/post-update/index.ts Show resolved Hide resolved

lib/modules/manager/npm/extract/pnpm.spec.ts Outdated Show resolved Hide resolved

fpapado mentioned this pull request Jan 22, 2025

test(npm): use fs test utils in extract/pnpm.spec.ts #33756

Merged

6 tasks

fpapado force-pushed the feat/30079-support-pnpm-catalogs branch from 33b479c to a0316a3 Compare January 22, 2025 17:32

fpapado requested a review from secustor January 23, 2025 08:44

secustor reviewed Jan 26, 2025

View reviewed changes

fpapado added 4 commits January 28, 2025 11:03

WIP: prototype extraction in its own manager

b22fd55

Move pnpm catalog extraction to npm manager

610add0

Remove pnpmCatalog from api.ts

c09e7bc

Implement updatePnpmCatalogDependency in updateDependency

a3a5250

fpapado and others added 20 commits January 28, 2025 11:03

docs: add mention of pnpm.catalog depName in npm readme.md

6c4ee60

fix: ensure that pnpm-workspace.yaml duck-typing uses full schema

bfdae64

test: fix pnpm tests, by awaiting and by changing the object shape

9b7807d

test: fix tests that do assertions on the shape of npm manager config

8b596f3

Adding pnpm-workspace.yaml to the default matchFiles means that a few of the assertions were failing.

test: address coverage in lib/modules/manager/npm/update

1b349e9

chore: remove done TODO

5b90887

test: address coverage in lib/modules/manager/npm/extract/

77678e1

feat: extract locked versions from catalogs

668c511

chore: remove done TODO

6cfd893

test: tidy up coverage

b886aa6

fix: ensure that pnpm-workspace.yaml artifacts get written prior to l…

1555235

…ockfile This ensures that the lockfile update command will see the updated workspace file, and will update correctly. There was a hardcoded package.json check, that was previously precluding that.

test: add explicit tests for comments in pnpm workspace updates

ea535e9

chore: remove errant logging

d6820f7

Apply suggestions from code review

bfcc4e9

Co-authored-by: Sebastian Poxhofer <[email protected]>

test: inline npm/__fixtures__/lockfile-parsing/pnpm-lock-v9.yaml

c8062e0

test: use fs from test utils in npm/extract/pnpm.spec.ts

c3e69f2

This required a few more changes in the rest of the file, in order to mock fs and use the utils consistently.

chore: add explicit type annotation inside pnpmCatalogsToArray

35c8707

test: explicitly test lockfile acquisition in pnpm-workspace extraction

a3fb8df

Previously, this was being tested _incidentally_, due to not mocking the filesystem calls.

fpapado force-pushed the feat/30079-support-pnpm-catalogs branch from a0316a3 to a3fb8df Compare January 28, 2025 09:04

fpapado requested a review from secustor January 28, 2025 09:05

secustor approved these changes Jan 28, 2025

View reviewed changes

secustor changed the title ~~feat(npm): support pnpm catalogs~~ feat(managers/npm): support pnpm catalogs Jan 28, 2025

secustor added this pull request to the merge queue Jan 28, 2025

Merged via the queue into renovatebot:main with commit 0f06866 Jan 28, 2025
39 checks passed

fpapado mentioned this pull request Jan 28, 2025

Catalogs Tracking Issue pnpm/pnpm#7072

Open

18 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(managers/npm): support pnpm catalogs #33376

feat(managers/npm): support pnpm catalogs #33376

fpapado commented Jan 2, 2025 •

edited

Loading

fpapado commented Jan 5, 2025 •

edited

Loading

fpapado commented Jan 9, 2025 •

edited

Loading

viceice commented Jan 9, 2025

secustor left a comment •

edited

Loading

rarkins commented Jan 10, 2025

secustor commented Jan 10, 2025

rarkins commented Jan 10, 2025

fpapado commented Jan 14, 2025

secustor left a comment

secustor left a comment

fpapado commented Jan 28, 2025

secustor left a comment

renovate-release commented Jan 28, 2025

feat(managers/npm): support pnpm catalogs #33376

feat(managers/npm): support pnpm catalogs #33376

Conversation

fpapado commented Jan 2, 2025 • edited Loading

Changes

Open question 1: ways of picking up pnpm-workspace.yaml for extraction

Open question 2: depType and allowing granular updates to named catalogs

Context

Documentation (please check one with an [x])

How I've tested my work (please select one)

fpapado commented Jan 5, 2025 • edited Loading

fpapado commented Jan 9, 2025 • edited Loading

viceice commented Jan 9, 2025

secustor left a comment • edited Loading

Choose a reason for hiding this comment

rarkins commented Jan 10, 2025

secustor commented Jan 10, 2025

rarkins commented Jan 10, 2025

fpapado commented Jan 14, 2025

secustor left a comment

Choose a reason for hiding this comment

secustor left a comment

Choose a reason for hiding this comment

fpapado commented Jan 28, 2025

secustor left a comment

Choose a reason for hiding this comment

renovate-release commented Jan 28, 2025

fpapado commented Jan 2, 2025 •

edited

Loading

Open question 1: ways of picking up `pnpm-workspace.yaml` for extraction

Open question 2: `depType` and allowing granular updates to named catalogs

fpapado commented Jan 5, 2025 •

edited

Loading

fpapado commented Jan 9, 2025 •

edited

Loading

secustor left a comment •

edited

Loading