-
Notifications
You must be signed in to change notification settings - Fork 329
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
Fix Yarn detection when using workspaces, fix .yarnrc.yml
loading
#1148
Conversation
Converting to draft because, I realized the logic for finding the |
determinePackageManager
, fix Yarn detection when using workspaces.yarnrc.yml
loading
I appreciate the point, though let's stick with a separate test folder to be consistent with the project's conventions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
Thanks for the feedback, should be good now :) |
Thanks for merging. I checked out your changes, it all looked good except, I find nested ternaries to be hard to read. I guess not everyone feels that way though. No need to change 👍 |
Thanks for your contribution! I'll release it shortly.
👏👏👏 preferences 🙃🙃🙃 |
Issues Addressed
packageFile
option was passed, the code still looked in.
(current directory) when checking for ayarn.lock
. Now it starts looking in the directory of the package file. I believe this is the correct behavior..yarnrc.yml
always being loaded from./.yarnrc.yml
, which would not work if thepackageFile
option is passed or we are in a non-root workspace directory..yarnrc.yml
never being loaded. The code usedfs.existsSync('~/.yarnrc.yml')
, but this does not interpret~
as the user's home directory.Notes
mocha
script to enable running a single test file, e.g.npm run mocha -- src/lib/determinePackageManager.test.ts
.npmrc
loading with npm workspaces and it worked, so that doesn't need to be fixed. 🎉