Skip to content

Comments

Add 'rush-pnpm up' support for catalogs#5585

Open
benkeen wants to merge 2 commits intomicrosoft:mainfrom
benkeen:main
Open

Add 'rush-pnpm up' support for catalogs#5585
benkeen wants to merge 2 commits intomicrosoft:mainfrom
benkeen:main

Conversation

@benkeen
Copy link
Contributor

@benkeen benkeen commented Jan 31, 2026

Addresses: #5578

Right now, when you update packages in a Rush/pnpm repo using rush-pnpm up, it updates the catalog entries in common/temp/pnpm-workspace.yaml, but doesn't update Rush's actual catalog, stored in pnpm-config.json, so the update command doesn't fully work. This has been supported since pnpm@10.12.0. This PR enhances it to update the pnpm-config file too.

I also updated it to fix a small issue where JsonFile.save could throw an error due to an undefined $schema property.

Besides the tests, I've ran manual checks with our own Rush repo to confirm the catalog is getting updated properly.

@benkeen
Copy link
Contributor Author

benkeen commented Feb 5, 2026

Howdy @iclanton, @dmichon-msft - don't suppose either of you have context or time to look at this?

@iclanton iclanton moved this from Needs triage to In Progress in Bug Triage Feb 9, 2026
@iclanton
Copy link
Member

iclanton commented Feb 9, 2026

Shouldn't users be running rush update instead of rush-pnpm update?

@benkeen
Copy link
Contributor Author

benkeen commented Feb 9, 2026

Huh, neat. I never associated those two commands in my mind.

rush update has a very limited, custom interface for Rush itself. What I'm trying to do is offer up all the extra options with pnpm update, for updating subsets of dependencies, like:

pnpm update -r "@babel/*"

That could always be done with rush-pnpm (but not rush update); this PR just lets that command handle catalogs too.

Tell me if I'm being barmy.

@benkeen
Copy link
Contributor Author

benkeen commented Feb 18, 2026

Hey @iclanton - sorry to bug, got some time to look at this, this week? [We'd love this feature for our repo!]

@benkeen
Copy link
Contributor Author

benkeen commented Feb 24, 2026

Hi @octogonz, @dmichon-msft would either of you be free for a review?

Copy link
Member

@iclanton iclanton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This overall looks fine, but would it be better to just include whatever functionality is missing from rush update?

* @param workspaceYamlFilename - The path to the pnpm-workspace.yaml file
* @returns The catalogs object, or undefined if the file doesn't exist or has no catalogs
*/
public static loadCatalogsFromFile(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you do this async?

this._json.globalOnlyBuiltDependencies = onlyBuiltDependencies;
}
if (this.jsonFilename) {
JsonFile.save(this._json, this.jsonFilename, { updateExistingFile: true });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do this async?

Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com>
@benkeen
Copy link
Contributor Author

benkeen commented Feb 24, 2026

This overall looks fine, but would it be better to just include whatever functionality is missing from rush update?

I see what you're saying. I'm on the fence, but I think I'd lean towards keeping them separate. Feels like we'd be overloading rush update.

Couple of thoughts:

  • if we did this, we'd need the rush command to work with all supported package managers, and the features and nuances of how each of those update/upgrade commands work (I haven't looked into all the available options for each). We'd only want to enhance the rush update command if it supported them all.
  • API-wise, we'd need to separate the "raw" pnpm update command args from the existing rush update command args so there's no conflicts when running rush update, of course. The pnpm cmd allows various arguments we'd need to offer in order to make the command useful, which could be a challenge. They seem low-level enough that I immediately went to updating the rush-pnpm wrapper.

I dunno. I'd say enhance the rush-pnpm command first, then depending on how it goes and what sort of demand is shown, consider elevating the functionality to the full rush update later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants