-
Notifications
You must be signed in to change notification settings - Fork 3
Add support for pnpm v10 #43
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
Conversation
a1c50d7 to
00e3c4d
Compare
| if (!pnpmVersion || !(pnpmVersion.startsWith('8') || pnpmVersion.startsWith('9'))) { | ||
| if ( | ||
| !pnpmVersion || | ||
| !(pnpmVersion.startsWith('8') || pnpmVersion.startsWith('9') || pnpmVersion.startsWith('10')) |
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.
@gavinxgu Seems like '80' also startsWith('8'). Should we check for something like 8.?
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.
Should lockfileVersion also be changed from '6' to '6.'? However, I noticed that in the type definitions for lockfile v6, lockfileVersion is defined as string | number. In some cases, if it returns the number 6 and we apply toString(), wouldn't that cause '6.' to fail to match?
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 parse it into a number and then do a numeric comparison for that one. I doubt that lockfileVersion will use X.Y.Z syntax -- it is probably just an integer (or maybe regular decimal).
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.
lockfileVersion is semver consisting of major and minor. It is the version of pnpm that first shipped the new format. The current format is 9.0, which was shippped in pnpm v9.0.0.
- Add pnpm v10 compatibility - Add multi-version pnpm testing to ensure cross-version compatibility
00e3c4d to
61172f6
Compare
| } | ||
| } | ||
|
|
||
| export const isPnpmV8 = (pnpmVersion: string): boolean => pnpmVersion.startsWith('8.'); |
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.
@octogonz I extracted the logic for checking pnpm version into three utility functions.
| */ | ||
| packages: Record<string, ILockfilePackage>; | ||
| } | ||
| export type ILockfile = LockfileObjectV6 | LockfileObjectV9; |
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.
@octogonz Because we changed to use the official lockfile type provided by pnpm, this change might be a breaking change to the return value type of the readPnpmLockfile function. P.S. However, I'm not quite sure why we previously defined our own pnpmlock file type instead of using the official one.
Uh oh!
There was an error while loading. Please reload this page.