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] Let's get NPM 6 working with Rush #886

Open
octogonz opened this issue Oct 17, 2018 · 33 comments
Open

[rush] Let's get NPM 6 working with Rush #886

octogonz opened this issue Oct 17, 2018 · 33 comments
Labels
general discussion Not a bug or enhancement, just a discussion

Comments

@octogonz
Copy link
Collaborator

octogonz commented Oct 17, 2018

Rush shouldn't be in a position of recommending that people use the ancient NPM 4. Due to recent interest, I am opening this meta-issue to track the punch list of work items needed to get the latest NPM version to be supported by Rush.

Off the top of my head:

@octogonz
Copy link
Collaborator Author

@jbcpollak
Copy link

@pgonzal can I suggest you coordinate with @zkat?

@octogonz
Copy link
Collaborator Author

octogonz commented Oct 19, 2018

Sure! @zkat this would be the absolute most valuable issue for NPM to help us out with:

I can provide an isolated repro if needed. But the basic idea is that when we do this...

package.json

{
  "name": "rush-common",
  "description": "Temporary file generated by the Rush tool",
  "private": true,
  "version": "0.0.0",
  "dependencies": {
    "@rush-temp/app1": "file:./projects/app1.tgz"
  }
}

...after npm install has run once, if the "app1.tgz" file is updated by Rush, thereafter npm install seems to always install the old tarball. The only way to make it recognize the change is EITHER:

  1. Delete both node_modules AND package-lock.json (which defeats the point of an incremental update), OR
  2. Delete node_modules AND run npm cache clean --force (which is terrible performance)

Anything less than that, and NPM continues to install the old tarball from the cache, ignoring our updates. This more or less was working in NPM 4, but repros in both NPM 5 and NPM 6.

@octogonz
Copy link
Collaborator Author

When I tried workaround #2 again just now with the latest NPM 6.4.1, it's crashing with this call stack:

Unhandled rejection Error: invalid config key requested: errors
    at BadKeyError (C:\Users\pgonzal\AppData\Roaming\nvm\v8.10.0\node_modules\npm\node_modules\figgy-pudding\index.js:93:23)
    at pudGet (C:\Users\pgonzal\AppData\Roaming\nvm\v8.10.0\node_modules\npm\node_modules\figgy-pudding\index.js:101:5)
    at FiggyPudding.get (C:\Users\pgonzal\AppData\Roaming\nvm\v8.10.0\node_modules\npm\node_modules\figgy-pudding\index.js:27:12)
    at Object.get (C:\Users\pgonzal\AppData\Roaming\nvm\v8.10.0\node_modules\npm\node_modules\figgy-pudding\index.js:159:16)
    at Object.checkData (C:\Users\pgonzal\AppData\Roaming\nvm\v8.10.0\node_modules\npm\node_modules\ssri\index.js:232:22)
    at write (C:\Users\pgonzal\AppData\Roaming\nvm\v8.10.0\node_modules\npm\node_modules\cacache\lib\content\write.js:34:31)

If I also delete package-lock.json then this error goes away.

@octogonz octogonz added the general discussion Not a bug or enhancement, just a discussion label Oct 19, 2018
@scsewall
Copy link

I also ran into that during my investigations. For me, I think that it was caused by NPM 6 encountering a package-lock.json generated by NPM 5. I had to delete npm-shrinkwrap.json to get past it.

@octogonz
Copy link
Collaborator Author

octogonz commented Oct 19, 2018

In this case the package-lock.json was created by the same version of NPM. (This was an isolated manual repro, not a real Rush installation.)

@zkat
Copy link
Member

zkat commented Oct 26, 2018

hey, y'all! I'd love to help out here with any questions y'all have (though I've got pretty low availability lately). One thing to note: npm is moving away from request entirely with the next release, so that one will at least be addressed.

I'll also note that I don't believe anything having issues with npm@5 is worth fixing: we no longer support that version, and we won't be doing any additional releases in the 5.x line unless there's a very major security issue.

I'm curious what the read-package-tree issue is -- this is another thing we're planning on rewriting very soon, so knowing what we need to watch out for will be useful.

@octogonz
Copy link
Collaborator Author

hey, y'all! I'd love to help out here with any questions y'all have (though I've got pretty low availability lately). One thing to note: npm is moving away from request entirely with the next release, so that one will at least be addressed.

That's really great! It should really improve reliability of NPM under heavy usage.

@zkat what about the "file:" version specifiers issue that I mentioned above? That one really hurts performance for NPM. It's definitely our biggest need.

@octogonz
Copy link
Collaborator Author

I'll also note that I don't believe anything having issues with npm@5 is worth fixing: we no longer support that version, and we won't be doing any additional releases in the 5.x line unless there's a very major security issue.

That's fine. The issues I called out at the top of this issue all affect the latest NPM. If we can get the latest NPM working, that should be sufficient for most people's needs.

@abirmingham
Copy link

In the meantime, is it best practice to use PNPM? Thanks!

