fix(modules-folder): Fix --modules-folder handling in several places. #6850
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes a few issues with the `--modules-folder` parameter. The extraneous file check would always also clean files from `node_modules`. This no longer happens, only the `--modules-folder` location is checked. The `yarn check` command used to always only check `node_modules`. Now it will check the `--modules-folder` location instead. When running a command or executing a lifecycle script, yarn would build a `PATH` that included `node_modules/.bin` first, then add the `--modules-folder`'s `/.bin` after, which would have led to the wrong binaries being executed. fixes yarnpkg#5419 and fixes yarnpkg#6847
|
Seeing failures on Appveyor (I tried running it 3 times), but locally on OSX and Win 10 with Node 8 and 10, I the same tests pass. Can anyone else see if they can reproduce the Appveyor failures? |
|
Fixes #2784 |
|
Any chance this could be prioritized? This will help us a ton. 🙇 |
rally25rs
added a commit
to rally25rs/yarn
that referenced
this pull request
Mar 27, 2019
…nds. Fixes run in workspaces. This fixes a bug that was introduced in yarnpkg#6850 where the bin path was being built only from `config.lockfileFolder`. However in workspaces, bins may not be hoisted to the workspace root, causing bins to not be found. This change adds `config.cwd` to the bin search path, so the `yarn run` command will look in a workspace package's node_modules, as well as the workspace root. fixes yarnpkg#7126
arcanis
pushed a commit
that referenced
this pull request
Oct 30, 2019
* fix(run): change run command to check cwd/node_modules/.bin for commands. Fixes run in workspaces. This fixes a bug that was introduced in #6850 where the bin path was being built only from `config.lockfileFolder`. However in workspaces, bins may not be hoisted to the workspace root, causing bins to not be found. This change adds `config.cwd` to the bin search path, so the `yarn run` command will look in a workspace package's node_modules, as well as the workspace root. fixes #7126 * modify chagelog
arcanis
pushed a commit
that referenced
this pull request
Nov 22, 2019
* fix(run): change run command to check cwd/node_modules/.bin for commands. Fixes run in workspaces. This fixes a bug that was introduced in #6850 where the bin path was being built only from `config.lockfileFolder`. However in workspaces, bins may not be hoisted to the workspace root, causing bins to not be found. This change adds `config.cwd` to the bin search path, so the `yarn run` command will look in a workspace package's node_modules, as well as the workspace root. fixes #7126 * modify chagelog
VincentBailly
pushed a commit
to VincentBailly/yarn
that referenced
this pull request
Jun 10, 2020
…kg#7151) * fix(run): change run command to check cwd/node_modules/.bin for commands. Fixes run in workspaces. This fixes a bug that was introduced in yarnpkg#6850 where the bin path was being built only from `config.lockfileFolder`. However in workspaces, bins may not be hoisted to the workspace root, causing bins to not be found. This change adds `config.cwd` to the bin search path, so the `yarn run` command will look in a workspace package's node_modules, as well as the workspace root. fixes yarnpkg#7126 * modify chagelog
VincentBailly
pushed a commit
to VincentBailly/yarn
that referenced
this pull request
Jun 10, 2020
…kg#7151) * fix(run): change run command to check cwd/node_modules/.bin for commands. Fixes run in workspaces. This fixes a bug that was introduced in yarnpkg#6850 where the bin path was being built only from `config.lockfileFolder`. However in workspaces, bins may not be hoisted to the workspace root, causing bins to not be found. This change adds `config.cwd` to the bin search path, so the `yarn run` command will look in a workspace package's node_modules, as well as the workspace root. fixes yarnpkg#7126 * modify chagelog
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a few issues with the
--modules-folderparameter.The extraneous file check would always also clean files from
node_modules. This no longer happens, only the--modules-folderlocation is checked.The
yarn checkcommand used to always only checknode_modules. Now it will check the--modules-folderlocation instead. (change tosrc/config.js)When running a command or executing a lifecycle script, yarn would build a
PATHthat includednode_modules/.binfirst, then add the--modules-folder's/.binafter, which would have led to the wrong binaries being executed. (change tosrc/util/execute-lifecycle-script.js)There were a couple places where
config.registries[name]was used, instead ofconfig.registryFolders(which would contain the--modules-folderoverride) so I fixed those. (change tosrc/cli/commands/autoclean.jsandsrc/cli/commands/check.js)fixes #5419
fixes #6847
fixes #2784
fixes #6918
Test plan
test added for checking that extraneous file removed from
--modules-folderlocation instead ofnode_modules.Not easy to test in our framework but should be manually tested; make sure
--modules-folderworks as a relative and absolute paths. I think the biggest risk in this PR is that I changed a couplepath.joins topath.resolveto handle the case wheremodules-folderis an absolute path. I think it's working correctly everywhere, but something to keep an eye out for.