Yarn workspaces hoisting deep dependencies instead of direct dependencies #5705
Comments
ghost
assigned 
lostpebble
changed the title
|
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 [email protected] and 2 dependencies on [email protected], 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: If the package Node would only move up to the root level if the dependency was missing in 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 |
|
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.js seems to resolve dependencies relative to the This is the current crux of the problem, imagine this scenario (which was how I ran into the issue first):
When we install and run we would now expect things to work correctly - as all is well defined in But And instead we now get this problematic folder structure:
So when the code inside of It's looking in its
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 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 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. |
|
Ah! Thanks for the additional details. This actually seems like a problem with 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 |
|
Ah, okay I see. So are you saying that if they defined it correctly in I'll follow up on that repo as it does seem from what I can find that |
|
Just looking at documentation, could this kind of thing not be solved with https://docs.npmjs.com/files/package.json#optionaldependencies ? The |
|
I must say that through all this - if yarn just respected and gave hoist preference to directly defined dependencies in Those sub-modules and their differing dependencies can be hoisted too, but would have the proper structure of |
|
If the "optional" deps were listed as
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 @yarnpkg/core what do you all think? Does this seem like a reasonable change to the workspace hoisting? |
It will work I'm sure (my current way to get around this issue is just defining 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:
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. |
|
I've just run into another issue with this, this time with some As you can see I've set a direct dependency of 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. |
|
@lostpebble it seems there are 2 separate issues discussed here:
To conclude, I think you probably have to live with the workarounds until material-ui-pickers get fixed. |
|
@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
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 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.
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. |
|
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 |
|
@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 [email protected], B has [email protected]. yarn has 3 choices for hoisting 0 or 1 package:
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:
As @arcanis pointed out above, dependancies should always be specified at where it is actually used. I hope it is clear now that when |
@rally25rs Hi, could you explain about what counts as a 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 This could cause issues during a server runtime because of the mismatched version. Let's say another module Update: I created a separate issue for this #7572 |
|
@jackyef I beleive |
|
@rally25rs I see, so peerDependencies aren't counted. Is it expected that |
|
Shouldn't it be possible to |
…f needed
It's known that using workspaces usually produces a slightly different
`node_modules/`[1]. While this used to be no big deal for us, we actually
triggered a case where "dependency hoisting" — the process of moving
dependencies of workspaces up in the final `node_modules/`-tree —
becomes a problem, basically issue 5705[2].
The problem was triggered by the following - simplified - dependency
tree:
<root>
|- commander (6.1)
|- mocha (3)
| `- commander (2.9)
`- sql-generate (1.5)
`- commander (2.9)
`yarn2nix` created a workspace of the package `root` with
`node_modules/` containing `mocha`/`sql-generate`. In the `yarn
install` operation these are all placed into a single `node_modules/` in
`$out`, however `commander` from 2.9 is preferred over 6.1:
$ yarn why commander
[...]
=> Found "[email protected]"
info Has been hoisted to "commander"
info Reasons this module exists
- "workspace-aggregator-dc20c757-df45-41d7-bdba-927d267212a3" depends on it
- Hoisted from "_project_#root#mocha#commander"
- Hoisted from "_project_#root#sql-generate#commander"
=> Found "root#[email protected]"
info This module exists because "_project_#root" depends on it.
Even specifying `nohoist` for both dependencies inside the temporary
`package.json` that's built by `yarn2nix` doesn't solve the problem
since `yarn` still doesn't put the newer `[email protected]` into
`$out/node_modules/`, I don't really know why, could be a `yarn`-bug
though.
Anyways, we're not even using the workspace feature and since it's known
to produce different results, it's IMHO easier to just disable the
entire feature if no `workspaces` are defined in `package.json`.
[1] https://classic.yarnpkg.com/lang/en/docs/workspaces/#toc-limitations-caveats
[2] yarnpkg/yarn#5705
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[email protected]) - but we also have other dependencies in those workspacepackage.jsons, such aswebpack-cliorconcurrently, which themselves require (a different version) of that dependency[email protected](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-fnsI get this response:This causes all kinds of headaches when using things like babel and other build tools which look in the root
node_modulesfolder for our code dependencies. For all intents and purposes - we have specifically said to use[email protected]in ourpackage.json, but now because of the strange hoisting we are getting[email protected]instead.Actually the biggest headache is when another dependency in your
package.jsonactually needs that[email protected]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 atworkspaces-project/node_modules/xyz-module/code-with-require-date-fns.js, it will only resolve in that same root/node_modulesfolder 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_modulesdependencies. 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
The text was updated successfully, but these errors were encountered: