kevva / get-proxy

Get configured proxy
MIT License
15 stars 2 forks source link

npm doesn't use rc #5

Closed dominictarr closed 7 years ago

dominictarr commented 7 years ago

you should probably use npm's config module instead of rc

https://www.npmjs.com/package/npm-config

because npm doesn't do exactly the same thing as rc, because of historical reasons, although it may be similar.

where as rc does this: https://github.com/dominictarr/rc/pull/88#issuecomment-261767255

but I can't change that because I have no idea what else it might break

kevva commented 7 years ago

npm used to use https://github.com/npm/npmconf but it's documentation clearly states that it shouldn't be used. I'm afraid that changing to another module would introduce other bugs, and apart from this one, I've never encountered any issues.

dominictarr commented 7 years ago

hmm, it says

However, if you are writing a new Node.js program, and want configuration functionality similar to what npm has, but for your own thing, then I'd recommend using rc, which is probably what you want.

but what you want is to load npm's configuration. rc will not attempt to be fully compatible with npm, because that would be a huge maintenance headache.

Probably the simplest thing would be to depend on npm, but just the configuration loading utilities, and probably lobby the npm crew to maintain their configuration system at a dependable module for other npm related tools to depend on. Obviously extra tooling around npm improves their ecosystem and thus their business so I think this is an easy argument to make.

kevva commented 7 years ago

Obviously extra tooling around npm improves their ecosystem and thus their business so I think this is an easy argument to make.

Indeed, surprised they moved the config logic into core since it's kinda useful for tools outside of it.

Anyway, I tried using npm directly and as far as I can see it works as intended. Hurts having to depend on it just to get the config though.

dominictarr commented 7 years ago

you should let them know you are doing this, otherwise they might change the api, maybe not because it's config, but maybe.

kevva commented 7 years ago

Fixed in https://github.com/kevva/get-proxy/commit/2869a1747639f68b61d40fc0ce983c68efd9ac3c.