Yarn workspaces hoisting deep dependencies instead of direct dependencies

[lostpebble]

Do you want to request a feature or report a bug?

bug, I think.

What is the current behavior?

At the moment when using Yarn workspaces and we have multiple workspace packages which require the exact same dependency on something in their package.json (example date-fns@2.0.0-alpha.7) - but we also have other dependencies in those workspace package.jsons, such as webpack-cli or concurrently, which themselves require (a different version) of that dependency date-fns@1.29.0 (a version we don't want to actually use in our direct project) - the behaviour seems to be that those deep package dependency versions are the ones being hoisted instead of my direct dependency versions.

So now when I run yarn why date-fns I get this response:

yarn why v1.6.0
[1/4] Why do we have the module "date-fns"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "date-fns@1.29.0"
info Has been hoisted to "date-fns"
info Reasons this module exists
   - "workspace-aggregator-80181383-68fe-4e24-ade4-e23892f06bf6" depends on it
   - Hoisted from "_project_#vs-worker#concurrently#date-fns"
   - Hoisted from "_project_#vs-frontend#webpack-cli#listr#listr-verbose-renderer#date-fns"
=> Found "vs-worker#date-fns@2.0.0-alpha.7"
info This module exists because "_project_#vs-worker" depends on it.
=> Found "vs-frontend#date-fns@2.0.0-alpha.7"
info This module exists because "_project_#vs-frontend" depends on it.
Done in 3.84s.

This causes all kinds of headaches when using things like babel and other build tools which look in the root node_modules folder for our code dependencies. For all intents and purposes - we have specifically said to use date-fns@2.0.0-alpha.7 in our package.json, but now because of the strange hoisting we are getting date-fns@1.29.0 instead.

Actually the biggest headache is when another dependency in your package.json actually needs that date-fns@2.0.0-alpha.7 dependency as a peer dependency, but it got hoisted to the root, and now when node tries to resolve that dependency from inside that module at workspaces-project/node_modules/xyz-module/code-with-require-date-fns.js, it will only resolve in that same root /node_modules folder and obviously not find the correct version which we explicitly set so it could function correctly.

What is the expected behavior?

I would expect my direct project / workspace dependencies to over-rule those which are deeply nested in other node_modules dependencies. I see no reason as to why the deeply nested dependencies would ever "win" over those.

Please mention your node.js, yarn and operating system version.

Yarn v1.6.0 Node 9.11.1 Windows 10

[rally25rs]

Normally yarn will take whichever version has the most references to it and make it the hoisted top-level version, because that would result in the least duplication (least amount of wasted drive space)

In this case there are 2 dependencies on date-fns@2.0.0-alpha.7 and 2 dependencies on date-fns@1.29.0, so it is sort of a toss-up as to which it hoists. I'd not familiar enough with the logic to know if it just hoists the first one it finds or what.

If you think about it from a "now would node resolve dependencies" standpoint, then it should function correctly either way. You end up with the structure:

node_modules
  |- date-fns@1.29.0
  |- vs-worker
  |    +- node_modules
  |        +- date-fns@2.0.0-alpha.7
  |- vs-frontend
  |    +- node_modules
  |        +- date-fns@2.0.0-alpha.7
  |- concurrently
  +- listr-verbose-renderer

If the package vs-worker has a require('date-fns') statement in it, then node.js would first look in node_modules/vs-worker/node_modules/date-fns which would correctly have @2.0.0-alpha.7

Node would only move up to the root level if the dependency was missing in vs-worker/node_modules, so in the case of concurrently it would first look for node_modules/concurrently/node_modules/date-fns and when that doesn't exist, move up a directory and resolve node_modules/date-fns@1.29.0 which is correct for it's dependency.

Your application shouldn't depend on the hoisted position of libraries, because hoisting location is always subject to change. I suspect your problem is a result of your webpack build not resolving deps like node would, or being configured to always look in the root node_modules first.

However what you might want to do in your case is try using resolutions to make it so that @2.0.0-alpha.7 is the only versions (force the other packages to use @2.0.0-alpha.7 instead of 1.29.0). Although if those libraries don't work correctly on v2 of date-fns then that could cause problems as well.

[lostpebble]

Actually the main problem comes with using Node.js, not webpack which as you say can be relatively easily configured to first look at our workspace node_modules instead of the root one.

Node.js seems to resolve dependencies relative to the node_modules of wherever the module is found.

This is the current crux of the problem, imagine this scenario (which was how I ran into the issue first):

vs-frontend has defined a dependency material-ui-pickers in its package.json. But material-ui-pickers has a peer dependency of date-fns@2.0.0-alpha.7 - which must be that version for some new and specific functionality - so we explicitly add that version of date-fns to our vs-frontend package.json too.

When we install and run we would now expect things to work correctly - as all is well defined in package.json.

But vs-frontend also has another dependency of webpack-cli defined which depends on date-fns@1.29.0 - now usually we would expect this sub-dependency to actually not cause an issue, because it is just that, a dependency of a dependency and would go into its isolated section of node_modules/webpack-cli/node_modules/date-fns when there is a clash with the defined date-fns in package.json.

And instead we now get this problematic folder structure:

node_modules
  |- date-fns@1.29.0
  |- vs-worker
  |    +- node_modules
  |        +- date-fns@2.0.0-alpha.7
  |- vs-frontend
  |    +- node_modules
  |        +- date-fns@2.0.0-alpha.7
  |- concurrently
  |- material-ui-pickers
  +- listr-verbose-renderer

material-ui-pickers is hoisted because it can be according to the rules. But it now exists at a depth which is not aligned with its required version of date-fns.

So when the code inside of material-ui-pickers gets called in a regular Node.js context, such as this:

var _addDays = require('date-fns/addDays');

It's looking in its node_modules context. And its finding the old version and throwing this error:

Error: Cannot find module 'date-fns/addDays'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:548:15)
    at Function.Module._load (internal/modules/cjs/loader.js:475:25)
    at Module.require (internal/modules/cjs/loader.js:598:17)
    at require (internal/modules/cjs/helpers.js:11:18)
    at Object.<anonymous> (D:\Dev\_Projects\xx\xx\node_modules\material-ui-pickers\utils\date-fns-utils.js:19:16)
    at Module._compile (internal/modules/cjs/loader.js:654:30)

