feat(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.

There's a test failing that makes assertions about the settings, but let me tell you about a couple of open questions before I go down changing those tests 😌

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 believe no documentation update is required, but I'm more than happy to add anything you deem necessary ✍️)

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.

This keeps YAML parsing centralised, while allowing use of the Document represenation, in addition to the JS one.

fpapado · 2025-01-02T17:43:42Z

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

+  let document;
+  let parsedContents;
+
+  try {
+    document = parseSingleYamlDocument(fileContent);
+    parsedContents = pnpmCatalogsSchema.parse(document.toJS());


I initially opted to do the YAML replacements with a document representation, instead of string replacements, because it seemed simpler as a start. I particularly did not want to drill down into the catalogs, accounting for the different YAML Map representations, string quoting etc 💫

Now that the tests are relatively set, I could change the implementation to do string replacement, if you think that is best.

Does your solution keep whitespaces and comments?

If yes, you can keep it like it is.

fpapado · 2025-01-02T17:45:03Z

lib/util/yaml.ts

+export function parseSingleYamlDocument(
+  content: string,
+  options?: YamlParseDocumentOptions,
+): Document {


I added this just to keep the parsing centralised, even if there are no other use-cases for YAML Document transforms at the moment.

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 🗒️

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.

secustor · 2025-01-09T21:04:17Z