fix(config): allow binLinks to be set in .yarnrc

[iansu]

Summary

Fixes #4925. Allow setting bin-links false in .yarnrc. Currently setting this option doesn't have any effect.

Test plan

• Create a package with a .yarnrc file containing the line bin-links false
• Run yarn add gulp-cli (or any package with binaries)
• Verify that no bin links were created in node_modules/.bin

[iansu]

Look like this broke some stuff. 😬

I'll look into the failing tests tomorrow.

[arcanis]

Isn't this a duplicate of

--no-bin-links true

?

[iansu]

I think the issue was that you just couldn't set bin-links (or no-bin-links) in .yarnrc at all. It could only be specified as a command line argument.

[kaylie-alexa]

@iansu I think the issue is that the config isn't being read properly, rather than not being set properly. This line should be using .getOption instead of

yarn/src/package-linker.js

Line 403 in 7ed01cf

if (this.config.binLinks) {

Would you mind testing the fix for the bug after that?

[iansu]

@kaylieEB Sure, I'll take a look at that tomorrow.

[iansu]

@kaylieEB I tried the change you suggested in package-linker.js but it doesn't seem to work.

I changed that line to if (this.config.getOption('bin-links')) { and tested the original code as well as my changes. I tested with an empty .yarnrc file as well as one with bin-links true and bin-links false. In all cases bin links were not created.

[kaylie-alexa]

hmm, I was able to verify it on my end w/ this.config.getOption('bin-links')

~/playground/testing 🦊  yarn config set bin-links true
yarn config v1.4.0
success Set "bin-links" to "true".
✨  Done in 0.06s.
~/playground/testing 🦊  yarn add gulp-cli > /dev/null && ls node_modules/.bin
atob		color-support	gulp		loose-envify	semver		which
~/playground/testing 🦊  yarn remove gulp-cli
yarn remove v1.4.0
[1/2] Removing module gulp-cli...
[2/2] Regenerating lockfile and installing missing dependencies...
success Uninstalled packages.
✨  Done in 0.97s.
~/playground/testing 🦊  yarn config set bin-links false
yarn config v1.4.0
success Set "bin-links" to "false".
✨  Done in 0.06s.
~/playground/testing 🦊  yarn add gulp-cli > /dev/null && ls node_modules/.bin
ls: node_modules/.bin: No such file or directory

[iansu]

Well that's interesting. The only difference I can see is that you're using yarn config set bin-links true which sets that in ~/.yarnrc whereas I was creating a .yarnrc at the root of my test package. I'll take another look at this.

[iansu]

I tested this again and got different results. No idea what happened the first time, I followed the same process both times. Anyway, using this.config.getOption('bin-links') I was able to set bin-links in a local .yarnrc file.

However, I did find another problem. With this.config.binLinks bin-links seems to default to true. With this.config.getOption('bin-links') it seems to default to false. I ran yarn config list to make sure I didn't have bin-links set to false somewhere else and it was not in the list. I'm also seeing 18 failed tests, most of which reference bin links.

At this point I think the best solution is for me to revert my change in config.js and apply your change in package-linker.js and then figure out how to make sure bin-links is defaulted to true.

[kaylie-alexa]

@iansu ping! any updates?

[iansu]

Not much of an update. I tried a few things to get bin-links to default to true but none of them worked properly. I can look into again this week. Any tips on where or how to change the default for that setting (or settings in general) would be appreciated.

[kaylie-alexa]

Sorry for the delay, did you try setting the default here?

yarn/src/registries/yarn-registry.js

Lines 15 to 30 in 8918b6c

	export const DEFAULTS = {
	'version-tag-prefix': 'v',
	'version-git-tag': true,
	'version-git-sign': false,
	'version-git-message': 'v%s',
	'init-version': '1.0.0',
	'init-license': 'MIT',
	'save-prefix': '^',
	'ignore-scripts': false,
	'ignore-optional': false,
	registry: YARN_REGISTRY,
	'strict-ssl': true,
	'user-agent': [`yarn/${version}`, 'npm/?', `node/${process.version}`, process.platform, process.arch].join(' '),
	};

[iansu]

I tried changing it in a number of places but not at the registry level. It looks like that fixes all the tests except for one. I'll update the PR and push my changes so we can see the CI results.

[rally25rs]

This is already supported without this PR in a roundabout way. no-bin-links is a flag (or CLI argument as it is called in the docs) not a configuration option and so is supported like other flags in the .yarnrc file.

Try using --*.no-bin-links true in .yarnrc

~/Projects/yarn-test 🐒   ls node_modules/.bin/
har-validator  in-publish     node-gyp       nopt           not-in-publish sassgraph      sshpk-conv     sshpk-verify   uuid
in-install     mkdirp         node-sass      not-in-install rimraf         semver         sshpk-sign     strip-indent   which

~/Projects/yarn-test 🐒   rm -rf node_modules/

~/Projects/yarn-test 🐒   echo "--*.no-bin-links true" > .yarnrc

~/Projects/yarn-test 🐒   yarn
yarn install v1.3.2
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
[4/4] 📃  Building fresh packages...
✨  Done in 10.94s.

~/Projects/yarn-test 🐒   ls node_modules/.bin
ls: node_modules/.bin: No such file or directory

[BYK]

@rally25rs looks like this is also a config option in npm: https://docs.npmjs.com/misc/config#bin-links

For this reason, I'm in favor of merging the PR. What do you think?

[rally25rs]

@BYK didn't realize it was also an npm config setting. In that case I'm not opposed to adding it.

[BYK]

Thanks a lot for the patch @iansu and sorry for the wait!

[iansu]

Awesome! Nice to see it finally make it in.