-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC: Catalogs β Shareable dependency version specifiers #1
Conversation
"@types/react": "^17.0.43", | ||
"react": "^17.0.2", | ||
"react-dom": "^17.0.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For publishing, I think it would be most useful if the definition of the version group defined both the exact version that is to be installed in the workspace, and separately the specifier that is to be written during publishing. This will allow developers to loosen the version for their customers while still controlling exactly what gets installed in the workspace (which is increasingly necessary in the current security landscape).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open to this, but want to ask a few more questions.
-
Does specifying both a range specifier for internal upgrades and publishing run into the scenario described here? Namely:
- If you have an internal upgrade range of
^17.0.0
, and a resolved internal lockfile version of17.0.1
what should a publish range of*
result in?17.0.0
or17.0.1
? - In a similar scenario but a publish range of
^
, should the dependency publish with^17.0.0
or^17.0.1
? I think the answer to this scenario is more clearly^17.0.0
, but it results in confusing behavior viewed next to the ambiguous behavior for*
.
- If you have an internal upgrade range of
-
I might be missing something, but I think this is the existing situation for how dependencies are declared and resolved at the moment too? Is there something special about workspace consistent versions that means this feature should do something additional?
- To elaborate, if someone wants to keep the version range for customers loose while controlling exactly what gets installed into the workspace, they have to modify the
pnpm-lock.yaml
file (either manually or through a command).
- To elaborate, if someone wants to keep the version range for customers loose while controlling exactly what gets installed into the workspace, they have to modify the
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused by your linked example, in that I'd expect the only data in package.json to be "react": "workspace-consistent:react-rc"
. The version range is specified in the definition of "react-rc"
. I'd like to be able to do something like:
"react-rc": {
"react": {
"publish": "^17",
"install": "17.0.2"
}
}
"dependencies": {
// Resolves to "17.0.2" at install time, when published is "^17"
"react": "workspace-consistent:react-rc"
}
The value in "publish" is a superset of the value in "install". Technically you could do the same thing by declaring two dependency sets "react-rc-dev" and "react-rc-publish" and having the former as devDependencies
and the latter as regular dependencies
, since devDependencies
supersede regular dependencies
in local packages, but that's rather cumbersome compared to declaring it inside the definition of the version group.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the late response. I do appreciate your input.
Are there downsides to keeping the resolved version in pnpm-lock.yaml
? Adapting your example:
"react-rc": {
"react": "^17"
}
# pnpm-lock.yaml
workspaceConsistent:
react-rc:
react:
specifier: ^17
dependency: 17.0.2
Although it may not be intuitive, it's possible to edit the dependency: 17.0.2
line in pnpm-lock.yaml
instead of editing an "install": "17.0.2"
field in package.json
. I think this accomplishes the same thing?
The benefits of keeping resolved versions in pnpm-lock.yaml
are:
- Follows the current convention of keeping concrete/resolved versions in
pnpm-lock.yaml
. pnpm update
doesn't have to edit the rootpackage.json
, which I think can become strange. Admittedly I haven't tested this command before, but from the description in the docs I'd imagine it only editspnpm-lock.yaml
at the moment.- Users don't have to manually look up or decide concrete versions on npmjs.com when adding a new dependency.
I do see the benefit of having resolved versions in package.json
being:
- It's apparent to users that you can edit the resolved version in
package.json
. (Editingpnpm-lock.yaml
is probably discouraged on most teams.)
Is everything above correct? Did I misunderstand the pros/cons?
Without the colon, this conflicts with the existing npm dist tag namespace. pnpm#1 (comment)
The original @types/react example wasn't great. pnpm#1 (comment)
|
||
As monorepos grow in workspace package count, merge conflicts become increasingly more probable. On GitHub, any merge conflict in `pnpm-lock.yaml` blocks pull requests due to lack of custom merge driver support. | ||
|
||
With first-class support for workspace consistent versions, edits on upgrades are limited to the root `package.json` and the `workspaceConsistent` portion of `pnpm-lock.yaml`. This reduces churn in `pnpm-lock.yaml`, and **prevents merge conflicts in all the cases listed above**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get it, how does it reduce the nubmer of conflicts? The same conflicts will happen just in another place, in the workspaceConsistent
section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spacing in the workspaceConsistent
section helps a lot. I took some time tonight to commit the example merges in this RFC to a GitHub repo. I added a theoretical pull request for each scenario in an imaginary world using the workspace-consistent:
protocol.
For the PRs in this repo:
- The
main
branch has just upgraded React. - The
main-workspace-consistent
branch has also just upgraded React, but is using theworkspace-consistent:
protocol.
Note that all PRs using workspace-consistent:
show as ready to merge in GitHub, while the commits not using the protocol have merge conflicts in pnpm-lock.yaml
.
Example | Not Using the New Protocol | Using the New Protocol |
---|---|---|
Changes the version of a dependency line-adjacent to the upgraded dependency. | ||
Add or removes a dependency line-adjacent to the upgraded dependency. | ||
Adds a new declaration of the upgraded dependency. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like simply adding new lines also solves the issue: gluxon/pnpm-workspace-consistent-examples#7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for testing that alternative and adding an example. Mentioned in a related thread #1 (comment), but that does solve 1 and 2.
I am still interested in this RFC since:
- It addresses the 3rd "Adds a new declaration of the upgraded dependency." case that the spacing doesn't.
- Allows
package.json
files to be more merge conflict resistant too without adding newlines to thedependencies
section. I could see slow adoption for adding newlines todependencies
as a best practice in repos with consistent versions. Whereas if repos use this feature, they get the merge conflict resistance for free. - Having
workspace-consistent:
inpackage.json
still better reflects intent.- Namely, the "How do I add a new dependency?" is common question amongst new developers on my team. The current answer is you go check if another
package.json
has it listed and copy the version range if it does exists. That feels a bit strange. Havingpnpm add <pkg>
simply added"<pkg>": "workspace-consistent:"
if<pkg>
is configured feels much better.
- Namely, the "How do I add a new dependency?" is common question amongst new developers on my team. The current answer is you go check if another
react-dom: 17.0.2 | ||
``` | ||
|
||
The alternative format better matches the existing shape of `importers`. However the proposed format is more resilient to merge conflicts. Separate commits changing the version of `react` and `jest` would merge conflict with the alternative format, while the proposed format would merge those changes cleanly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, so then wouldn't it be enough to change the format of the importers? Merge conflicts would be fixed without adding the complex workspaceConstraints logic to the lockfile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the scenarios mentioned:
- Changes the version of a dependency line-adjacent to the upgraded dependency.
- Add or removes a dependency line-adjacent to the upgraded dependency.
- Adds a new declaration of the upgraded dependency.
You're right. I think a new format for importers would address 1 and 2. 3 isn't handled, but that may be good enough. Will think about this a bit more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the merge conflicts in pnpm-lock.yaml are less of an issue (the current format helps a lot) than merge conflicts in package.json
when upgrading dependencies across the repository. A common situation that I run into is one developer adding a new project that uses the common version of a dependency colliding with a PR that upgrades said dependency. This actually manifests not as a git merge conflict but as a break in main due to an invalid lockfile and/or breaking the checker (since my team uses Rush we have version consistency checking during install, which fails in this case).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gluxon I think we can implement this for importers and solve a big chunk of the issues. It is not hard to implement, so we can do it as a breaking change to the lockfile format in pnpm v7.
I think workspace constraints are a bigger change. I don't know yet if it is a good idea to put so much additional logic to the lockfile. In any case, this will require a lot of time for review.
Maybe there could be alternative solutions to solve the issues where the version is influenced by peer deps.
Like instead of this:
importers:
project:
dependencies:
react:
specifier: ^1.0.0
version: [email protected]
react-dom:
specifier: ^1.0.0
version: 1.0.0
we could do this
importers:
project:
dependencies:
react:
specifier: ^1.0.0
version: ['1.0.0', 'react-dom']
react-dom:
specifier: ^1.0.0
version: 1.0.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The situation @dmichon-msft described is something I see happen about once a week on my team. It's a flavor of the "3. Adds a new declaration of the upgraded dependency." conflict type (mentioned above) that the changes to the importers
section described in the above comment would not address. At that frequency, it's not the most pressing problem. I tend to get an alert that our CI is broken, run pnpm install
, and force push any updated package.json
and pnpm-lock.yaml
files to develop
.
Tangent on bors
Last year I was trying to convince my team to adopt bors as another way to prevent any kind of develop
branch breaks, but I think we'll just switch to the GitHub merge queue when it's available on GitHub Enterprise instead of asking everyone to learn how bors works only to switch away from it later.
I think we can implement this for importers and solve a big chunk of the issues.
I think you're right. This solves 1 and 2 (#1 (comment)), which is indeed a big chunk of the problem.
Another tangent on pnpm-lock.yaml and newlines:
In January I ended up writing a space-lockfile
command that is set to run on postinstall
. It simply goes through pnpm-lock.yaml
and adds any newlines before/after lines that contain an element in a list of package names. (The package names in this list are the consistent versions in the repo.) This made pnpm-lock.yaml
merge conflict much less and has worked well since it was introduced 2 months ago, but the readability of the file was sacrificed a bit. I'm not completely satisfied with my space-lockfile
solution either. If pnpm had a hook for serializing/deserializing lockfiles, I would have implemented a minimal version of this RFC without version groups with it instead.
It is not hard to implement, so we can do it as a breaking change to the lockfile format in pnpm v7.
I appreciate you being open to this. I am a bit worried making the importers
sections 25% newlines might cause other unexpected problems though, and wouldn't want to risk pnpm v7's release since it seems like we're in the beta phase already. Is it possible for these formatting changes to have edge cases where commits that merged cleanly before no longer do with the new format? Even if that is possible, perhaps that's a non-concern if it decreases the probability of conflicts as a whole.
I think workspace constraints are a bigger change. I don't know yet if it is a good idea to put so much additional logic to the lockfile. In any case, this will require a lot of time for review.
I was imagining the workspace-consistent:
protocol changes to primarily happen during pnpm-lock.yaml
serialization and deserialization. Everything else in pnpm would see the same PnpmLockfile
type as it does now. (With a workspaceConsistent
key that can be ignored.) Does that reduce the scope/risk of this change? Or is it strange to implement in this manner? There might need to be some tweaking to the peersSuffix
format when there's a hash involved for this to happen during (de)serialization, but it feels doable.
Maybe there could be alternative solutions to solve the issues where the version is influenced by peer deps.
I like the direction this idea is heading. I think the peersSuffix
portion (especially when it's a hash) is the only non-intuitive part of the lockfile. I was only able to understand the concept because you explained it well here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can implement this for importers and solve a big chunk of the issues. It is not hard to implement, so we can do it as a breaking change to the lockfile format in pnpm v7.
@zkochan I'd actually like to experiment with this a bit. We proposed this format above:
importers:
project:
dependencies:
react:
specifier: ^1.0.0
version: [email protected]
react-dom:
specifier: ^1.0.0
version: 1.0.0
But I think we can just do:
importers:
project:
dependencies:
react:
specifier: ^1.0.0
version: [email protected]
react-dom:
specifier: ^1.0.0
version: 1.0.0
I actually don't think newlines are strictly required since react:
and react-dom:
are on their own lines and not changing as much as specifier
or version
. Git can use those lines to determine where diff regions apply rather than the newlines.
@zkochan When I start experimenting, would it be preferable to localize this change to the lockfile serialization/deserialization, or update the PnpmLockfile
shape itself? I lean towards the later despite the wider surface area it'll affect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, I would localize the change to serialization/deserialization. Because we will probably hide the new format under a feature flag. Of course, when we'll be sure about the format, we will make it the default and refactor PnpmLockfile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for discussing! Filing an issue on myself to track: pnpm/pnpm#4725
@zkochan I know you still had reservations back when we discussed this last year (understandably). Have the feelings leaned one way or the other since then? Despite the new (currently feature flagged) lockfile format, I'm still very interested in this for the reasons in #1 (comment). I think this would be a very popular pnpm feature if it lands. Unless you were heavily opposed, I was going to start a prototype pull request for resolving |
See my suggestions for the new lockfile format in pnpm/pnpm#5810 I am not against a workspace-consistent feature but adding it to the lockfile I don't believe is a good idea.
Maybe with the new format, we could do a simpler syntax in the lockfile. Something like this: /@typescript-eslint/[email protected](eslint)(typescript):
resolution: {integrity: sha512-x2jrMcPaMfsHRRIkL+x96++xdzvrdBCnYRd5QiW5Wgo1OB4kDYPbC1XjWP/TNqlfK93K/lUL92erq5zPLgFScQ==}
engines: {node: ^12.22.0 || ^14.17.0 || >=16.0.0}
peerDependencies:
eslint: '*'
dependencies:
'@types/json-schema': 7.0.11
'@types/semver': 7.3.13
'@typescript-eslint/scope-manager': 5.48.0
'@typescript-eslint/types': 5.48.0
'@typescript-eslint/typescript-estree': 5.48.0(typescript)
eslint: 8.31.0
eslint-scope: 5.1.1
eslint-utils: 3.0.0(eslint)
semver: 7.3.8
transitivePeerDependencies:
- supports-color
- typescript
dev: false |
Thanks! I can get a PR together that uses Future looking, do you think it's possible to make improvements to the lockfile parsing logic that simplify it? I'm curious if you think the logic itself is complex, or if it's the existing implementation. I'm happy to help work on the lockfile parsing parts of pnpm for a bit if we think implementation can be simplified. |
workspace-consistent does not solve all the challenges that we have with sharing dependencies between projects. See the suggestion I have with environments: pnpm/pnpm#2713 (comment) In theory, such environments could also be used to "compress" the lockfile. But we can also think out of the box and redesign the lockfile completely to minimise potential conflicts. |
I actually think the environments idea that came up earlier this week is very similar to the proposal here. The concepts translate very closely:
Sharing and consuming environmentsOne important difference is that the environment (e.g. I could certainly see this being used on my team, which has multiple monorepos we work on. Defining consistent version usageFrom the other thread, I saw that the configuration and properties of the environments proposal was still in discussion. pnpm/pnpm#2713 (comment)
If I may offer my opinion, I think a new protocol would offer more improvements than new Instead of: {
"name": "foo",
"version": "1.0.0",
"pnpm": {
"environment": "@comp/[email protected]",
"dependenciesFromEnv": ["react"],
"devDependenciesFromEnv": ["eslint"],
"peerDependenciesFromEnv": ["react-dom"]
}
} I think we should strongly consider: {
"name": "foo",
"version": "1.0.0",
"pnpm": {
"environment": "@comp/[email protected]"
},
"dependencies": {
"react": "pnpm-env:"
},
"devDependencies": {
"eslint": "pnpm-env:"
},
"peerDependencies": {
"react-dom": "pnpm-env:"
}
} (Edit: The first version of the block above kept the The reasons are:
How would dependencies from environments be represented in the lockfile? Would be interested in iterating with you on that if you're up for it.
Sweet. I'm glad to hear. β€οΈ |
Sounds good, but in your example you have not removed the {
"name": "foo",
"version": "1.0.0",
"pnpm": {
"environment": "@comp/[email protected]"
},
"dependencies": {
"react": "pnpm-env:"
},
"devDependencies": {
"eslint": "pnpm-env:"
},
"peerDependencies:" {
"react-dom": "pnpm-env:"
}
} I think your points are valid. We can use this syntax instead (not sure how the protocol will be named yet). The envs feature probably requires its own RFC and we can discuss there if we need changes in the lockfile for envs. |
One more problem that I want to solve with envs is proxy packages that people use just to get their subdeps hoisted to the root of node_modules. But the issue with these is that they will want all deps from the env be installed. They don't want to list the deps. I guess we could have an option to autoinstall all dev deps from the env as dev deps. |
Thanks for considering the points. Appreciate it. For the name, I saw a suggestion for
Would you be open to me drafting the initial RFC this week? I think it's a very close evolution of the RFC here and want to help build it.
Interesting. Is the proxy package technique elaborated on somewhere? If someone needs those deps hoisted to the root |
I think we can use environments not only for versions. For instance, we could share scripts with it. We could share patch files to patch deps. We could also share the engines, license and author fields with it. We can rename the
ok
They don't want to add any dependencies to any I guess we could have some field in the env's package.json to list dependencies that should always be installed as dev deps. {
"name": "@comp/utils",
"version": "1.0.0",
"alwaysIncludedDevDeps": ["jest", "typescript"],
"dependencies": {
"jest": "17",
"typescript": "4",
"ramda": "15"
}
} But I am not sure this feature is needed in the first iteration of the feature. |
Oh that's interesting. I do agree this is something most monorepos end up needing. I use a tool that checks whether a
π
Haha, well this "bug in pnpm" is my favorite feature. I could see why this company or team landed on the solution they did though. I'm sure
Sounds good! |
Forgot to respond to this. Yes, that example should have removed the |
actually, we could also probably share overrides, packageExtensions and other settings from the pnpm field of package.json via environments. maybe even some settings, like hoist-pattern and public-hoist-pattern. |
Sounds doable! Since the discussion has evolved beyond the initial scope of this RFC, I'm hoping to ask more questions in a short-lived discussion specific to the environments feature. pnpm/pnpm#5974. Would want to pick your brain a bit more in that discussion to make sure the new draft RFC matches your vision as closely as possible. |
Going to close this in favor of #3 for now. The ideas translate fairly closely.
A few ideas don't currently carry over to the new RFC, but I'm optimistic we can revisit them in some manner:
I kept this PR open for a bit because there's an alternative design where the Thanks for everyone who reviewed this RFC! I'm looking forward to #3 accomplishing the same goals. |
@pnpm/collaborators We'll need at least 1 more approving review to merge/ratify. Would another collaborator have time to read through the RFC and give thoughts? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the detailed RFC and it's a really smart idea!
So catalog:
would replace the functionality from what react": "workspace:*
does currently across multiple packages? Or I guess it's more advanced since you can specify multiple different catalogs for packages depending on different versions of a dependency.
From what I have seen, https://github.com/JamieMason/syncpack which you also mentioned works quite well with keeping deps in sync, so I would consider this more "nice to have".
Are there any related GH issues in the pnpm repo that specifically wish for this functionality? Is there any real user interest in having this apart from some upvotes of this RFC?
Personally, since there are always limited resources in OSS, I would consider bug fixes and perf improvements to pnpm more important than this feature. But then again, I don't have to struggle with huge monorepos, so I don't feel the pain.
text/0001-catalogs.md
Outdated
// Can be referenced through "catalog:default" or "catalog:" | ||
"default": { | ||
"jest": "^29.6.1", | ||
"redux": "^4.2.0", | ||
"react-redux": "^8.0.0" | ||
}, | ||
|
||
// 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" | ||
}, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagine that most people will only need default
, so for them, having to write a "default"
object after "catalogs"
will feel like a hassle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. I do agree. Here's another potential config structure:
// package.json
{
// Can be referenced through `catalog:default` or `catalog:`.
"catalog": {
"jest": "^29.6.1",
"redux": "^4.2.0",
"react-redux": "^8.0.0"
},
"namedCatalogs": {
// 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"
}
}
}
We could also name these defaultCatalog
and catalogs
, but under the idea that most users will just have a default catalog, I'd lean towards keeping that name shorter and just catalog
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also name these
defaultCatalog
andcatalogs
, but under the idea that most users will just have a default catalog, I'd lean towards keeping that name shorter and justcatalog
.
I'm fine with either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the document for the catalog
and namedCatalogs
split here: b8b4527
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also put everything to catalog like this
// package.json
{
// Can be referenced through `catalog:default` or `catalog:`.
"catalog": {
"jest": "^29.6.1",
"redux": "^4.2.0",
"react-redux": "^8.0.0"
// 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"
}
}
}
Inspired by npm's overrides syntax. If you want to use the same name for the catalog group as a dependency name, you can use .
. For instance:
{
"catalog": {
"jest": "^29.6.1",
"redux": "^4.2.0",
"react-redux": "^8.0.0"
"react": {
".": "^17.0.2",
"react-dom": "^17.0.2"
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how I feel about a separate file. I am not sure it is a great idea because we already have so many different ways to configure pnpm. But I will not reject the RFC if you decide to go with a separate file.
We might want to consider putting the new config file to a .pnpm
directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Catalogs looks like a workspace feature. I think it should be in pnpm-workspace.yaml. This config file currently only has one top-level field called packages
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From a solely end user experience perspective, it feels like there's no disagreement that a separate file (or reusing pnpm-workspace.yaml
) results in a better configuration story? Since the contention is codebase complexity and reducing the multiple ways to configure pnpm, I definitely trust @zkochan to speak to that best.
I will wait for @zkochan's thoughts on reusing the pnpm-workspace.yaml
file since I see a few others have reacted positively to that last suggestion. I personally want to provide the best user experience but maintaining a simpler codebase is also very important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will definitely need to revise the different ways we configure pnpm. But pnpm-workspace.yaml
already exists and as @KSXGitHub said the catalogs in the current state only make sense in a workspace, so I like this idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect! Here's the commit modifying the RFC to specify pnpm-workspace.yaml
. 2610503
I did switch back to catalog
/namedCatalogs
since it allows users to define a named catalog and a default catalog entry of the same name without the .
syntax.
Thanks for taking a look @mcmxcdev. Appreciate it.
That's right. Your take on this is accurate. My personal mental model is that this brings the nice ease-of-use ergonomics of
Good question! Off the top of my head, there's a few discussions I've seen related to this.
Another angle I'm hoping catalogs to one day address is reduce lockfile merge conflicts, which is another frequently discussed topic. Some merge conflict resistant parts of this RFC were punted, but I'm hoping we can revisit them if there's a way to implement them without introducing significant complexity to the codebase.
The resourcing question is definitely an important one to ask. My opinion is a bit biased from working heavily in huge monorepos day to day, but I do personally believe catalogs are a feature that uniquely boosts pnpm, and substantially. In the last few years, pnpm has shown that it scales much better than its alternatives. One particular reason is that it encourages "correctness" when declaring dependencies. The common workaround for sharing dependencies across monorepo packages is to put them in the root But now we have a new problem with the same dependency needing to be written multiple times in many I would also prioritize bug fixes and perf improvements to pnpm over new features. I think this is one of the cases we should do both since the feature complements pnpm's strengths. It enhances a user story that's made pnpm popular over the last few years. |
This makes it easier to define the default catalog, which we expect to be the most common usecase most of the time. pnpm#1 (comment)
I'll note my voracious interest here. We're struggling with this problem badly in our monorepo, especially as we attempt to start deploying module federation. We're in the midst of cobbling together a solution which involves strictly pinning every workspace package's dependencies to one version (e.g. every single the |
I am not at all an expert on build systems, but I did just want to add my voice saying
we are using pnpm with a monorepo, and are quite happy, but we have been wanting this exact feature. I've been following it with great interesting. just wanted to reaffirm that this work doesn't exist in a vacuum, though I of course know OSS resources are limited and I respect whatever the pnpm team chooses to do. that said, I hope that this feature makes it into pnpm :) (it looks like it is going that direction!) keep up the great work everyone |
Thank you @superfreddialpad and @jcoveney-anchorzero. It is really valuable hearing that we're creating a feature that will actually be useful. β€οΈ |
I too have been following this with great interest. Maintaining consistent versions in a monorepo is a pain, this feature would be super valuable! |
Where can we track the implementation of this? |
Thanks to everyone for their thoughts and feedback. The RFC is in a much better place from where it started. I'm glad to see this ratified and excited to begin working on it.
Good question. I've just opened pnpm/pnpm#7072 to track the implementation side of this. I'll also comment on this pull request so anyone following the RFC will be notified of its availability in the future. |
@gluxon with all the recent buzz, is there any way to incorporate bun into pnpm along with this RFC? or will they always be incompatible/competing? |
If I'm correct that you're referring to https://bun.sh/ β without regard to the merits of your particular suggestion nor those of Aside from the alleged recent buzz, is there anything about incorporating bun into pnpm that's germane to the |
@superfreddialpad The point is that since bun is itself a package manager, it might make sense to keep the APIs in sync between the two. This feature doesn't exist in either ecosystem so now would be the time to make that happen. |
@jakeleventhal I'm definitely in favor of any cross-pollination of ideas between package managers. (Zoltan in the past contributed to npm's isolated mode RFC as a prior example.) Implementing this in pnpm would be the first priority, but I'd personally be excited to see this feature in Bun (or other package managers) and happy to lend help if asked. |
This is an RFC for a new
catalog:
version specifier protocol in pnpm. All comments and feedback welcome. πEdit July 7th, 2023: A previous version of this RFC was titled "RFC: First-Class Support for Workspace Consistent Versions". We've renamed the proposal to "Catalogs" following a similar idea from the Gradle build tool.
Previous discussions: