yarn
yarn copied to clipboard
Yarn workspace incorrectly resolves node_modules path during script execution of workspace dependencies
What is the current behavior? When you install the main project, it correctly begins installing the workspace dependencies. However, if one of the scripts in a dependency attempts to execute a command, the location of the node_modules folder is incorrectly resolved.
As an example of the error: \workspaceModuleBug*node_modules\node_modules*\shx\lib\cli.js
If the current behavior is a bug, please provide the steps to reproduce. With yarn workspaces enabled, clone the following repository and do a yarn install. https://github.com/labriola/workspaceModuleBug
Yarn will throw an error when attempting to do an shx echo.
What is the expected behavior? The node_module path should recurse upward correctly instead of making (what I believe to be) an invalid assumption.
Please mention your node.js, yarn and operating system version. node v.6.11.3 yarn 1.1.0 windows 10
Happy to attempt to resolve if I can get a hint on where to poke around.
Apparently this is windows only.
Long and short of it is that, when creating the output shim, we compute the path relative to the root. So in my example, we compute the relative path from 'test2' as a folder directly under the workspace to the node_module folder of the workspace.
from - ..\workspaceModuleBug\test2\node_modules\.bin to - ..\workspaceModuleBug\node_modules\[package]\dist\index.js
..However, before executing the spawned process, we add the nested path (underneath the node_module\test2\node_module):
...\workspaceModuleBug\node_modules\test2\node_modules\.bin
So note, we wrote a CMD that understands the relative path between test2\node_module\.bin and the workspace's node_modules, but we add the installed module, node_modules\test2\node_modules to the path. So, when we execute the spawned child process, it fails because the path incorrectly resolves to:
...\workspaceModuleBug\node_modules\node_modules\[package]\dist\index.js
Note, the duplicate node_module. If I can get advice on the proper place to patch this, I will be happy to try but I am unsure if altering the path we create is a good idea. (or how to tell when I should) in the current code.
Also, since the same physical package installation will now be available via two different physical paths, I don't know we can fulfill both requirements with the current template of the CMD file as generated by cmd-shim. So it would either need to be more intelligent or would only work in one of the two circumstances.
This line is where the potentially errant path is added:
https://github.com/yarnpkg/yarn/blob/d64512c632e1ed12f5bffe8a1f50d235c18f0196/src/util/execute-lifecycle-script.js#L152
I am not saying this is necessarily where the fix should go but I am hoping it provides a quick way for someone to review and let me know a valid way to proceed.
Wondering if anyone had further thoughts on this?
Still an issue in yarn 1.7.0. Have not found any workaround yet.
@BYK is this still on your list? It seems like a rather big blocker for using workspaces.
We have the same issue with yarn 1.9.4, node_modules
twice and scripts in node_modules/.bin
cannot be executed.
This seems to be Windows only problem, so I wonder if junctions are somehow not properly handled which causes node_modules being added twice to the bin path?
error C:\Users\pronebird\mullvad\mullvadvpn-app\gui\node_modules\@mullvad\components: Command failed.
Exit code: 1
Command: yarn run build
Arguments:
Directory: C:\Users\pronebird\mullvad\mullvadvpn-app\gui\node_modules\@mullvad\components
Output:
yarn run v1.9.4
$ run-s private:build:clean private:build:compile
module.js:549
throw err;
^
Error: Cannot find module 'C:\Users\pronebird\mullvad\mullvadvpn-app\gui\node_modules\node_modules\npm-run-all\bin\run-s\index.js'
at Function.Module._resolveFilename (module.js:547:15)
at Function.Module._load (module.js:474:25)
at Function.Module.runMain (module.js:693:10)
at startup (bootstrap_node.js:188:16)
Update
I was able to fix this by adding a preinstall
script to one of workspace packages that linked node_modules\node_modules
to node_modules
. Ugly but it works. I wonder why this is not being addressed for almost a year. Seems like a critical bug.
I was able to fix this by adding a preinstall
script that linked node_modules\node_modules
to node_modules
to one of packages in workspace. Ugly but it works. I wonder why this is not being addressed for almost a year. Seems like a critical bug.
I'm facing same issue.
Issue still exists in 1.22
Can confirm that still happens on version 2.4.0 :c
Seeing this as well inside a Yarn Workspace. OS: Windows, Yarn v1.22.5, Node v14.15.3.
error C:\Users\hi\Documents\...\node_modules\@myapp\app: Command failed.
Exit code: 1
Command: electron-builder install-app-deps
Arguments:
Directory: C:\Users\hi\Documents\...\node_modules\@myapp\app
Output:
internal/modules/cjs/loader.js:883
throw err;
^
Error: Cannot find module 'C:\Users\hi\Documents\...\node_modules\node_modules\electron-builder\out\cli\cli.js'
at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)
at Function.Module._load (internal/modules/cjs/loader.js:725:27)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12)
at internal/main/run_main_module.js:17:47 {
why this critical issue is not been solved, and even not a single contributor commented on this???
Not a solution, but in some cases another workaround could be to prevent yarn from hoisting the required dependencies:
I.e., in the failing packages (not in the root package.json!):
"workspaces": {
"nohoist": [
"rimraf"
]
}
This will make sure that rimraf is resolved in the package-local node_modules.
But I would appreciate a fix in yarn, too :-)
I'm running into this as well and tried the "nohoist"
workaround. Unfortunately, "nohoist" is not allowed in public packages in yarn, so it would need to be added to the root package.json if the failing package is a public package.
Just ran into this with a co-worker on windows.
Also running into this. 1.22.5
Still a problem in 2023 ...
just ran into this issue on windows as well
Is there a fix for this? Having the same issue
Still the same problem in 2024...
Quick update on what's happening here.
The failure is happening when running the generated cmd files.
Detail
Here's an example:
Say we have
- /package.json
- /packages/a
- /packages/b
Now assume our packages are scoped as: @pkg/a
@pkg/b
/packages/b/node_modules/.bin/rimraf.cmd
@IF EXIST "%~dp0\node.exe" (
"%~dp0\node.exe" "%~dp0\..\..\..\..\node_modules\prisma\build\index.js" %*
; Added this line
echo "%~dp0\..\..\..\..\node_modules\prisma\build\index.js"
) ELSE (
@SETLOCAL
@SET PATHEXT=%PATHEXT:;.JS;=;%
; Added this line
echo "%~dp0\..\..\..\..\node_modules\prisma\build\index.js"
node "%~dp0\..\..\..\..\node_modules\prisma\build\index.js" %*
)
When it gets run from the symlink path (eg. C:\project\node_modules\@pkg\a\node_modules\.bin
) instead of the resolved (real) path (eg. C:\project\packages\a\node_modules\.bin
), it will resolve incorrectly, because the files are written assuming we're relative to the repo root.
For example, this is what the paths would look like:
bin file path:
C:\project\node_modules\pkg\a\node_modules\.bin\\..\..\..\..\node_modules\prisma\build\index.js
resolved:
C:\project\node_modules\\node_modules\prisma\build\index.js
Why it Happens
The issue is due to the fact that yarn is adding the symlink path instead of the real path to the PATH env.
@labriola was correct in it originating here:
https://github.com/yarnpkg/yarn/blob/7cafa512a777048ce0b666080a24e80aae3d66a9/src/util/execute-lifecycle-script.js#L198
This gets added as the top-most search location. Using the symlink path instead of the fully resolved real path will cause the relative paths to be broken.
Solutions
Solution 1 : Modify execute-lifecycle-script.js
This would be the most ideal IMO, assuming there are no side-effects.
A simple change is to edit makeEnv
to the following:
// Add node_modules .bin folders to the PATH
for (const registryFolder of config.registryFolders) {
const binFolder = path.join(registryFolder, '.bin');
if (config.workspacesEnabled && config.workspaceRootFolder) {
pathParts.unshift(path.join(config.workspaceRootFolder, binFolder));
}
pathParts.unshift(path.join(config.linkFolder, binFolder));
// We resolve the path here before adding to PATH
const realBinFolder = await fs.realpath(path.join(cwd, binFolder));
pathParts.unshift(realBinFolder);
}
Solution 2 : Modify cmd files
We can resolve symlinks in the generated command files.
Here is an example:
@echo off
SETLOCAL
FOR /f "tokens=*" %%i IN ('fsutil reparsepoint query "%~dp0" ^| find "Print Name"') DO SET RealPath=%%i
SET RealPath=%RealPath:*Print Name: =%
@IF EXIST "%~dp0\node.exe" (
"%~dp0\node.exe" "%RealPath%\..\..\..\node_modules\rimraf\dist\esm\bin.mjs" %*
echo "%RealPath%\..\..\..\node_modules\rimraf\dist\esm\bin.mjs"
) ELSE (
node "%RealPath%\..\..\..\node_modules\rimraf\dist\esm\bin.mjs" %*
echo "%RealPath%\..\..\..\node_modules\rimraf\dist\esm\bin.mjs"
)
ENDLOCAL
We'd want to ensure that this works with Win XP and up, but I believe it will.
If not, we can also do something more simple, like using cd to the path.
Questions for the Yarn Team
Questions I'd have for the Yarn team are:
- Solution 1 a. Would it be better to just use realpath at a higher level? b. Is there any potential downside to modifying it here?
Also: I see that some patches are being accepted for Yarn 1 which are not strictly security related. Notably, I tried this on Yarn 3 and had the same issue.
I'd love to get this merged, and if you require, I'll do an update Yarn 3 as well on whatever route we determine.
PR
- #9078
PR is submitted. Hopefully I can get in touch with a team member so we can get it through.
Here's a release you all can use for now:
I'll keep the latest
tag pointing to the most recent
Looks like it's working here. Would love to hear feedback if you try it out.
Workaround & Update
I'm holding on completing the PR for now to see if I get a response. My published version works in the case of lifecycle scripts, but there are certain other issues where it plays a factor.
It may make more sense to hit this in the generated .cmd
files. In any case, I'll wait to see if the team responds.
In the mean time, I made an npx tool we can use that will correct the .cmd files by making them resolve and use the realpath, where possible / necessary.
- https://github.com/nonara/yarn-fix-bin-cmds