dominictarr / rc

The non-configurable configuration loader for lazy people.
Other
1.02k stars 97 forks source link

Start search for rc file from running script and not from cwd #53

Closed guyellis closed 9 years ago

guyellis commented 9 years ago

I have a situation where the node application is being started from the root of the directory structure.

App location: /home/guy/myapp/index.js Start location: /

i.e. my app is started by using: node home/guy/myapp/index.js

This makes the process.cwd() start searching from the root so it never finds my app's rc file. If we change it to __dirname then it will start searching from (in my example) /home/guy/myapp/node_modules/rc/lib/. This would solve the problem.

Implication for other users of rc that are starting from the normal home directory of the app: So long as they didn't have an rc file for their application in node_modules, node_modules/rc, or node_modules/rc/lib then it will behave as normal. I can't think of a use case where they would have their app's rc file in any of those locations...

dominictarr commented 9 years ago

if you want to include a default configuration in your app there is already a way to do that, pass them in as defaults: https://github.com/dominictarr/rc#usage

If you want to store that as a separate file, then if it is a json file you can require('./config.json')

The way that rc searches for a local file is useful for building tools (such as git or npm) that have local configuration.

If your usecase is user configuration, and you are running rc as root it will find your config it should probably find your config in /root (check $HOME as root to check)

guyellis commented 9 years ago

@dominictarr thanks for the comment.

We have a private module (or two) that are npm installed into the app that work hand in hand with the app (and many other private/internal apps we've written). The app has the .apprc file in its home directory. So for my local setup:

/home/guy/app/.apprc /home/guy/app/node_modules/myprivate/somefile.js (uses rc) /home/guy/app/node_modules/myprivate/node_modules/rc (by implication)

App is started from root / by running node home/guy/app/index.js

It's the myprivate module that's accessing the .apprc file of the application that installed it. It's not the actual running app that's using rc (in fact it's not even installed in that app). It's one of the running app's dependency’s that's using it. How should that dependency access its "parent's" .apprc file?

(I realize in re-reading that it is the running app that's using rc because it's a single process. I'm sure that you get what I mean.)

dominictarr commented 9 years ago

I don't know why you'd want to do that. Can you describe what your app does and what myprivate does? I don't think a module should be coupled to an application's configuration file - if the configuration is a part of the app it should do the loading, and then pass that config to the module.

guyellis commented 9 years ago

@dominictarr Good question and the way that I've explained it does make it look like a badly designed and too tightly coupled system. There is method to this madness and one of the alternatives that we considered is exactly what you suggested.

We have a suite of applications which have very similar functionality but different enough that they can't be a single application with configuration driving the differences. The parts of the application that are identical are pulled out into dependencies that are npm installed into each application trying to follow the DRY principle as much as possible. These dependencies still have some functionality that needs to vary based on the context that it is running in. In this case that configuration is driven by a .apprc file in the requiring application.

You are correct that the containing application could load the rc file and pass that configuration object to the required module. We would still have the same problem that the containing application would not be able to find the rc file because it's being started from the root.

neonstalwart commented 9 years ago

@guyellis is there any reason you can't process.chdir(__dirname); in /home/app/guy/index.js before you bring in rc?

guyellis commented 9 years ago

@neonstalwart I like that idea. I think that I would do that in the required module instead of the index.js in the app as that is where rc itself is being required.

Let me check to see if this would have any other ramifications. i.e. are there any other parts of our system(s) that rely on the cwd remaining the root as once this is done it will be system wide.

Another thought:

var previousCwd = process.cwd();
process.chdir(__dirname);
var rc = require('rc');
...
process.chdir(previousCwd);

Thanks for the idea. I'll get back to you...

dominictarr commented 9 years ago

okay it sounds like you make have a solution. I cannot merge this, by the way, because this would break many other applications that use local configurations from the working directory.

guyellis commented 9 years ago

@dominictarr How would it break other applications?

Is there a situation where the cwd is not a parent of __dirname?

neonstalwart commented 9 years ago

@guyellis from /foo/bar start an app in /foo/baz

guyellis commented 9 years ago

@dominictarr Another thought - instead of merging this. Is there a way to configure rc such that the default is to use cwd on this line and dirname if it's been configured to do so? i.e. another param that (in the simplest form) defaulted to process.cwd and could be explicitly set to `function() { return dirname; }`?

guyellis commented 9 years ago

I can do a PR for that instead...

dominictarr commented 9 years ago

@guyellis if your app is installed with npm -g then it's a global command. process.cwd() will not be a prefix to __dirname. is bower is one example.

@guyellis also note that the pitch for this module is "the non-configurable configuration loader", I only add configuration very reluctantly and I'm fairly confidant there is a better way to solve your problem.

By the way, If you do wish to use a specific configuration file, you can set a path to that file with --config path_to_file (or setting $appname_config)

guyellis commented 9 years ago

Agreed. I'll close this. I've created a new PR for comment: https://github.com/dominictarr/rc/pull/54

trhinehart-godaddy commented 2 years ago

Ran into this as well, my CLI's support a --cwd option to support remote execution. This is a typical pattern for monorepos and systems where a CLI task needs to be run on various isolated directories.