Requiring nconf prevents using browserify to "flatten" this module.

[brewnerd]

Browserify is a popular solution for converting node modules into a single js file to include in the browser. In my case, I want to include this code in my HTML (Sencha Touch) based mobile app for iPhone/Android.

The "nconf" package is used in one small place in this code, and it causes errors in the browser. Specifically, "Uncaught TypeError: fs.readdirSync is not a function"

There's already a request for nconf to be browserify-friendly (indexzero/nconf#130) but it'd be nice if this project wouldn't use it.

[KarbonDallas]

@brewnerd hey, thank you for the report! I plan to use browserify myself, so this is good to know.

Perhaps we should just use minimist or something along with parsing ENV vars manually? I'd love to hear your suggestions on this, and of course pull requests are welcome!

[brewnerd]

@emilyrose Ideally, it's be nice to have a module that works in both the browser and nodejs.

Here's a pretty good article on that: http://www.richardrodger.com/2013/09/27/how-to-make-simple-node-js-modules-work-in-the-browser/#.VdH8hHUVhBc

Here are some of the issues I've found trying to port (okay, hack) this into the browser:

1. Using "nconf" causes unrecoverable errors when the module is "browserified". Really, any module that requires filesystem access may not work right in the browser (even w/ HTML5 local storage).
2. The "net" module can't be used from the browser b/c the browser doesn't allow direct access to sockets.

Regarding "nconf": honestly, I think the read-config-from-file feature can be dropped. It's easy for developers to add this feature themselves, they probably have their own "master" config file anyway, and it causes issues in the browser. If your target audience includes non-developers then command line options might work, but you'll probably want to conditionally exclude that feature when not running in node.

I don't see a way to get the "tcp" protocol to ever work in the browser w/o Photon support for websockets (this is what "net-browserify" uses). The TCP API will only work in nodejs and needs to be excluded in the browser version. Maybe drop the protocol altogether once the HTTP API is implemented in the Photon?? This would simplify things... one protocol to rule-them-all. At a minimum I think you have to detect if you're running in node (see above link) and not execute any of that code.

Other options I can see:

1. support 2 versions (blah)
2. split the conf-file, tcp-protocol, and http-protocol into modules that are conditionally included (seems overkill)

For the record, I think this module is written well and I'm thankful it's available. Your audience may be broader than developers like myself, but here's what I care about:

1. Work seamlessly in node and the browser (no special npm dependencies I have to install or funky Browserify command lines).
2. Only needs to support the HTTP API.
3. Pass options programatically. No filesystem support needed.

This is just the perspective of one developer. Anyways, until the HTTP API is available, it's a moot point. (:

Cheers!

[KarbonDallas]

@brewnerd,

Thank you for taking the time to give such a thorough description! I really appreciate your feedback on this. I think you bring up several very compelling points, and it seems like @lourd has some similar thoughts as well. Perhaps we should continue the discussion over on #2?

I agree with you specifically about breaking things up into several modules, and also the filesystem/environment stuff is probably overkill for most people anyway, so that defeats most of the benefit of nconf.

[kefir135]

@emilyrose,

a simple question from an amateur (non-developer trying to develop something):
Since I got a little lost in this discussion, I'd like to ask a simple question - can we or can we not use this library in the browser now ? If yes, how ?

[lourd]

Hi @kefir135,

No, this package isn't ready for use in the browser yet.

[KarbonDallas]

This is now fixed! thanks @brewnerd!