@octogonz
Copy link
Collaborator Author

octogonz commented Aug 5, 2019

We don't really recommend one of the package managers as being "best". The decision depends on your needs. But I believe the Rush docs call out some non-subjective considerations:

  • NPM has known compatibility issues with Rush, as detailed in this thread. We want to get that fixed, but there doesn't seem to be a lot of community interest in it, not even for fixing the bugs that were identified in NPM itself.
  • Classic Yarn uses the same installation model as NPM, which suffers from the phantom/doppelganger issues that are solved by PNPM's symlinking algorithm
  • Yarn "Plug 'n Play" has similar benefits as PNPM, but this mode is relatively new, and Rush doesn't support it yet. (We'd welcome a PR for that!)
  • If you choose PNPM, certain older libraries may encounter compatibility issues because they aren't honoring the NodeJS spec, and instead rely on quirks of NPM. Most of these issues have been solved by now, but there are still a few stragglers.

Fortunately, switching package managers is pretty easy with Rush, and rush install almost completely hides these details from developers working in your repo.

@Jabher
Copy link
Contributor

Jabher commented Aug 24, 2019

I'm experiencing this error while using my company's proxy (while using proper syntax like ^1.2.3). For now I've rolled back to yarn, which is working, but I hope this will be fixed someday.

@octogonz
Copy link
Collaborator Author

Status update: NPM recently announced that version 7 will finally provide support for monorepos. Today, the monorepo support implemented by Yarn and PNPM is already reasonably close to what we do in rush install and rush link. Thus, we're considering an overhaul of Rush that would delegate most of the installation/linking operations to the underlying package manager. The Rush maintainers currently spend a lot of time investigating/maintaining issues with installation edge cases, so this would offload a lot of that, freeing us up to focus on more differentiating features such as sharded builds, multi-project watch, publishing workflows, etc.

One downside is that NPM and Yarn classic will lose Rush's protection against phantom dependencies. But we've found that users with complex installs inevitably move to PNPM or Yarn Plug'n'Play anyway. But an upside is that you would be able to use the package manager's monorepo commands in your Rush repo.

@kenotron
Copy link
Member

Awesome direction, @octogonz. Do you have another tracking issue on this? Would love see more details on what it would mean to have workspaces support with rush

@NikitaIT
Copy link

NikitaIT commented Sep 14, 2019

I have 2 projects one with vue-cli and one with webpack.
when i installed through:

npm WARN tarball tarball data for @rush-temp/my-project@file:projects\my-project.tgz (sha512-efOjexF3ji60ua49SHlPR1f1QU2cwUYRZiaFVxf0YM2vVj8hFSi8usX0UxTXkkOt3d2p3QUy0+tAn253d2mQ8w==) seems to be corrupted. Trying one more time.
npm ERR! cb() never called!

and etc

I spent a lot of time to earn at least something, but nothing worked.
With pnpm, you simply cannot build a vue-cli project.
With yarn, you cannot resolve dependencies of a dependent project.
With npm do not install dependent project.

@NikitaIT
Copy link

NPM was my last hope and I left it here

@octogonz
Copy link
Collaborator Author

octogonz commented Sep 15, 2019

@NikitaIT Could you share a repro of your build that failed using PNPM?

I've heard from several people that vue-cli does not correctly declare its imports, making it incompatible with PNPM and Yarn Plug'n'Play. I remember seeing that their Troubleshooting page says "It is because webpack resolves symlinks to their real locations by default, thus breaks ESLint / Babel config lookup". This suggests that the Vue maintainers don't understand that Vue itself is the cause of this trouble. I investigated it a while ago, but since I don't use Vue myself, I simply followed their tutorial and was unable to repro the error.

If you could share a PNPM repro, I'd be willing to debug it, determine whether the problem is really Vue, and if so open a proper issue to help get this solved. As stated above, Rush does have plans this fall to enable NPM/Yarn to use their conventional ("shamefully flatten") installation strategy, which should solve your problem. But that's not a scalable way of installing packages (due the phantom/doppelganger concerns), so this "fix" wouldn't help larger monorepos that really do need a PNPM or Yarn Plug'n'Play installation strategy.

@NikitaIT
Copy link

NikitaIT commented Sep 22, 2019

@octogonz
Repo: rush pnpm by default + vue-cli(dart-sass, babel, typescript) by default

rush init 
vue create vue-cli-3-ts-project
// add vue-cli-3-ts-project to rush.json
rush update
rush install
rush build
// oops!
// run with 
rush-vue-cli\common\temp\pnpm-local\node_modules\pnpm\lib\bin\pnpm.js run serve --scripts-prepend-node-path=auto

Init only:
https://github.com/NikitaIT/rush-vue-cli

  • Failed to resolve loader: cache-loader // I know that it will be for all loaders, even if fix it for cache-loader
  • 36:1 This syntax requires an imported helper but module 'tslib' cannot be found

Symbolic Links in node_modules(Troubleshooting):
https://github.com/NikitaIT/rush-vue-cli/tree/off-resoleve-symlinks

  • same errors, nothing happened, expectedly

Win 10, node v10.16.3

@octogonz
Copy link
Collaborator Author

BTW I've opened this issue to track the feature to enable Rush to use the package manager's monorepo support (workspaces): #1553

@ChocPanda
Copy link

Hey @octogonz has any progress been made on this?

@octogonz
Copy link
Collaborator Author

octogonz commented Nov 4, 2019

It's next in the queue in our roadmap.

@jblevins1991
Copy link

Any movement on this? I am getting some serious npm errors in my pipeline due to corrupted package.tgz files. I am hoping this fixes that issue.

@oravecz
Copy link

oravecz commented Mar 15, 2022

If you choose NPM, you may need to use an older release. NPM 5.x and 6.x are both known to have unresolved regressions that cause trouble in Rush repos. NPM 4.5.0 is the most recent version that's known to work very reliably, but unfortunately it's pretty old. (We'd greatly appreciate community help improving this situation. We're using GitHub issue #886 to track this effort.)

Ok, if NPM 5 and 6 are no good, how about NPM 7 and 8? Really surprised to see the docs so far behind on this issue. And this issue hasn't been updated in 2+ years. Feel like I am missing something rather obvious?

@fahmifan
Copy link

fahmifan commented Aug 10, 2023

any updates on this? I plan to move from yarn because of this issue
#1748

and pnpm is not possible, since it breaks some build on my apps

@iclanton iclanton moved this to Needs triage in Bug Triage Aug 23, 2023
@iclanton
Copy link
Member

@fahmifan - We encountered a number of issues with NPM over the years, so supporting it hasn't been a priority. We'd happily take a PR to update support for NPM, but our team likely won't put that together ourselves.

What issues are you hitting with pnpm?

@iclanton iclanton moved this from Needs triage to General Discussions in Bug Triage Aug 23, 2023
@octogonz
Copy link
Collaborator Author

Today the important differences are not the package manager itself, but rather the underlying installation model. The Lockfile Explorer docs summarize the reasons why NPM's installation model is not a good tech bet for a monorepo, regardless of whether you are using Yarn Classic or NPM.

and pnpm is not possible, since it breaks some build on my apps

From my experience, there is almost always a way to get PNPM working, and although it can be nontrivial work, it generally turns out to be less work than other approaches such as trying to run your monorepo with Yarn or NPM. The method for fixing PNPM incompatibilities generally involves .pnpmfile.cjs overrides, or else a small patch to a poorly behaved tool so that its module resolver correctly handles symlinks. For newer PNPM versions, Rush now supports rush-pnpm patch which allows patching to happen during installation, avoiding the need to modify or fork the upstream project.

@fahmifan
Copy link

fahmifan commented Aug 29, 2023

@iclanton I have issue when running our FE app built with quasar v1 locally, the error is:

This dependency was not found
$WORKSPACE/$PROJECT/quasar/client-entry.js in multi $PROJECT/common/temp/node_modules/.pnpm/webpack-dev-server@3.11.3_webpack@4.46.0/node_modules/webpack-dev-server/client?http://0.0.0.0:8080 (webpack)/hot/dev-server.js ./.quasar/client-entry.js

I think maybe it's caused by babel-loader, so I tried pnpm shamefully-hoist, got different error

@vjau
Copy link

vjau commented Dec 23, 2023

I have been moving all my packages in a monorepo with rush and pnpm for some months with great success.
However, i found out, that there are two (big) environments that end up being problematic (near impossible ?) with pnpm :

  • electron
  • react native

@drazisil
Copy link

drazisil commented Apr 2, 2024

👋 This issue mentions npm 6, but we are currently (as of Node.js 20) on npm 10. Is this still an issue, or something that was missed in a prioritization/close cycle?

@octogonz
Copy link
Collaborator Author

octogonz commented Apr 4, 2024

Yes, we should probably close this ticket. NPM's installation model is not a good fit for large code bases, and the compatibility issues with PNPM are now fully solved, so it is very hard to find anyone willing to work on NPM support for Rush.

You can find some background here:
https://lfx.rushstack.io/pages/concepts/install_models/

@iclanton
Copy link
Member

iclanton commented Apr 5, 2024

@octogonz - you want to just go ahead and close this issue?

Maybe before we close this we should add a feature to Rush test prints an explanation for our minimal support for NPM.

@bijela-gora
Copy link

Just wondering how many users are using Rush with npm?

@GaryB432
Copy link

GaryB432 commented Oct 25, 2024

I am bumming out with NX and my svelte-kit project and was investigating rush. I use a windows machine and npm 10!! with node 20. my workspace would be pretty small. the svelte-kit app and another node app or 2 and 5 node libraries. should i upskill to pnpm?

my early reading has me under the impression that rush uses its own pnpm under the hood. so is my package manager even germane?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general discussion Not a bug or enhancement, just a discussion
Projects
Status: General Discussions
Development

No branches or pull requests