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

[RRFC] Add nohoist option for workspaces #287

Open
socialwyze-franklin opened this issue Nov 11, 2020 · 62 comments
Open

[RRFC] Add nohoist option for workspaces #287

socialwyze-franklin opened this issue Nov 11, 2020 · 62 comments

Comments

@socialwyze-franklin
Copy link

Motivation ("The Why")

Yarn already supports this option. It makes it possible to work with tooling that expects certain modules to be located in inside the same package that consumes them.

Example

In order for a React Native app to build properly within a monorepo, some packages that react-native depends on must reside inside the node_modules folder within the app package. With yarn, I'm able to accomplish it with this configuration:

  "workspaces": {
    "packages": [
      "api",
      "app",
      "shared/*"
    ],
    "nohoist": [
      // my-app is the name of the package inside the app folder
      "my-app/**"
    ]
  }

How

See: https://classic.yarnpkg.com/blog/2018/02/15/nohoist/

References

@ljharb
Copy link
Contributor

ljharb commented Nov 11, 2020

What I'd prefer, rather than adding "nohoist" and having to manually configure it, is finding a way for nothing to hoist, so that each folder's node_modules directory only ever contained the dep graph that its own package.json would have produced.

@darcyclarke darcyclarke added the Agenda will be discussed at the Open RFC call label Nov 18, 2020
@simllll
Copy link

simllll commented Nov 19, 2020

What I'd prefer, rather than adding "nohoist" and having to manually configure it, is finding a way for nothing to hoist, so that each folder's node_modules directory only ever contained the dep graph that its own package.json would have produced.

sounds very much likes "pnpm" approach of doing things.

@isaacs
Copy link
Contributor

isaacs commented Dec 2, 2020

Current status:

  • At the very least, the canonical option should be hoist: false rather than nohoist: true to avoid the "notfalse" config antipattern. (Whatever direction we go, we can support nohoist as well.)
  • Some discussion around whether this should be per-workspace, or for the project as a whole.
  • Main hazard: can easily result in workspace projects that install fine in dev, but cannot be correctly used together in production. Eg, packages/foo has a peerDep on react@16.8.0 and packages/bar has a peerDep on react@16.13.0. Without hoisting, this conflict is not evident in development. As @ljharb points out, this is desired behavior for projects that provide separate connectors for different react versions, but definitely something that should be an opt-in.
  • Discussed other pnpm-style tree generation approaches, and/or having separate "tree strategy" settings for different workspaces. Eg, packages/foo uses global-style, packages/bar uses legacy-peer-deps, etc. (This would be a pretty significant refactor for arborist, but not impossible.) Decided that this is orthogonal to the question of whether we hoist to workspace parent dir or not.

@ljharb
Copy link
Contributor

ljharb commented Dec 2, 2020

I'd amend this to say, "hoisting" is the improper term in every case - that what is actually desired is control around "sharing".

I think by default, the root, and individual subpackages, should share all peer deps, and all subpackages themselves, but observably share nothing else (iow, it's fine if they actually share the same copy of lodash, but a workspace shouldn't be able to require lodash unless its own dep graph includes it).

@darcyclarke darcyclarke removed the Agenda will be discussed at the Open RFC call label Dec 16, 2020
@zameschua
Copy link

Hi guys is there any progress on this issue? I really want to use npm for workspaces but I can't because react-native projects require a nohoist option..

@BitForger
Copy link

BitForger commented Oct 18, 2021

This would actually be very helpful for monorepos with Angular and certain dependencies that look for angular core and common as peer deps.

I'm encountering a scenario where I have an angular theme library that looks for angular/core and angular/common as a peer dep but it's getting hoisted to <project_root>/node_modules. Angular isn't because I have other packages that depend on a newer version of rxjs then angular.

@rmuchall
Copy link

I would also like to add my support for a nohoist option. I've ran into problems with eslint configurations in a monorepo because it expects to be installed at the local package level.

@d3lm
Copy link

d3lm commented Oct 22, 2021

I agree. A nohoist option would be very useful.

@will-fagerstrom
Copy link

I'd like to add an additional use case for being able to disable hoisting in workspaces. I recently had to abandon workspaces in favour of Lerna to resolve this for myself.

