-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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(config): allow .npmrc config to supersede yarn defaults #5825
Conversation
Running `yarn config get registry` within a project that contains a .npmrc which overrides the registry config key is not respected and instead the default yarn registry option is returned. Conceptually if you look at the code for `getOption` it first checks to see if the config exists within yarn and if not found it will check for that config within npm's config. However, the problem arises as the config property that is built up gets merged with yarns default values. This means that any config key that exists as a yarn default cannot be overwritten via .npmrc. This change differencates yarn default configs and configs specified via .yarnrc. This allows the proper config fall back.
#5812 also seems related. |
Hey @thoov - thanks very much for this PR! I think right now @otaku is working on it, but if not and you wish to go for it yourself - I'd be happy to merge that future PR (and/or provide clarifications beforehand as needed). How does that sound? EDIT: @thoov - if you'd like to work on this, it's yours. If not, I'll open an issue and we'll address this ourselves if there are no other takers. |
@imsnif is the guidance specifically this comment #5812 (comment)? |
@stefanpenner - yep. |
@imsnif Sorry I stepped away from this for a few days. I would gladly help out on implementing what you have described in #5812 (comment). Is someone working on that? |
No worries, @thoov - to the best of my knowledge no-one is working on this specifically. However, there is a general issue about merging the two registries here: #5873 @arcanis - would it help out if we do as described here: #5812 (comment) ? |
Yeah, I think it would be a good thing. I wonder if we should whitelist the options and only keep the ones that we are sure are safe, since they might have different type or interpretation depending on the package manager. |
Hey @thoov - what do you think? Still feel like working on this? |
Summary
Running
yarn config get registry
within a project that contains a .npmrc which overrides the registry config key is not respected and instead the default yarn registry option is returned. Conceptually if you look at the code forgetOption
it first checks to see if the config exists within yarn and if not found it will check for that config within npm's config. However, the problem arises as the config property that is built up gets merged with yarns default values. This means that any config key that exists as a yarn default cannot be overwritten via .npmrc. This change differentiates yarn default configs and configs specified via .yarnrc. This allows the proper config fall back.Test plan
Returns
https://registry.yarnpkg.com/
Returns
http://example.com/