date-fns/addDays is resolvable on the new alpha version of date-fns.

Because of the Node.js context, it can't know where else to resolve modules except exactly where it is and then in parent folders. It will not know to look in the symlinked node_modules/vs-frontend/node_modules.

Unless I'm missing something and there is some other way to tell Node.js where else it should look to resolve modules, that way I could tell it to always search from workspaces/vs-frontend/node_modules, and it can find its way up to root workspaces/node_modules as it needs as well. As it seems now, once Node.js has found a module in node_modules, it will be using that modules folder as its context for any dependencies within that module.

Just realising that is probably a fundamental way of how Node.js actually works. It would be silly to tell Node.js where to look exactly for dependencies, because of what I described above of how a dependency of a dependency can sometimes be nested inside an actual module when there are version clashes - therefore, the context always has to be that exact module when resolve modules in Node.js.

[rally25rs]

Ah! Thanks for the additional details. This actually seems like a problem with material-ui-pickers because it's code import date-fns but it actually doesn't list it as a dependency or as a peerDependency at all https://github.com/dmtrKovalenko/material-ui-pickers/blob/master/lib/package.json#L34-L50 They do have it listed as an externalDependency but I have no clue what that is. It doesn't seem to be a documented package.json field according to npm docs.

If they actually listed it as a dep then it should end up in the correct place after being yarn installed. It looks like they just assume that you will have that version installed without them listing it as a dep or peerDep.

🤔 I'm not sure what the best way would be to solve that. I would be hesitant to change the hoisting behavior of yarn for a package doing things a non-standard way. I've never used it myself, but I think the nohoist feature might be useful here. I think you should be able to mark date-fns@1.29.0 as a package that shouldn't be hoisted.

[lostpebble]

Ah, okay I see. So are you saying that if they defined it correctly in peerDependencies we would not run into this issue in workspaces?

I'll follow up on that repo as it does seem from what I can find that externalDependencies was an idea that never got traction: https://github.com/npm/npm/issues/14529

[lostpebble]

Just looking at documentation, could this kind of thing not be solved with https://docs.npmjs.com/files/package.json#optionaldependencies ?

The material-ui-pickers library author has correctly stated that peerDependencies doesn't really work for his context. Basically you have a choice of three libraries to use: date-fns, moment or luxon. If he put all three in peerDependencies - yarn and npm will see them all as required and throw warnings if they are not installed.

See: https://github.com/dmtrKovalenko/material-ui-pickers/issues/369#issuecomment-383280181

[lostpebble]

I must say that through all this - if yarn just respected and gave hoist preference to directly defined dependencies in package.json above those of sub-module dependencies, it would solve a lot of issues. That way we can always deliberately set things in our package.json and get expected results - right now its a bit of a toss-up as to what is going to be hoisted inside the rigmarole of dependencies on dependencies.