I am working on a very large project which has quite a few serevrless components to it. We have a bunch of queueing and lambada workers. The project is setup with the following folder structure

/applications/admin-application
/applications/queue-system/
/applications/queue-system/lambda-1
/applications/queue-system/lambda-2
/applications/queue-system/lambda-etc
/applications/queue-system/cdk-infrastructure
/packages/shared-code-packages

each of those folders is a npm package which maintains its own set of dependencies. Some shared some not, some packages that only exist in this project and wont be published to any registry.

Part of publishing an aws lambda is the package needs to contain all of the dependencies in a package (ignoring layers for now). What we want to be able to do is, after an install be able to zip up each of the lambdas with all and only their dependencies so the cdk deployment can pick that up and push it out too aws.

With hoisting everything is in the root which is many millions of unneeded files for each of the lambdas. Not only that, you must handle the folder structure of walking up the tree and pulling everything down to the lambda folder or you end up with a deeply nested folder structure in the lambda.

I was very close to creating some sort of symlink system of constructing the node_modules folder in each of the lambdas at deploy time, then zipping that up. But Lerna more or less just works

@khitrenovich
Copy link

khitrenovich commented Nov 1, 2021

Our use case is very similar to what @will-fagerstrom just described, with a twist of Lambda Layers.

We pack all 3rd party deps to the shared layer (as described here) and nohost all internal dependencies to the lambdas themselves. Currently it works perfectly with Yarn 1. Our workspaces config looks similar to the following:

  "workspaces": {
    "packages": [
      "api/*",
      "services/*",
      "client/*"
    ],
    "nohoist": [
      "@octobat/bat-services-lambda-layer/**",
      "@octobat/bat-services-*/@octobat/*"
    ]
  },

I would rather go back to NPM than to Yarn 2+.

@isaacs
Copy link
Contributor

isaacs commented Nov 5, 2021

If and when something like this lands, #375 is probably going to be the first step. If you have a moment, please go weigh in there about how this will affect your use case, if it will satisfy your needs, etc.

I will update that PR now with my notes from our discussion in the open RFC call a few days ago.

@isaacs
Copy link
Contributor

isaacs commented Nov 5, 2021

#375 (comment)

@khitrenovich
Copy link

@isaacs -

Thank you for pointing out the proposal in #375.
Unfortunately, I don't think it will help in the case I described above.

As you've mentioned in the proposal,

  1. The wording of "nohoist" is overly specific to a particular
    implementation. That is, the declaration describes the "what" of
    package folder tree implementation, instead of the "why" of dependency
    graph resolution semantics.

This is exactly the way we use nohost option in Yarn. It has nothing to do with the version resolution per se. Instead, we are dealing purely with the deduplication aspect of hoisting. The primary reason for that is the need to package individual parts of the project tree to be used outside - either individually (AWS Lambdas, as described by @will-fagerstrom) or in conjunction with other dependency resolution mechanisms (AWS Lambda + AWS Lambda Layers use case above).

@isaacs
Copy link
Contributor

isaacs commented Nov 5, 2021

@khitrenovich So, iiuc, you're relying on all dependencies being local to the workspace's node_modules folder so that you can npm pack that folder (or zip/tgz/git archive, whatever) and get all your deps?

I think #375 will still satisfy that need, no? Basically, "nohoist" will be the default, unless you have a peer dependency, or a dependency on a sibling workspace. If we implement it by symlinking sibling workspaces into one another when they have a dependency on each other, and they are listed as bundleDependencies, and you're bundling the workspace up with npm pack, then it seems like it'll do the right thing.

@khitrenovich
Copy link

@khitrenovich So, iiuc, you're relying on all dependencies being local to the workspace's node_modules folder so that you can npm pack that folder (or zip/tgz/git archive, whatever) and get all your deps?

Not exactly.

We are developing a serverless system, with backend implemented mostly with AWS Lambdas. Each packaged lambda is supposed to be self-contained (that is, contains both its own code and the dependencies). Yet, it is possible to compose a single lambda from several so-called "layers" and it is possible to share those layers between several lambdas. (more)

