[Bug] Wrong dependencies when using the node_modules linker

[nicolo-ribaudo]

• I'd be willing to implement a fix (I am investigating)

Describe the bug

yarn install installs a lot of packages in workspaces' node_modules which shouldn't be there.

To Reproduce

https://github.com/nicolo-ribaudo/babel/tree/yarn-2-compat

Run yarn install. Then, check codemods/babel-plugin-codemod-object-assign-to-object-spread/node_modules.

It contians these dependencies

@babel/code-frame (symlink)
@babel/helper-plugin-utils
@babel/helper-transform-fixture-test-runner (symlink)
@babel/parser (symlink)
@babel/plugin-codemod-object-assign-to-object-spread (symlink)
@babel/plugin-syntax-object-rest-spread
@babel/template (symlink)
@babel/traverse (symlink)
@istanbuljs/load-nyc-config
@jest/console
@jest/core
@jest/reporters
@jest/transform
@jest/types
acorn-globals
acorn-walk
anymatch
arr-diff
arr-union
babel-plugin-istanbul
browser-resolve
cliui
convert-source-map
cross-spawn
define-property
execa
expect
extend-shallow
find-up
gauge
get-caller-file
is-accessor-descriptor
is-data-descriptor
is-descriptor
is-extendable
is-fullwidth-code-point
is-number
is-stream
istanbul-lib-instrument
istanbul-lib-report
jest-changed-files
jest-cli
jest-config
jest-diff
jest-each
jest-haste-map
jest-jasmine2
jest-matcher-utils
jest-message-util
jest-pnp-resolver
jest-resolve
jest-runner
jest-runtime
jest-snapshot
jest-util
jest-validate
jest-watcher
jest-worker
jsdom
kind-of
locate-path
make-dir
mixin-deep
npm-run-path
path-exists
path-key
p-finally
pkg-dir
p-limit
p-locate
pretty-format
p-try
punycode
readable-stream
regex-not
request
request-promise-core
request-promise-native
require-main-filename
resolve
resolve-cwd
resolve-from
rimraf
safe-buffer
sane
shebang-command
shebang-regex
snapdragon-node
split-string
string_decoder
string-length
string-width
strip-bom
supports-hyperlinks
to-regex
to-regex-range
tough-cookie
tr46
uri-js
which-module
wrap-ansi
y18n
yargs
yargs-parser

This is the package.json file of that workspace:

  "dependencies": {
    "@babel/plugin-syntax-object-rest-spread": "^7.7.4"
  },
  "peerDependencies": {
    "@babel/core": "^7.0.0-0"
  },
  "devDependencies": {
    "@babel/core": "workspace:^7.7.4",
    "@babel/helper-plugin-test-runner": "workspace:^7.7.4"
  }

I would expect node_modules to contain at most two symlinks (@babel/core and @babel/helper-plugin-test-runner), @babel/plugin-syntax-object-rest-spread and it single transitive dependency: @babel/helper-plugin-utils.

Also, I noticed that the @babel/core symlink is not present and is not hoisted to the root node_modules folder, which correctly contains @babel/[email protected] downloaded from npm.

Environment if relevant (please complete the following information):

• OS: Linux
• Node version 13.7.0
• Yarn version 2.0.0-rc.29.git.20200225.e4bf47c2 (master)

Additional context

[larixer]

@nicolo-ribaudo Its being worked on here:
#955
I will let you know when this PR will be ready for testing. I can confirm that this PR hoists codemods/babel-plugin-codemod-object-assign-to-object-spread/node_modules the way you expect and the total size is:

$ du -s node_modules codemods/*/node_modules eslint/*/node_modules packages/*/node_modules | tr -cd '0-9\n' | xargs | tr ' ' '+' | bc
1257848

but at the moment hoisting pass takes 6 minutes, so this PR is not yet ready. I have faster not yet comitted algorithm at the moment, but it needs some troubleshooting, as it slightly underhoists... so, it's a work in progress....

[larixer]

Should be fixed via #955

[bertho-zero]

I have a similar problem:

In a monorepo I have jest@25 everywhere, which requires jsdom@15 which requires [email protected], yesterday I did a yarn add -D jest in a new package required nowhere and the version installed is jest@26 with jsdom@16 and [email protected]

I also use jsdom@15 as a direct dependency of a package in the monorepo, the problem is that it is the w3c-xmlserializer@2 version which is hoisted in the root node_modules and jsdom@15 (from root node_modules) can no longer find [email protected].

Error: Cannot find module 'w3c-xmlserializer/lib/XMLSerializer' (v2 is hoisted)
Require stack:
- /home/bertho/oa/node_modules/jsdom/lib/jsdom/living/index.js
- /home/bertho/oa/node_modules/jsdom/lib/jsdom/browser/Window.js
- /home/bertho/oa/node_modules/jsdom/lib/api.js
- /home/bertho/oa/packages/cibul-node/services/mails/lib/incomingEmailsMw.js (use jsdom@15 > [email protected])
- /home/bertho/oa/packages/cibul-node/services/mails/index.js
- ...

EDIT:

I found two solutions:

• either I install jest@25 in my freshly created package.
• either I install jest@26 in the root package.json, in addition

[larixer]

@bertho-zero This is generally a bug in jsdom, not in yarn, the packages should not rely on some dependency to be hoisted to the top-level

[bertho-zero]

I don't understand why jsdom wouldn't find a package that is in the "dependencies" field of its package.json.

The problem appeared when I installed jest@26 while jest@25 was installed in another package.

[larixer]

@bertho-zero This problem will be actionable on our side only if you open a new issue and provide exact reproduction steps or repo that we can clone and troubleshoot problem from there.

[bertho-zero]

I understand that but find out and a problem is quick, create its reproduction by learning to use Sherlock less.

I don't see an example of monorepo or an example that tests the file structure on https://yarnpkg.com/advanced/sherlock.

[larixer]

@bertho-zero Sherlock is the best, but repo somewhere on GitHub will work as well for a demonstration of a problem

[bertho-zero]

@larixer I created a minimal repro: https://github.com/bertho-zero/yarn-bug-repro

[larixer]

@bertho-zero Thank you! I was able to reproduce this. Looks like hoister is aware that [email protected] should be placed inside jsdom/node_modules, but it is not there, checking why...

Could you open a new issue, please so that we continue discussion there, since this issue was different and has been fixed already?

[arcanis]

@bertho-zero We ask to open new issues for bookkeeping. Your issue isn't related to the one originally reported, so there's no need to spam Nicolo. I'm going to lock this issue, please open a new one if you wish to be notified of any progress 🙂