Background:

Peer dependencies in lerna have an extensive history of discussion:

#1018
#3671
#2402
#955

Based on that past (that we inherited) any changes in this area are highly controversial.

I don't think we would land any changes in this area unless at a minimum gated by an option which was false by default.

Before considering this change in general, can you please elaborate on what you mean by "maintain a monorepo with Lerna that is used to build a suite of libraries that require peerDependencies to share memory"?

Are you referring to developing packages exclusively in the same monorepo that reference relationships to each other? Or publishing those packages to the outside world as well?

Can you expand upon the point around "sharing memory"?

@JamesHenry

I'll expand on the monorepo use case first before explaining why the need for shared memory exists and that drives the need for peerDependencies over ordinary dependencies.

The use case is a monorepo which is used to publish multiple packages (to npm). These packages form a Directed Acyclic Graph (DAG). The packages themselves may also become the peer dependencies for externally developed packages or more internally developed packages. When a client developer uses the packages, she chooses certain starting packages and the closure over the DAG determines all of the other packages that must be present at runtime. If the client developer is extending the ecosystem with a new package, she would define peerDependencies, but if the client developer were creating an application there would be regular dependencies.

I have two examples for this that I am working on. 1) A graphics library suite, and 2) A symbolic mathematics library suite. In both cases the idea is to create a suite of published packages where the client only incurs the loading cost (bytes) of what they use, and the client can write further packages to extend the ecosystem.

In both cases examples of needing to use peerDependencies over regular dependencies occur. These are:

1. A signal library which has a signal function and an effect function. One package may create signals, some other package may use effects. If copies of the signal library are embedded in the consumers then the mechanism will not work.

2. The ability to use JavaScript's instanceof to identify an instance of a class which won't work if the code defining a class is copied through ordinary dependencies by a bundler into more than one consuming package. An instance created in one consuming package will not be recognized by the other consuming package.

I understand that peerDependencies are a nuanced issue and I am familiar with the discussions in the issues that you listed.

#1018 does not seem to be relevant because it deals with semantic versions and not references using the file protocol. It is therefore misleading on this basis to say that peerDependencies should not be touched. It's also rather wierd that there is both a dev dependency and a peer dependency in this example, but that shouldn't be used as an argument to stop Lerna users who know what they are doing.

#3671 seems a little outdated. I think I am correct in saying that the file protocol has replaced the workspace syntax. However, the thrust of this issue is exactly what I am calling for.

#2402 doesn't deal with peer dependencies established through the file protocol. I agree with the comments that you don't want to change version numbers. This PR distinguishes between use of the file protocol and anything else and only makes changes for the former (and has tests to verify).

#955 Note the comment by Qwenty on Sep 5 "There's an easier edge-case I would like here, which is if the peer dependency is linked with file:../mypackage then we should just write this to the current known version of that package." This is exactly what this PR does. Nothing more. I think somewhere else someone put it stronger and said that we should update peer dependencies that are linked with the file:../mypackage protocol. Leaving a file protocol in a published package.json is simply a bug.

Finally, a word on some other approaches and why Lerna, IMHO, is still viable. I have visited turborepo and I am somewhat disappointed that it's CLI assumes that I'm trying to write a monorepo with a Next.JS application. The documentation is lacking in describing how to create a library-only repository. I'm an Nx user for an Angular application with around 159 libs. It works great, but I don't publish any of the libs. Maybe I can, just haven't figured how to bend Nx to the Lerna way of doing things. To me Lerna is to Nx as rollup is to webpack. Lerna is good for libs (or would be with this PR).

Thanks for looking into this James.

@JamesHenry

Given that this PR is a lot less controversial than a superficial reading of the related issues would suggest and that this PR really amounts to a bug fix, does your original statement...

"I don't think we would land any changes in this area unless at a minimum gated by an option which was false by default.",

still hold?

My thinking at this point is that a feature flag is not appropriate for several reasons.

1. It really is a bug fix and so to require a feature flag is therefore rather surprising and would be counter-productive for users.

2. As a measure to limit possible damage (potential damage control) it doesn't really provide much value and in fact amounts to technical debt that just kicks the can down the road. The worst case is that some user has a use-case that was not foreseen and that the change needs some adjustment. In that case, the user can simply use an exact prior version and report the issue. We learn something, make the fix, and move forward.

3. With an opt-in feature flag we never get to discover the issues until the flag is removed. Meantime it is technical debt that is really providing no value.

Thank you for the comprehensive write up @stemcstudio @geometryzen (two accounts on one PR 😅 ?)

I am happy to proceed with this as you have stated, with it's scope limited to replacing non-semver version identifiers with the latest known semver versions within peerDependencies, as the non-semver identifiers are objectively useless without modification.

However, your comment around file being more modern than workspace is actually the exact opposite way around AFAIR, workspace came much later, and we would definitely want to support it here.

workspace is more nuanced than file, depending on the package manager, because of what can come after the : but I think for now if we just ship support for workspace:* explicitly that will be enough and will allow us to close #3671 as well.

Please could you make that update?

I pulled in the latest main just to give you the initial CI feedback ahead of further changes

☁️ Nx Cloud Report

CI is running/has finished running commands for commit a5957b3. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

Sent with 💌 from NxCloud.

I have updated the PR title to match the conventions needed for merge (and preemptively added workspace support in there)

@JamesHenry

Thanks James, this is becoming a useful education!

Re-reading your comment about supporting "workspace:*" may make some of the following sound like I was not paying attention. Please forgive that. I think there is still a useful question to be asked about support for npm.

I have a question(s) for you (or more precisely the "steering committee"), and I'll present my suggested approach.

Here's the background...

I tried changing one of my "file:../some-folder" references to "workspace:*" and got the following from npm

Unsupported URL Type "workspace:": workspace:*

That's interesting! npm does not understand the "workspace:" syntax (not sure it's correct to say it's a protocol here).

So my question(s) to you is/are, would the Lerna "steering commitee" be more interested in supporting the "workspace:" syntax and therefore not providing support for npm, or would there be more interest in being more even-handed with regards package managers and thus support the "file:" protocol ( perhaps with the caveat that the Lerna "npmClient" property is "npm")?

The further question is about the way we might support the "workspace:" syntax (this is the bit where you already stated your opinion so feel free to disregard everything up to where I present my suggestion, sorry!). The yarn documentation (which seems to be the most explicit about the "workspace:" syntax) describes four cases,

"wokspace:{semver}", "workspace:^", "workspace:~", and "workspace:*".

The last case corresponds (I think) most closely to the "file:../" use case. I will have to re-read the other issues to see whether the other cases lead us into "contrversial" territory.

The question then is whether a PR relating to "workspace":" would be more acceptable if it only supported only the "workspace:*" case or if it tackled the full range of possibilities?

Finally, my suggestion.

My suggestion is that we nibble at the problem. Modify this PR to support the "file:../" protocol of npm, but have it only be active if the "npmClient" property in Lerna is "npm" ("npm" is the default, but oh well!)

Then we look at a following PR that supports "workspace:" for the other package managers ("yarn" and "pnpm").

Is this idea too "legacy"? I'm happy to move on to yarn or pnpm but that feels like abandoning npm too easily, throwing the baby out with the bath water?

I'm sorry for not being clearer before. I was not suggesting to support workspace instead of file, I was suggesting in addition to file. file can be used with the package managers that support workspace, but as you note, npm supports only file and not workspace.

And yes exactly there is nuance within workspace depending on if it's yarn or pnpm, and I think workspace:* is the one we can focus on initially. We already support replacing workspace references in other types of dependencies so hopefully you adapt that logic in this PR.

Please let me know if once you take a look at the existing precedent you feel I am oversimplifying (it's definitely possible) and you are struggling to pull it together and I can take a deeper look myself.

@JamesHenry

My fault for the misunderstanding, I should have read the title change more carefully (file AND workspace).

I'll take another look at the code.

Thanks

@JamesHenry

While I know you only asked for the "workspace:*" use-case, the latest draft handles all use-cases of the workspace protocol in peer dependencies:

workspace:*
workspace:^
workspace:~
workspace:{SemverRange}

We can discuss whether you think we should like to "dial-back" the functionality to just the "workspace:*" case. I took this approach for two reasons:

1. The handling of '^' and '~' was already implemented.
2. The handling of "workspace:{SemverRange}" took a bit more work to discover why it was not being considered as a dependency, and then some code to make the appropriate transformation.

The transformations are as documented at pnpm.io/workspaces#workspace-protocol-workspace.

My inclination is to accept all the use-cases. However, I understand that none of the package manager specs really distinguish between the different kinds of dependencies. So I could be missing something here but I don't think there are any lurking dragons and it seems like a bug to not transform them. On the other hand, I have only needed the "workspace:*" use-case in my own work.

Please note that issues #4024 (blocking) and #4022 (annoying!) would be good to be cleared before accepting this.

As an aside, I had to change my personal projects to use pnpm instead of npm (because npm doesn't recognize the workspace protocol). As a side benefit, pnpm caught some missing dependencies that I hadn't declared, which was nice.
I'm now a pnpm convert when using Lerna.

@JamesHenry

I'm struggling a bit here. I've managed to figure out how to run the E2E tests. I've reproduced the CI failure locally by running

npm run e2e-start-local-registry
npx nx e2e e2e-version.

However, I'm also getting the same result with a clean clone of the repo (i.e. without my changes).

The error is...

 ERR_PNPM_NO_MATCHING_VERSION  No matching version found for lerna@999.9.9-e2e.0

Browsing localhost:4782 confirms that only lerna@8.1.3 is installed.

Is there a regression on the main branch?

@stemcstudio I've had to do some work to update npm utility dependencies and pipelines, that's all now been merged and I've pulled it into your branch. Fingers crossed that makes it green, but at the very least it should now be in a better place to troubleshoot

Handling the different types of workspace specifier is great, glad it wasn't a lot of extra work

@stemcstudio Your e2e failure does seem legitimately related to your changes, I have responded on your linked issues

Fixing the environmental problem with #4024 has resolved the problems of running the e2e tests against mainline!

@JamesHenry

Ready for review! Almost!!

There was a problem with the e2e tests that was causing a timeout when I was testing locally. Although I wasn't able to debug step my way through it, it seemed to be caused by pnpm waiting on a dependency that was invalid. You will see that I have refactored the code in a recent commit to preserve legacy behavior when eraseWorkspacePrefixes is false, and to achieve the new behavior of re-writing the workspace specs otherwise. Although I don't fully understand the legacy behavior and how it impacts pnpm in detail, the tests show that the legacy behavior is preserved and there are no timeout issues.

Locally, I have been able to run unit, integration, and e2e tests.

I have not yet implemented the file protocol re-writing. There is a single line in the code that could be changed to produce this effect but it needs more testing. I backed out the change when I was debugging the timeout issue and didn't reinstate it. I'd rather do that in a new PR so this issue could be re-titled "update workspace specifiers in peerDependencies".

I also haven't thrown in the fix for #4022 as it isn't essential to this issue.

The only problem now is that the versioning in my own projects seems to be hanging!

I would however appreciate it if you could run the CI on the changes and give me any of your feedback.

Thanks

@JamesHenry

After upgrading pnpm from 8.x to 9.4.0, the issue I was having with hanging (logging reported updating pnpm-lock.yaml) resolved itself and I was able to publish my own monorepo packages.

I'm seeing that integration failure locally now. May have been a caching issue. Looks to be related to the change I made to ensure legacy behavior, but a test was not updated. Hopefully, an easy test fix.

The latest commit should fix the integration tests. It was a new test that I had added with the fixture being

libs/commands/publish/fixtures/relative-file-specs/packages/package_d/package.json

It's a strange use case. I'm not sure that it even makes sense or if someone would do it. But it is consistent with how Lerna would have treated the dependency specification on other dependency collections. It's possible that we might not touch such a specification when it happens under peerDependencies but I don't know if there is a good argument that it should be changed from how it behaves now.

@JamesHenry
It's starting to look to me, from my own experiments, that the success of e2e tests and real-life projects, both on the clean main branch and for the code to address this issue, depends on the version of pnpm. What is not good is that it appears that real-life projects favor a more recent pnpm, while e2e tests favor an older pnpm. It's painful to verify this because the e2e tests take so long. Anyway, that's my hypothesis.

Upon further experiments the results are not clear cut. I've had successes and failures on the clean and modified branches. There is some correlation with the pnpm version (tried 9.4.0, 8.15.8, 8.10.0) but this isn't 100%. There also appear to be a significant number of timeouts.

@geometryzen the CONTRIBUTING covers how you can run specific subsets of the e2e tests, please give that a try

@JamesHenry

Thanks. I was running smaller tests before but lost confidence that I was using the right incantations when things were failing. Now I see empirically that it doesn't matter. There seems to be a very strong correlation between pnpm@8.10.0 giving success and pnpm@9.4.0 giving failure. I'm considering options at this point. pnpm appears to have a chronic unresolved bug giving ERR_PNPM_NO_MATCHING_VERSION (which is what we see) for no apparent reason. And BTW, this is all on a clean Lerna codebase without my changes. I'm feeling that resolving this requires digging in to pnpm. I may try to bracket the version of pnpm when things go south.

If there's anyone out there following this and would like to give me a sanity check, try pulling a clean copy of Lerna and run the e2e tests with pnpm at the versions I have mentioned.

@geometryzen I'm not quite following why you are using pnpm v9 given our pipeline doesn't right now?

The focus of this PR should be getting it to work for our current pipeline, and you can leave migrating our pipeline to pnpm v9 to me

@JamesHenry

I'm in agreement! The situation I'm in right now is that Lerna, on my machine, with and without my changes, is passing all tests with 8.15.8 of pnpm. It's failing with 9.x. I don't know how the CI or anyone else is pinning the pnpm version, but that would be nice. Unfortunately I'm running out of runway. I'm not going to be able to apply any effort in the next three weeks and then probably not for the months of Aug and Sep. I'm using Lerna from my fork and that will have to do for the time being. I'd sure appreciate anyone else's help to bring this one home.

All the best,

David

@JamesHenry

Hi James,

just got back to this after the anticipated absence!

I've made the changes that you requested, and added a few extra test cases.

I have also elected to not support the file protocol in the case of peerDependencies as this could introduce incorrect behavior. While using the file protocol in other dependencies introduces the "^" token, I didn't want to make that assumption for peerDependencies. In other words, I'm electing to be conservative here by preserving the existing behavior, and deferring that use case until it is demanded and with better specification.

Finally, I'm not sure I can add much to your pnpm v9 observation; I don't really understand what you did in .npmrc. I still can't run the e2e tests to completion (with v9 of pnpm incidently). Maybe they work with version 8.15.8 (as I experienced before), but without a way to pin the version I'm not going to redo that test. I have successfully used my local version of Lerna in support of one of my own projects which uses pnpm@9.12.0

I really appreciate you coming back to this one, thank you! Just the one remaining query about the *NUMBER syntax

Regarding pnpm v8, everything looks to be working on the PR so it must be good here. I will open an explicit PR to check against pnpm v9

Then we look at a following PR that supports "workspace:" for the other package managers ("yarn" and "pnpm").

Is this idea too "legacy"? I'm happy to move on to yarn or pnpm but that feels like abandoning npm too easily, throwing the baby out with the bath water?

It's very interesting that workspace: specified starts to be supported by many package managers, including Deno (denoland/deno#18192), but not npm. In effect, you can't use it inside dependencies when using npm, but it seems like npm doesn't complain when you use it with peerDependencies 🤷🏻

In the WordPress project, we switched to npm workspaces a few weeks ago. As part of the effort, we replaced regular dependencies from relative paths prefixed with file: to *, but it doesn't work well with the Lerna publishing workflow as now all dependencies get published as *, which is not what we want. In effect, we had to rollback to relative paths:

• Packages: Fix published * workspace dependencies WordPress/gutenberg#68240

Ideally, we would be very happy if we could use workspace:* (or workspace:^) with both dependencies and peerDependencies knowing that Lerna can handle proper replacements during npm publishing.

Thanks so much again, @geometryzen, and I'm sorry this took so long, merged in #4203