Here is a sample of our current workspaces configuration:

  "workspaces": {
    "packages": [
      "api/*",
      "services/*",
      "client/*"
    ],
    "nohoist": [
      "@octobat/bat-services-lambda-layer/**",
      "@octobat/bat-services-*/@octobat/*"
    ]
  },

Our lambdas (@octobat/bat-services-* workspaces above) rely both on internal dependencies (sibling workspaces under the same root project) and on external (3rd party) dependencies. The external dependencies are typically the same for most of the lambdas and rarely change, so we pack them all in a shared lambda layer (@octobat/bat-services-lambda-layer) in order to speed up the deployment. Those dependencies should be hoisted up for all the workspaces except for the lambda layer workspace itself (see the first line on the nohist config). The rest of the dependencies (that is, internal dependencies) appear in the lambda package directly for the ease of development/debugging. We cannot hoist them, otherwise we cannot properly pack them into the workspace artifact (see the second line on the nohist config). Obviously, they are symlink'ed and the packaging tool follows symlinks to obtain the actual content.

Besides the lambda-based backend services the project contains API definitions, client-side workspaces etc. - all those are not affected by nohost configuration and currently benefit from the default hoisting mechanism.

Maybe I'm misinterpreting what #375 is about, but I don't see how it will be possible to configure the same dependency of the same version to be hoisted in one workspace and not hoisted in another workspace.

@isaacs
Copy link
Contributor

isaacs commented Nov 5, 2021

Maybe I'm misinterpreting what #375 is about, but I don't see how it will be possible to configure the same dependency of the same version to be hoisted in one workspace and not hoisted in another workspace.

Well, no, effectively nothing would be hoisted at all, except explicitly declared root dependencies. Any non-peer/non-sibling dependencies of the workspace itself would not be hoisted beyond the workspace boundary. For sibling workspace dependencies, we could create a symlink in the dependent workspace's node_modules folder, so that npm pack would follow the symlink if it's in bundleDependencies.

@souzaramon
Copy link

Same problem here, i'll just move to yarn again

@vimtor
Copy link

vimtor commented Dec 14, 2021

I am having this problem for Next.js

@bd82
Copy link

bd82 commented Dec 23, 2021

Same problem here, i'll just move to yarn again

@souzaramon might want to give PNPM a try, as far as I understand it never hoists anything because it utilizes links so there is no duplication...

@KayBeSee
Copy link

KayBeSee commented Jan 3, 2022

I'd like to +1 the nohoist option.

I maintain a monorepo that has an electron app and web app that serves the same frontend and shares backend logic but when I try to distribute the electron app, I get an error because electron is hoisted.

Relevant discussion: electron-userland/electron-builder#3984 (comment)

@garyo
Copy link

garyo commented Jan 20, 2022

Same here -- need this for deploying Google Cloud Functions which always use npm for the cloud build even though I use yarn locally. I currently can't deploy cloud functions with Node 16 because npm rejects the workspaces.nohoist section in my package.json.

@rfearing
Copy link

Hi there, I have been struggling with npm-workspace. I found a tricky workaround for this no-hoist-like behavior with npm@8:
$ npm i foo@ #=> Install into node_modules/foo
$ npm i foo -w some-workspace #=> Install into node_modules/some-workspace/foo
$ npm un foo #=> Uninstall node_modules/fooCopy and SaveShare

It must not a perfect solution obviously, but npm-workspaces cannot be used without such knowledge.

Thanks. It worked.

Another workaround I found is to use install-strategy = "nested" I created a .npmrc file at the root of my project with

install-strategy = "nested"


Copy and Save

Share
And deleted all node_modules folders + package-lock.json files

find . -type f -name 'package-lock.json' -ls -delete
find . -name 'node_modules' -type d -prune -print -exec rm -rf '{}' +


Copy and Save

Share
And then installed again with npm install. Now all deps are installed in respective folders. It's not ideal because I only needed expo-router not to be hoisted.

Thank you @mrkpatchaa 🙌🏼

@benlesh
Copy link

benlesh commented Sep 20, 2023

This issue is going to single-handedly force me to use Yarn in the RxJS repository when I really don't want to. :\

