Skip to content
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

feat: pnpm workspaces support #3284

Merged
merged 35 commits into from Aug 31, 2022
Merged

feat: pnpm workspaces support #3284

merged 35 commits into from Aug 31, 2022

Conversation

fahslaj
Copy link
Contributor

@fahslaj fahslaj commented Aug 9, 2022

Description

Add support for pnpm workspaces:

  • Detect packages using pnpm-workspace.yaml when npmClient is pnpm (and enforce "useWorkspaces": true in lerna.json)
  • Detect workspace: protocol when building the package graph for commands.
  • Properly update workspace: protocol dependencies during lerna version
  • Prevent usage of lerna bootstrap, lerna link, and lerna add when using pnpm and direct the user to the pnpm docs. Instead dependencies should be managed and installed directly using pnpm.
  • Prevent usage of pnpm without useWorkspaces: true
  • Update the pnpm lockfile after lerna version

Adds typescript as an explicit dependency of Lerna to satisfy strict peer dependency checking.

Motivation and Context

#1818
#3195

This change allows users to use pnpm and lerna easily in the same project.

How Has This Been Tested?

I tested changes manually with multiple test workspaces. Commands I tested manually include: version, publish, ls, run, bootstrap, link. I also added comprehensive unit tests around the new functionality and a couple end to end tests for versioning and publishing.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • 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)

Checklist:

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

@fahslaj fahslaj marked this pull request as ready for review August 11, 2022 22:58
commands/add/index.js Outdated Show resolved Hide resolved
commands/add/__tests__/add-command.test.js Outdated Show resolved Hide resolved
commands/bootstrap/__tests__/bootstrap-command.test.js Outdated Show resolved Hide resolved
commands/bootstrap/index.js Outdated Show resolved Hide resolved
commands/link/__tests__/link-command.test.js Outdated Show resolved Hide resolved
e2e/utils/fixture.ts Outdated Show resolved Hide resolved
e2e/utils/fixture.ts Outdated Show resolved Hide resolved
e2e/utils/fixture.ts Outdated Show resolved Hide resolved
e2e/utils/fixture.ts Outdated Show resolved Hide resolved
e2e/utils/fixture.ts Outdated Show resolved Hide resolved
@JamesHenry
Copy link
Member

Great work @fahslaj! Excited about this

@lefreet
Copy link

lefreet commented Aug 20, 2022

look forward

Copy link
Member

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

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

Please see slack conversation for final testing plan before we land this

Copy link
Member

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

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

Let's do this!

@JamesHenry JamesHenry merged commit 1b18dbe into lerna:main Aug 31, 2022
@fahslaj fahslaj deleted the pnpm-support branch August 31, 2022 17:07
@AviVahl
Copy link
Contributor

AviVahl commented Aug 31, 2022

Adds typescript as an explicit dependency of Lerna to satisfy strict peer dependency checking.

Could you clarify the reasoning here? how is lerna using typescript? is it using it via a third party?

@JamesHenry
Copy link
Member

@AviVahl typescript is used at runtime for workspace package-graph analysis

@AviVahl
Copy link
Contributor

AviVahl commented Aug 31, 2022

Thanks. I've checked the code and this is only done by nx.

Installing nx, and now typescript, for everyone who uses lerna is beyond me. The direction this project has taken is quite worrisome from my perspective.
Others might not agree with the way I look at these things, but I'm personally starting to look for alternatives. :(

@JamesHenry
Copy link
Member

JamesHenry commented Sep 1, 2022

Hi @AviVahl @PeterShershov @YardenPorat 👋

We can appreciate the instinctive concern, however please allow me to break down the full context here. Hopefully it will set your minds at ease.

Adding a dependency on Nx in order to use it within the implementation details of Lerna increases the size of the node_modules but only by a very small amount. An empty Lerna workspace without Nx is about 72MB, and an empty Lerna workspace with Nx is 75MB. It’s about a 5% increase for a completely empty workspace. For non-empty workspaces with real app and libs, the increase should be less than 1%. This is very small, and even this very small increase is a temporary one. By adding Nx, we will now be in a position to remove some older packages, so in the longer run the result will be smaller node_modules for all workspaces.

Lerna provides many capabilities: running tasks, bootstrapping, publishing, generating packages, importing existing packages. Unless you use all of those things, some percentage of the 72MB that Lerna had is already something you didn’t need or use. The small, temporary increase you see should therefore not be significant.

So that's the small, and temporary, trade-off covered, but what exactly does using Nx within some of lerna's implementation details enable for us and all lerna users?

Well, it's very, very significant:

  1. One of the most-requested, longest-standing feature requests to Lerna is a dry-run mode for lerna version and lerna publish. Things like this are very hard to implement correctly. Thankfully Nx has such a capability baked in already, which has been battle-tested over many years. Thanks to Nx being there behind the scenes, we are going to be able to provide every single user with a --dry-run flag on lerna version and lerna publish which will show you what it will update (including CHANGELOG.md files), but without changing anything on disk (or making network requests) until you are ready.
  2. lerna create right now is not particularly useful or configurable, Nx's mature code generation allows us to rapidly improve on what lerna create can do (again with a reliable --dry-run capability "for free")
  3. lerna import, a relatively complex command to reason about, can also potentially be improved by showing what the result would be like without updating anything on disk.
  4. It’s very hard to evolve lerna configuration right now because a lot of cleanup items would be breaking changes and it would be left to all users individually to deal with the consequences (just as with any OSS project). However, Nx has a built-in capability to autofix configurations, which we can leverage to get rid of legacy options in lerna.json and more. These improvements will be autofixed via a single command thanks to Nx being there in the implementation details.
  5. Via Nx we can instantly provide a rich VSCode experience for Lerna users, including the built-in project graph visuazlier and other things like that. A lot of folks (e.g., RedwoodJS folks) have already been using this on their pure Lerna workspace and found it very useful for understanding how the workspace is structured and what will need to be refactored
  6. Task running is faster and more ergonomic, with built-in distributable (vital for CI) caching

In summary then, leveraging Nx inside of lerna's implementation details should allow us to improve a lot of core capabilities of Lerna: code generation, importing, publishing, and running tasks - and also implement hotly requested, but otherwise complex to achieve, feature requests. All without requiring users to refactor their existing workspaces, or change their existing commands!

I hope that is all clear, if you have any questions about the above, or specific concerns about a particular package, or something is causing issues for you, please let us know and we will look into it for you.

Many thanks for being a part of the lerna community and for sharing your thoughts 🙏

@lefreet
Copy link

lefreet commented Dec 1, 2022

@fahslaj pnpm-lock file not update after published.

  • so get some error in ci
 ERR_PNPM_OUTDATED_LOCKFILE  Cannot install with "frozen-lockfile" because pnpm-lock.yaml is not up to date with __packages__/ep-basic/package.json
Note that in CI environments this setting is true by default. If you still need to run install in such cases, use "pnpm install --no-frozen-lockfile"

@JamesHenry
Copy link
Member

JamesHenry commented Dec 1, 2022

Hi @lefreet I commented on the discussion here: pnpm/pnpm#4241 (comment)

Please don't comment on old closed issues/PRs, it's not a scalable way to get support as it will likely be missed and/or be combined with stuff which isn't actually applicable to your specific concerns.

Please take a look at what I have written on the discussion and if you are still having trouble please open a new issue with as much information as possible about your specific situation.

@lerna lerna locked and limited conversation to collaborators Dec 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants