Yarn: Yarn config merge order incorrect

[aikar]

yarn version 1.3.2

I have my global ~/.npmrc setup for the public npm repo and my account

I have a subfolder ~/work/ that has an .npmrc with registry and _auth set for my work local NPM

I have a project at ~/work/project

If I run yarn info <package> in ~/work/, it succeeds
If I run yarn info <package> in ~/work/project, it fails

yarn config get _auth inside of ~/work/ shows correct work token
yarn config get _auth inside of ~/work/project shows my public npm token

--verbose shows:

verbose 0.364 Checking for configuration file "/home/aikar/work/project/.npmrc".
verbose 0.364 Checking for configuration file "/home/aikar/.npmrc".
verbose 0.364 Found configuration file "/home/aikar/.npmrc".
verbose 0.365 Checking for configuration file "/home/aikar/.nvm/versions/node/v8.9.1/etc/npmrc".
verbose 0.365 Checking for configuration file "/home/aikar/work/project/.npmrc".
verbose 0.365 Checking for configuration file "/home/aikar/work/.npmrc".
verbose 0.365 Found configuration file "/home/aikar/work/.npmrc".

Potentially related to #5370

I expect that config files closer to my projects checkout to supersede the user folder one.

[aikar]

any update on this? This is a major annoyance when working from home, as I have to temporarily modify my ~/.npmrc and ~/.yarnrc then undo it when done.

[torifat]

This is a major annoyance when working from home

In that case feel free to send a PR. Happy to help you to get it through.

[aikar]

@torifat I think this requires discussion on how to resolve from the yarn team....

In my mind, Yarn has the configuration order backwards in terms of merge order.
It appears when merging configs, going down the line does not overwrite the original.

This could be a semver-major change to fix this, unless the team has some better idea?

My expected order would be

verbose 0.365 Checking for configuration file "/home/aikar/.nvm/versions/node/v8.9.1/etc/npmrc".
verbose 0.364 Checking for configuration file "/home/aikar/.npmrc".
verbose 0.364 Found configuration file "/home/aikar/.npmrc".
verbose 0.365 Checking for configuration file "/home/aikar/work/.npmrc".
verbose 0.365 Found configuration file "/home/aikar/work/.npmrc".
verbose 0.364 Checking for configuration file "/home/aikar/work/project/.npmrc".

Where each found config overwrites previous found keys.

[aikar]

Seems problem lies around here:
https://github.com/yarnpkg/yarn/blob/9cd66df76bcfff560e05a41903bcaa79feb84c61/src/registries/npm-registry.js#L210
https://github.com/yarnpkg/yarn/blob/9cd66df76bcfff560e05a41903bcaa79feb84c61/src/registries/npm-registry.js#L265

[buschtoens]

I can confirm this for .yarnrc as well. While the files _are_ merged, they are merged in the reversed order. This means that ~/.yarnrc will overwrite settings from ~/work/.yarnrc which IMO doesn't make any sense.

[buschtoens]

https://github.com/yarnpkg/yarn/blob/9cd66df76bcfff560e05a41903bcaa79feb84c61/src/registries/npm-registry.js#L225-L230

We either need to use .unshift() instead of .push() here or reverse the array after creation. Don't know what is better performance-wise.

This would also fix the YarnRegistry (.yarnrc) since it extends the NpmRegistry:

https://github.com/yarnpkg/yarn/blob/cb81cde932fea05bc2d94bf8984c7aea99d75bcd/src/registries/yarn-registry.js#L42

[buschtoens]

If we're semver lawyering, technically this could be a breaking fix, since some peeps might implicitly depend on this behavior.

@torifat I am happy to submit a PR. Just tell me, if the proposed solution is alright with you.

[alexkrolick]

This is actually quite dangerous when private repositories are involved - something could be published to a public registry accidentally if the publishConfig is not set in package.json.

[hilkeheremans]

Seems this is still an issue, at least on yarn v1.15.2. I bumped into this when using @pika/pack which tries to use yarn if available, and apparently I had an old token lying around somewhere up the tree.

Also happy to submit a PR, given some guidance from the team on how they would want to approach this. @arcanis Could you refer us to the proper person for some guidance?

[arcanis]

Unfortunately such a change would be significant enough to mandate a major version bump 🙁

On the bright side, the next major bump already overhauls the configuration loading (the algorithm to find the files will only consider the folder hierarchy with no special consideration for the home folder), and thus this particular bug should already be fixed there.

[hilkeheremans]

Alright, kudos and thanks for the _really_ swift reply, that clears things up. I can survive till then for my particular case.

[givanse]

@arcanis is this new algorithm in master or what branch would it be?