My issue is that I have a documentation app workspace that is using @types/jasmine, and the actual rxjs workspace which is using @types/mocha. Both types packages define a global xit, describe, etc, and TypeScript freaks out, because all of the above exist in a hoisted node_modules when I install with npm.

Moving to Yarn fixed this cleanly, but I'm a bit sad because all of my muscle memory is npm. And, like most tooling, if I move over to yarn for this project, it's unlikely I'll move back until I have a reason to... because messing with tooling that works is silly and time-consuming.

@1uss1
Copy link

1uss1 commented Oct 10, 2023

+1 for this feature request

@ranwawa
Copy link

ranwawa commented Oct 16, 2023

+1

@trusktr
Copy link

trusktr commented Oct 16, 2023

Today I tried npm workspaces for the first time, then ran into this, and had to abandon the idea.

Without nohoist or similar configurability, npm workspaces cannot be used with projects whose packages use <script type="importmap"> to specify where to get dependencies (in a browser when you run a static server for example), especially when those packages are git submodules that need to be able to function on their own (they can't be having import maps pointing to ../../../ all the way back in the root super module because that doesn't exist when they are cloned on their own).

I had to go with Yarn for managing install and linking. First time using Yarn, and I was greatly impressed.

Why Yarn is so great.

I have always used npm because its the default (Node's built-in corepack changes that nowadays though, allowing yarn usage out of the box!), with Lerna for install+link, but Lerna 7+ no longer installs/links and tells users to use npm, yarn, or pnpm for that now, so I naturally tried to use npm as usual but could not get things working, and now that I've tried yarn I've realized how great it is and I think npm should take some inspiration from it.

I had a good time with Yarn's nodeLinker: node-modules and nmHoistingLimits: workspaces options. This leads to a result that is similar to Lerna's but a lot quicker (20-30 seconds without yarn.lock files, instead of Lerna's 3 to 5 minutes minutes without package-lock.json files), with control over how packages are linked whereas NPM gives no control.

What we really need is workspaces hosting/sharing/whatchamacallit options that enable a git submodule to work on its own exactly the same way as when inside of a super module. Basically what @ljharb said, that the node_modules result is the same as it would be if you ran npm install inside of the package on its own.

I would be ok if node_modules in the sub-packages contain links to the modules in the root, as most static web servers operate fine that way (or have an option to operate that way) even if the link goes outside of the web server's root folder. This would be ok because then the importmap links to the local node_modules folder.

If links to the root would become a feature, the links should be in a structure that mimicks the same structure that would be in place with a regular npm install if the package were on its own.

@simon-abbott
Copy link

simon-abbott commented Oct 16, 2023

This is a must-have for us. We're trying to migrate away from Lerna since it dropped install support in v7 in favor of npm workspaces, but due to our liberal use of patch-package and copying files out of node_modules (legacy code, we're trying to move away from it but haven't been able to yet) it keeps breaking. When I npm install in a workspace I expect it to behave exactly the same as if I manually ran npm install and npm link in each sub-package. The fact that it doesn't is deeply unintuitive, confusing, and (as the ever-growing list of broken projects shows), counterproductive.

If it were up to me, I would stick with Lerna, but unfortunately that ship has sailed, so I guess we're over to yarn until this is fixed. Hopefully that doesn't break too many other things. 😞

EDIT: we decided that switching to yarn was too disruptive, so instead we moved all our patches up to the root where they were actually being installed. Not pretty, but it works.

@joshuat
Copy link

joshuat commented Oct 16, 2023

@trusktr Just want to say thank you for your edit - we're in a similar position and actively exploring options to move away from Lerna. We had hoped to move back to npm, but without hoisting limits of any kind, it's just not an option for a codebase with a significant number of packages.

@subzero10
Copy link

Just dropping in to say that I have an example repository that demonstrates the issue we are having with npm workspaces and hoisting.

@vgjenks
Copy link

vgjenks commented Nov 7, 2023

I am also interested in an option to disable hoisting. We have a monorepo of AWS lambdas. Because each lambda will be deployed individually it must have it's own dependencies (i.e. no shared dependencies). So far I have tried everything I can think of and for one reason or another they are not working. Even install-strategy = "nested" does not work, it still hoists some shared dependencies, i.e. node-fetch.

This is the exact problem I'm having right now with a new monorepo I'm putting together for a new project at work. We're 3 years into this feature request with no progress. Is there any hope? I think npm has a great workspaces implementation, but this is the Achilles heel. Many projects need direct, non-hoisted dependences inside of monorepos, for deployments. You cannot use npm workspaces for this very common, modern scenario. Yarn is a mess. I'm left looking to pnpm, but I wish I didn't have to.

I see a lot of "Well, I'll just use pnpm" here, but I tried that...and correct me if I'm wrong, but it still creates symlinks when you turn off hoisting. This defeats the purpose and doesn't work for Lambda and other types of projects which need to independently bundle the dependency.

What's the status on this? Will it ever happen? Can npm keep up?

@trusktr
Copy link

trusktr commented Nov 10, 2023

What's the status on this? Will it ever happen? Can npm keep up?

If npm can't keep up, there's at least this good news:

Both yarn and pnpm ship out-of-the-box with Node.js nowadays.

After you install Node.js, you can run corepack yarn ... or corepack pnpm ... off the bat! To add those to your PATH so you can avoid having to prefix commands with corepack, run corepack enable and it'll link yarn and pnpm into the folder where other Node exes are.

In case anyone migrating from npm(+Lerna) chooses to go with Yarn, there's one issue that you should be aware of: in some cases Yarn will absolutely not dedupe packages that are within compatible version ranges (even if you manually run yarn dedupe), and you may have to manually create resolutions (Yarn's alternative to npm's overrides) to get rid of in-range duplicates. Here's the tracking issue:

@philly-vanilly
Copy link

philly-vanilly commented Nov 22, 2023

  "workspaces": {
    "nohoist": [
      "@my-org/my-lib"
    ],
    "packages": [
      "apps/*",
      "libs/*"
    ]
  }

works for me. I had some trouble FORCING the installation of some packages to go into root because there were some peer deps that had other versions, so I installed them in root manually. then I deleted them from root package.json (but not package-lock.json) again and moved back to @my-org/my-lib and voila, they are still being installed in root on successive npm i

@maahtiin
Copy link

maahtiin commented Jan 8, 2024

Not fully tested, but if i specify typeRoots: {} with the needed types in my tsconfig.json of the specific package giving the errors, the compilation seems to succeed without errors. Might this be a way to circumvent conflicting @types dependencies?
Couldn't test it fully as i'm still building the project, but will update this post as i progress.

@ranwawa
Copy link

ranwawa commented Jan 8, 2024 via email

Dev-Akashili pushed a commit to Health-Informatics-UoN/rcc-monitor that referenced this issue Jan 15, 2024
Pipeline files for a UAT and production deployment, including:

- .NET Core Webapp
- Nextjs Node App
- .NET Functions App

And new pipeline environments: `NUH RedCap Monitor UAT` + `NUH RedCap Monitor Prod`, checks needed to be added on the production environment by an admin.

Also:
- Updates an incorrect typescript import
- Removes Cors Framework package.
- Removes the npm workspaces for now, as deploying with this is not easy, see: npm/rfcs#287
Hopefully can come back to implementing it with workspaces, but this is the most direct route to urgent deployment.
- Adds a `server.js` file for the app service to run the Next app as, via PM2: https://learn.microsoft.com/en-us/azure/app-service/configure-language-nodejs?pivots=platform-linux#run-with-pm2

Related work items: #125090, #125102
@raisiqueira
Copy link

Hey folks, what's the status on this? Will it ever happen?

@ranwawa
Copy link

ranwawa commented Apr 17, 2024 via email

@sweetpalma
Copy link

sweetpalma commented Aug 12, 2024

I have struggled with this issue for a while and eventually found a sort of workaround for a pretty specific case. This approach creates two separate node_modules folders containing dependencies ONLY for a particular sub-package, and then merges them, allowing you to deploy it - e.g. as a part of your CI pipeline.

Example:

cd my_monorepo_root
rm -rf ./node_modules
npm ci --install-links --ignore-scripts --workspace=azure-functions
cp -R ./node_modules ./packages/azure-functions

@ranwawa
Copy link

ranwawa commented Aug 12, 2024 via email

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

No branches or pull requests