-
Notifications
You must be signed in to change notification settings - Fork 604
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
[rush] Add support for PNPM version 3.x #1210
Conversation
…onstants should contain constants only
@iclanton I tested this over the weekend, and it's in good shape, but I found that PNPM 3.0.0 fails if the new option is provided. I was working on a fix for that yesterday but got distracted. I can finish it tonight. |
…on-strategy` option; add a new PackageManagerFeatureSet class
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.
AWS
export type PackageManager = 'pnpm' | 'npm' | 'yarn'; | ||
|
||
/** | ||
* Reports the known features of a package manager as detected from its version number. |
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.
Drop the "number" at the end of this sentence.
|
||
this.supportsPnpmResolutionStrategy = false; | ||
|
||
switch (this.packageManager) { |
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.
It kinda feels like this data should be expressed in JSON files. TS for code, JSON for data.
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.
Maybe, but let's collect a little more "data" before we design a specialized manager for it
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.
Sure - that's fine.
This PR was published with Rush 5.7.0 |
In PR #1244 the build is failing with this error:
It seems the shrinkwrap.yaml changed from this: 'file:projects/rush-lib.tgz':
dependencies:
'@pnpm/link-bins': /@pnpm/link-bins/1.0.3/@[email protected] ...to this in the new pnpm-lockfile.json: 'file:projects/rush-lib.tgz':
dependencies:
'@pnpm/link-bins': 1.0.3_@[email protected] @tnc1997 @bsiegel did you encounter this? Maybe we need to update the version parsing in PnpmShrinkwrapFile.ts |
Hi @octogonz, I've just checked out all of the latest changes and I can confirm that I too am getting the same error. As you mention, it seems to point towards a change in how version numbers are represented in the latest version of pnpm, which might require a modification to the version parsing used. |
I'm going to see if I can debug it tonight |
Okay, I have a fix. I'll create a PR later tonight. |
Here's the PR: #1246 |
This pull request follows on from issues #1145 and #1180 and adds preliminary support for pnpm version 3.
The
--resolution-strategy
flag has been added to thepnpmOptions
section ofrush.json
and both of the different filenames given to the pnpm 'lock' files based on the pnpm version are supported.