-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Workspaces complain about missing hoisted peer dependencies #4743
Comments
There was a fix in 1.2.1 which may affect the behavior around this: #4687 Would you like to give it a try? |
Aww I was hoping that was it, but i'm still seeing the warnings on 1.2.1 (just checked) |
Same here with 1.2.1. (Linux 4.10.0-37-generic / Node 8.1.4)
"dependencies": {
...
"react": "15.5.4",
"react-redux": "5.0.6"
...
} |
Yes, I can confirm this too with yarn 1.2.1 as well as nightly. With workspaces enabled I get peer dep warnings for the workspace aggregator that should be met by dev deps. And I get warning for all hoisted deps in each workspace. I'll try and look into this issue and see what I can find. |
I'm also seeing this with the following warnings:
The Node 8.2.1, Yarn 1.3.2 (Homebrew), macOS 10.12 |
Same issue here. In addition, it seems like it fails to install one of the modules that it's complaining about. |
Experiencing the same issue..
Where
but complains with |
Some of these issues seem to be fixed, but some still exist. For example the case from @likeavirgil no longer throw a warning in 1.3.2.:
Here is a minimal example of one that still doesn't work in 1.3.2
This seems to be because the code is checking the un-hoisted dependency tree to see if a peerDep is resolved up the trunk, but it fails to check the actual root |
I was able to fix it after some guessing... I'm new to yarn but fixed 4 warnings including the one here.
I'll list my fixes individually so you can see the pattern:
Be sure to use the appropriate dependency flags referenced in the warning, more info here: After I ran those lines... I got what I spent hours trying to achieve... 0 warnings 😄 :
|
@growthcode But the point is you shouldn't need to do that. |
I'm currently working on a fix for this, but can't assign it to myself. Is anyone able to grant me access to do so? |
@rally25rs How's this fix going? :) |
@johnnycopperstone ⬆️ (I can lend a hand with this if you haven't made much progress, just let me know.) |
I'm happy to give some pointers, I tried debugging this and hit a wall. The problem is definitively related to either how yarn builds the tree on a workspace or how the validator works against a workspace tree. For a simple example of a peerDep not being satisfied by a devDep I see the devDep added to the workspace tree fine, but then when the validator runs it finds that the dep is too far removed from the one with the peer declaration and so it warns (as it should). I don't know enough about how yarn structures the tree and I haven't had time to delve deeper. :( |
Alright, after much trial and error, I finally came up with a unit test that reproduces this. It calls for a dependency structure like:
so the peer dep ( I suspect that when Yarn goes to check for the peerDep, it finds it at But I have a failing unit test, so that's step 1... 😃 |
…pkg#4743 Added a failing unit test to reproduce issue yarnpkg#4743. It seems that if a peerDep exists deeper in the dep tree than where it is included, yarn will output a earning, even if that peerDep is satisfied by the same library included shallower in the tree, or at the root level.
…multiple levels. A missing peerDep warning was being issued if the exact same pattern was a deep transitive dep and a direct dep. This was due to only the first request for a pattern being added to the list of requests that peerDep was checking. Now all references are tracked. Also fixed a logic error in Warn where a dep would be reported multiple times. yarnpkg#4743
@johnnycopperstone I know you were working on this, but I went ahead and put together a PR ⬆️ If you could try it out and verify that it works, that would be great! |
Adding a reference to this issue: #4503 since it seems related. Generally hoisting seems to cause problems w/ both peer dependencies as well as a host of other tooling that expects scripts, types, etc. to be in the individual workspaces. |
Let's keep these issues separate.
1. Yes, hoisting should work as advertised. This includes NOT giving
spurious errors/warnings about unmet peer dependencies.
2. Scripts--assuming you mean things going into `.bin`--are already always
placed where expected. I am not aware of any bugs raised against this. No,
adding packages at the top level should **not** add their scripts to
sub-workspaces. That violates basic rules of expectations.
3. "Types"--assuming you mean things like `*.d.ts` files--pretty much work
as they should in the default case. In other words, TypeScript will consume
types from all `@type` directories up the npm search path, so it doesn't
matter if `@types/*` is hoisted. However, there are some problems when
types are being explicitly called for in the `typeRoots` property of
`tsconfig.json`, which may require doing things like `[../@types',
'../../@types`]` and so on. This is an issue which needs to be addressed by
TS, unless there's some work-around that I'm not aware of.
4. Packages which assume that they live in a particular place relative to
something else are poorly designed and should be fixed. Any kind of
hoisting whitelists or hoisting blacklists are merely bandaids for this
kind of poor behavior. For instance, at least in the past some Angular
tools insisted that some Angular packages be in the very directory they
were being run from. This violates the basic notion of npm search paths and
should be dealt with by fixing the tool.
Bob
…On Fri, Dec 15, 2017 at 4:08 AM, Brandon Ramirez ***@***.***> wrote:
Adding a reference to this issue: #4503
<#4503> since it seems related.
Generally hoisting seems to cause problems w/ both peer dependencies as
well as a host of other tooling that expects scripts, types, etc. to be in
the individual workspaces.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#4743 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABfR00VacEjEB5b8hNbyh188vZqvYGdks5tAaN5gaJpZM4P_aZh>
.
|
Awesome @rally25rs. I didn't get very far, wasn't sure how I could compare the patterns for each dependency with the actual hoisted tree (given it's hoisted as a flat tree with paths as far as I understand). I'll take a look at the PR and test this on our code base today. |
…t root level. (#5088) * test(resolution): Added a (failing) unit test to reproduce issue #4743 Added a failing unit test to reproduce issue #4743. It seems that if a peerDep exists deeper in the dep tree than where it is included, yarn will output a earning, even if that peerDep is satisfied by the same library included shallower in the tree, or at the root level. * fix(resolution): No longer warn for mising peerDep when it exists at multiple levels. A missing peerDep warning was being issued if the exact same pattern was a deep transitive dep and a direct dep. This was due to only the first request for a pattern being added to the list of requests that peerDep was checking. Now all references are tracked. Also fixed a logic error in Warn where a dep would be reported multiple times. #4743 * fix(tests): Remove snapshot test of "yarn why" due to different output local vs ci. Now assert with Remove snapshot test of "yarn why" due to different output local vs ci. Now assert with actual object value comparison.
I encoutered this issue using Yarn 1.3.2. The probleme was resolve when I upgrade to the last current version of Yarn (1.5.1) |
I'm encountering this on yarn 1.12.1 :( |
Same for yarn 1.15.2! |
Same for yarn 1.16.0! |
Annnd same for 1.17.3. |
I'm encountering this on 1.22.4 |
Same for yarn |
What is the current behavior?
peer deps that are hoisted to the repo root are seen as missing by each workspace
If the current behavior is a bug, please provide the steps to reproduce.
Add two workspaces with a Peer dependency and dev dependency on
react
(or anything in npm). Runningyarn
results in "[Workspaces package] has unmet peer dependency" warningYou can try it out with: https://github.com/jquense/react-widgets and would see complaints about react, react-dom, moment, and globalize being missing.
What is the expected behavior?
It not to warn
Please mention your node.js, yarn and operating system version.
Yarn 1.2.0, node 8.5 and Mac Sierra
The text was updated successfully, but these errors were encountered: