feat(version): update file and workspace specifiers in peerDependencies

[geometryzen]

Extend existing functionality of updating version specifiers (during publishing) in dependencies, devDependencies, and optionalDependencies to peerDependencies, but only for relative file specifiers.

Description

1. Extend getLocalDependency function to look at peerDependencies but only include if the version is a "file" protocol.
2. Extend updateLocalDependency to use peerDependencies as an additional possible dependency collection.
3. Update unit tests for publishing relative file specifiers with two new cases for peerDependencies that are relative and peerDependencies that are not.
4. Update unit tests for version command that uses the same fixture for the publishing relative file specifiers to ensure relative file specifiers are not overwritten in git commit.

Motivation and Context

I want to maintain a monorepo with Lerna that is used to build a suite of libraries that require peerDependencies to share memory. The libraries and are all published at the same time with the same version number.

#955

How Has This Been Tested?

By extending existing an existing unit test fixture which already deals with this functionality (but only for dependencies, devDependencies, and optionalDependencies) with two additional monorepo packages to cover peerDependencies.

I ran the unit tests with changes described. The integration tests ran without changes. e2e tests would not run on "clean" forked Lerna. Tested the changes in a local verdaccio server on real project (geomemtryzen/g20mono).

No impact expected on existing functionality.

Types of changes

• [ X] Bug fix (non-breaking change which fixes an issue)
• [ X] New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Chore (change that has absolutely no effect on users)

Depending on how you interpret the #955 issue, leaving relative file specifiers in a published package.json file is surely a bug, but it was not originally a feature because of the fear of updating semantic versions.

Checklist:

• [X ] My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• [ X] I have read the CONTRIBUTING document.
• [ X] I have added tests to cover my changes.
• [ X] All new and existing tests passed.

Note: There is no explicit mention in the documentation of using the file protocol, i.e. relative file specifiers. I learned about this technique from watching James' video.

[JamesHenry]

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"?

[stemcstudio]

@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.

[stemcstudio]

@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.

[JamesHenry]

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?

[JamesHenry]

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

[nx-cloud]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 612fc6e. 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

---

✅ Successfully ran 8 targets

• nx run-many --t e2e --parallel=1
• nx run-many -t lint --parallel=3
• nx run-many -t test --parallel=3 --ci --maxWorkers=2
• nx run-many -t build --parallel=3
• nx run integration:integration --ci --maxWorkers=2
• nx-cloud record -- npx nx format:check
• nx prepare-for-e2e e2e-run-task-runner
• nx run-many -t test --parallel=3 --ci --maxWorkers=2 --testTimeout=60000 --testPathIgnorePatterns=save.spec.ts --testPathIgnorePatterns=find-file.spec.ts --testPathIgnorePatterns=chain-class.spec.ts

---

Sent with 💌 from NxCloud.

[JamesHenry]

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

[geometryzen]

@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?

[JamesHenry]

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.

[geometryzen]

@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

[geometryzen]

@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.