Those sub-modules and their differing dependencies can be hoisted too, but would have the proper structure of workspace-root/node_modules/webpack-cli/node_modules/date-fns, for example- therefore not clashing with anything deliberately set in your project. This seems like it should be default behaviour.

[rally25rs]

If the "optional" deps were listed as optionalDependencies then yarn would install all 3 of them. That section basically means "install all these but if it doesn't pass engine or platform checks, then skip them"

if yarn just respected and gave hoist preference to directly defined dependencies in package.json above those of sub-module dependencies, it would solve a lot of issues.

It would solve this particular issue of a package looking for a package that it doesn't depend on, but I haven't seen widespread issues from the current hoisting scheme. There shouldn't be any issues when packages have dependencies properly set up that yarn can detect.

Is there a reason that nohoist won't work for you in this situation?

@yarnpkg/core what do you all think? Does this seem like a reasonable change to the workspace hoisting?

[lostpebble]

Is there a reason that nohoist won't work for you in this situation?

It will work I'm sure (my current way to get around this issue is just defining date-fns@2.0.0-alpha.7 in my workspaces root package.json). I'm just bringing this up still because it is an unexpected side-effect of the current hoisting process. I'm very sure I've run into similar problems before because of packages not picking the right version because of what's been hoisted.

I think this change to the hoisting behaviour would be in line with the default behaviour of yarn / npm, as it has always been. How I think it should work:

1. Yarn should look at our package.json's of all workspace packages and determine which dependencies should be hoisted from that.

2. In this case date-fns@2.0.0-alpha.7, material-ui-pickers@x and webpack-cli@x would all be hoisted.

3. Then at the root level it should treat all those dependencies as a regular project would - if there is a clash between modules, those that are direct dependencies of the project always take preference (date-fns@2.0.0-alpha.7 in this case)

4. The rest are placed directly in whichever module depends on them, like: workspace-root/node_modules/webpack-cli/node_modules/date-fns.

It just seems really logical IMO. And shouldn't actually break current projects using workspaces - unless of course they've already been unknowingly using modules of a version they didn't intend to because it was hoisted by a sub-dependency.

[lostpebble]

I've just run into another issue with this, this time with some @types packages:

yarn why @types/react
yarn why v1.6.0
[1/4] Why do we have the module "@types/react"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "@types/react@16.3.10"
info Has been hoisted to "@types/react"
info Reasons this module exists
   - "workspace-aggregator-4de3c119-a074-45d6-a75a-4f14b6f71285" depends on it
   - Hoisted from "_project_#@gt#gt-quick-react#@types#react-router#@types#react"
   - Hoisted from "_project_#@gt#gt-quick-react#@types#react-dom#@types#react"
   - Hoisted from "_project_#vs-frontend#material-ui#@types#react-transition-group#@types#react"
=> Found "@gt/gt-quick-react#@types/react@16.3.12"
info This module exists because "_project_#@gt#gt-quick-react" depends on it.
Done in 1.47s.

As you can see I've set a direct dependency of @types/react@16.3.12 in my gt-quick-react workspace module project. But because of some deep sub-dependencies in @types/react-router and @types/react-dom and an even deeper one in material-ui, the incorrect version for @types/react is being hoisted into my root workspaces node_modules.

I understand the hoisting thing is about saving space - so it seems to be trying to find the version with the most duplicates and hoisting those. But I think that providing expected structure of modules definitely trumps a few MBs here and there.

[connectdotz]

@lostpebble it seems there are 2 separate issues discussed here:

1. for your initial problem, "material-ui-pickers can't find the correct date-fns version": if a package didn't specify the correct dependencies via supported package.json scheme, it should have no expectation that the package manager(s) would behave consistently. This is not PM or hoisting issue, you are doing the right thing to follow up with material-ui-pickers directly.

2. You then reason maybe yarn should adopt different conflict-version resolution algorithm. It is true that the one you proposed does make sense, but so are other algorithms like the one used currently: hoist the most popular one; I am sure there are plenty more we haven't even thought about... It is not clear to me any one algorithm is better than the others, for all use cases. The point is yarn needs 1 (anyone from the pool above is just fine) predictable algorithm to break tie for conflict versions, and it has one now; in other words, which algorithm it selects should NOT matter to the project/package, if they are configured correctly. The real question here is not which algorithm should be used, but why your project is sensitive to this internal algorithm? (hint: see point 1)

To conclude, I think you probably have to live with the workarounds until material-ui-pickers get fixed.

[lostpebble]

