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

[rush] Rush is not compatible with pnpm v8 due to lockfile v6 format #4033

Closed
koen-dev opened this issue Mar 28, 2023 · 12 comments
Closed

[rush] Rush is not compatible with pnpm v8 due to lockfile v6 format #4033

koen-dev opened this issue Mar 28, 2023 · 12 comments

Comments

@koen-dev
Copy link

koen-dev commented Mar 28, 2023

Summary

When posting this PNPM just released v8.0.0 which introduces a major upgrade for the lockfile format. This format isn't compatible with a check that Rush executes after a rush install. The following error occurs after rush install:

ERROR: version.includes is not a function

Repro steps

  1. Upgrade PNPM in any Rush repo from v6 or v7 to v8 by changing the setting pnpmVersion in rush.json.
  2. Delete pnpm-lock.yaml in common/config/rush
  3. Run rush update --full --purge

Above mentioned steps are following the instructions from the Rush docs.

Expected result:
rush install succeeds and new pnpm-lock.yaml is created.

Actual result:
rush install fails with the following message: ERROR: version.includes is not a function. However the lock file is created and looks valid.

Details

This looks to be related to the following line:

return !version.includes('link:');

Due to the new format of the lockfile the dependencies aren't split like this anymore:

'@types/uuid': 8.3.4
axios: 0.21.4
date-fns: 2.29.3

The new format is:

'@babel/core':
  specifier: ^7.21.0
  version: 7.21.0
'@babel/plugin-proposal-dynamic-import':
  specifier: ^7.18.6
  version: 7.18.6(@babel/[email protected])

Let me also state that this is expected to not be compatible as it was just released and the Rush team needs time to support the newly released pnpm v8.

@Niek
Copy link
Contributor

Niek commented Mar 28, 2023

Just want to add that pnpm v7 can read lockfile v6 format, it just didn't use it yet.

@iclanton
Copy link
Member

Working on it.

@matthiasg
Copy link

There are also issues with shrinkwrapJson.specifiers which will lead to ERROR: Cannot convert undefined or null to object. Just in case someone has that issue and looks for this thread. Thanks @iclanton for working on it

@ankhzet
Copy link

ankhzet commented Apr 15, 2023

Spent several hours debugging this, from looking in all node_modules (nothing found, rush's own sources lie outside the managed repo), to digging into rush'es intestines.
The lack of debug info when the error occurs inside the rush's own code is disappointing and misleading (rush --debug gave false impression that it's unlikely to be an error within rush itself), as the stack trace is stripped from the error log, AND it prevents the community from helping you find and debug errors.

Would like to see stack-traces when --debug flag is set, at least if it's a native Syntax/Type error, and not an explicit throw new Error(<message>) it would make a lot of sense. What do you think @iclanton , RUSH team?

@woss
Copy link
Contributor

woss commented Apr 18, 2023

from the point of the community moving to pnpm8 is desirable but not mandatory. The most important thing is not to break the rushstack.

Judging by this, moving to pnpm8 is not critical.
image

@Niek
Copy link
Contributor

Niek commented Apr 18, 2023

@ankhzet
Copy link

ankhzet commented Apr 18, 2023

@woss the issue is not about moving to v8 support, the issue is, that

  1. You are locked to use <v8, (<v7) with caveats (pnpm's use-lockfile-v6 config option support should be implemented)
  2. If you do rush install in a rush monorepo, pmpm will print to you "new version of pnpm available". Some users will update pnpm, they will change the pnpmVersion in their config, and then they will receive cryptic errors in console.
  3. Some new users of rush will likely have pnpm v8 installed prior to using rush. Then they'll try to create rush monorepo and run into same problem as # 2. Some of them would not google for the problem, they'll just give rush up.
  4. Lockfile v6 would be an opt-in for pnpm v7, as per the feat!: lockfile format v6 pnpm/pnpm#5810, so a chunk of users might try it out and run into errors with rush

So solutions might be:
a) Prohibit pnpm >= v8 (>= v7 if use-lockfile-v6 config option not supported by rush) being specified in rush config
b) Detect lockfile version, warn about unsupported lockfile version (or re-convert back to v5.4 automatically)
c) Make use of pnpm's use-lockfile-v6 config option (https://github.com/pnpm/pnpm/pull/5810/files#diff-a489dadc450f91d0821f9648c94c7e63bba46fa6d22cf6d8ab2a20785f8c5812R10)
d) Implement adapters for both v5+ and v6 lockfile versions support

@woss
Copy link
Contributor

woss commented Apr 18, 2023

Valid points. I think that implementing option c would be the best fit for now. This would still allow people to use v7 and prep for the v8. nobody uses ( or should not use ) pnpm v6. And if people use it then rush should fallback to current behaviour.

Another thing is that there could be a message of equal size as pnpms that states something like DO NOT UPGRADE PNPM in the rush.json, it is not compatible

@iclanton
Copy link
Member

So I'm currently out of town with kinda spotty internet and I'm not going to be back until the first week of May. I was hoping to have this done before I left, but other things came up. I've opened a draft PR (#4065), but it isn't working yet. If someone wants to continue where I left off, feel welcome. Otherwise, I hope to get this done in early May.

FYI to @dmichon-msft, @D4N14L, @octogonz, and @elliot-nelson.

cofl added a commit to thatsa-gg/ScholarGlenna that referenced this issue May 9, 2023
That's the latest version compatible with Rush at this time. PNPM v8+
use lockfile format 6, which is not compatible.

See: microsoft/rushstack#4033
@azerum
Copy link

azerum commented May 10, 2023

Thank you for opening the issue. I was wondering why rush commands failed with seemingly random messages. As a newbie in Rush, error message about incompatibility with pnpm v8 would be appreciated :)

@smolinari
Copy link

smolinari commented May 11, 2023

I also ran into this issue. I saw pnpm's upgrade information banner and blindly upgraded to pnpm v8 and ran into the lock-file compatibility issue. Then I went searching the Internet for answers to find this issue.

Suggestion: if there is a major version upgrade of pnpm (or any other package manager) and Rush needs time to support it, the pnpm upgrade info banner should be suppressed somehow. Or add an additional banner saying, the version X of Y isn't yet supported by Rush. Not sure it will help, but this setting suppresses the banner (notifier).

And/ Or, have a compatibility table somewhere in the docs showing the different version matches.

Scott

@iclanton
Copy link
Member

This has been released as Rush 5.100.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

8 participants