@connectdotz I understand what you're getting at. And generally these kinds of things can be fixed with extra configuration. I am really struggling with some things every now and then though where this causes issues, where for the life of me I can't get it to work unless I force a certain version either with resolutions or defining it in the root package.json. It just seems like it shouldn't be an issue in the first place and the current behaviour seems unexpected.

the one used currently: hoist the most popular one

This is the crux of the problem really. I'm not sure ignoring how a regular non-workspace project handles installing of dependencies simply because a module version is seemingly more "popular" than others is the best choice here. In regular projects, versions of dependencies defined in package.json always override the "popularity" of other module versions, because we've directly stated that it should be the version we want to use for our project code.

I would like to see this discussed a bit more, but if you have decided it's not worth changing then I can accept that too. Just that it really has caused me lots of headaches in my use of workspaces so far, and I would like the pros and cons to of the situation to be thought of a bit more before dismissal.

It is not clear to me any one algorithm is better than the others, for all use cases.

But we can probably find one that is objectively better than the others, and would serve the most use cases. I'm just trying to bring to light that the current one might not actually be that one.

[arcanis]

As @connectdotz mentionned, the hoisting is something you shouldn't rely on. It's not a question on which algorithm we should use - we actively do not want to specify one, so that we have more latitude to update it every now and then.

If your workspace root depends on @types/react@16.3.12, you should mark it as such in your workspace root's package.json. Otherwise it will eventually break, because it's just an invalid construct (which happens to incidentally work in some cases).

[connectdotz]

@lostpebble I think I see where you got stuck: you assumed that yarn should pick the "right" one to hoist, where in theory there really shouldn't be any wrong one to pick...

Let's look at a simple theorem: you have a workspace A and B, A has dependency C@1.0, B has C@2.0. yarn has 3 choices for hoisting 0 or 1 package:

1. hoist C@1.0: A finds C@1.0 in root, B finds C@2.0 locally. checked.
2. host C@2.0: A finds C@1.0 locally, B finds C@2.0 in root. checked.
3. hoist nothing: A finds @C1.0 loaclly, B finds C@2.0 locally, checked.

This tells us that no matter which package we pick for hoisting, they should all work, as far as finding the right version goes. Therefore, if your project depends on yarn to pick a certain version for it to work, it is a highly likely that something in your dependency tree is not right.

Let's examine your use case again:

• if yarn knows material-ui-pickers needs date-fns@2.0.0-alpha.7, and it decided to hoist date-fns@1.29.0, then it would have created date-fns@2.0.0-alpha.7 under material-ui-pickers, and everything will work just fine under the current algorithm, wouldn't it?

• On the other hand, imagine the use case where your vs-frontend workspace has an actual dependency on date-fns@1.29.0, so it specifies that version in its package.json. Under your algorithm, date-fns@1.29.0 will then be hoisted, right? In such case, material-ui-pickers will still break.

As @arcanis pointed out above, dependancies should always be specified at where it is actually used. I hope it is clear now that when material-ui-pickers did not correctly specify its date-fns dependency, there is no algorithm can guarentee its integrity. Contrarily, if it is correctly specified, any algorithm should work. The current alogoritm just happens to be a pretty reasonable one as it is simple to implement, picks the version that could give us better load performance and smaller footprint.

[jackyef]

Normally yarn will take whichever version has the most references to it and make it the hoisted top-level version, because that would result in the least duplication (least amount of wasted drive space)

@rally25rs Hi, could you explain about what counts as a reference by yarn? Specifically, I would like to know if peerDependencies and devDependencies (or other type of deps) of a external module count as a reference as well?

I created a simple repo to reproduce this, and it seems like references from external modules is counted as well. Is this expected? In the repo, you can see that it causes different pair of react and react-dom is hoisted to the root.

This could cause issues during a server runtime because of the mismatched version. Let's say another module @apollo/react-hooks is hoisted to the root as well, and serviceB has codes that is requiring react and @apollo/react-hooks. The code from serviceB will get react from serviceB/node_modules/react but the code from @apollo/react-hooks will get react from node_modules/react.

Update: I created a separate issue for this #7572

[rally25rs]

@jackyef I beleive dependencies and devDependencies would count as "references" but not peerDependencies.

[jackyef]

@rally25rs I see, so peerDependencies aren't counted.

Is it expected that devDependencies of an external module be counted as reference as well? I have always assumed that devDependencies of any external module wouldn't be installed when we do yarn install, why would they count as reference then?

[roni-frantchi]

Shouldn't it be possible to nohoist a package only for Service X, making the "right" version still hoist?
I tried it, but it didn't work for me... the package was never hoisted...