Some updates to remove assumptions that the data is full of 'data' properties.

[markstos]

The remaining code is still not 100% getter-friendly, and I have doubts now that getters can be well-supported.

Still, I think the changes here are improvements, so I'm submitting them anyway. They pass all the tests.

• lookupGetter is not standards-compliant, although it happens to work in Node. I replaced it with the standards-compliant version.
• JSON.stringify() was replaced with a faster check to see if an object had any properties. The change also happens to avoid triggering potentially recursive getter calls.
• makeHidden no longer assumes that the property is a data property.

makeImmutable and getImpl still contain assumptions that object properties contain data values, and JSON.stringify() is still used in another location, which means that getters are converted into the values that they return in that case.

---

Here's what I found about getter support after exploring it and why I'm changing back to my alternate approach.

• The more useful case for binding 'this' in configuration getters is to bind it to the top-level configuration objects. However, JSON.stringify() binds 'this' to the local object, and that can't be changed. Also, because of the way that recursion is used elsewhere in the code, properties are always expected to be bound to the local object, and that's not really changable.

  Having getteres that reference 'sister' properties in the same object still
  has some value, but it significantly reduced.

• getters can violate the "principle of least surprise" becase they appear to be normal properties and are easy to accidently convert to the returned value. This doesn't matter for immutable values as the value doesn't change, but it was definitely an assumption that internal of the config system made in a number of cases.

Just before I discovered getters, I was close to finishing implementing the alternate approach of supporting 'deferred values'. Having reviewed that now side-by-side with getters, I think it's the better route to go.

The implementation would look like this. In a config file, you would declare a value to be "rendered" once during a final step of the configuration process. This would allow you to declare a function in a "default" file which would refer to another value. However, because the function would be called at the very end, the value the function refers to could be overridden by another configuration file at that point. Something like this:

{
  siteTitle: "The website!",
  email : {
    subject: defer(function (config) {
           return "Welcome to "+config.siteTitle";
    }),
  }
}

defer() would return the function with the prototype set to Deferred.

As a final processing step in generating the configuration, the merged configuration would be recursed to look for values that are functions with a prototype of Deferred. When found, the values would be replaced the result of calling the function bound to the top level configuration object (so the whole object would be available, not just a piece).

From there on, the values be static, so they would render to JSON normally and there would be no surprises about them being non-normal data values.

Loren,

Are you interested in the "deferred values" feature? It could be implemented outside of node-config, but would be a little simpler and more pleasant to use if it was available internally.

[markstos]

I also recommend a policy update regarding getters to say either that "getters are not supported (but may work in some in some cases)" or "getters are expected to be only be used bound to the local object context". With the later expectation, it would be a mistake to refer to a property by the same name, which is the source of recursion. Thus that class of getter-recursion issue would go away.

[lorenwest]

Hi Mark,

This looks like a healthy pull request. It doesn't solve all the problems, but it's an improvement.

Personally, I'm not a fan of getters in configs. Considering the effort you've gone through and ongoing problems they may cause, I agree with stating they are experimental and discouraging their use.

Thanks for the pull request. I'll merge and get a new version into NPM today.

-Loren

[lorenwest]

I like your deferred proposal, and think it's a better approach for "templating" config values. I would very much like the 2.0 version of node-config to be pluggable, so node-config isn't managing CSON, etc. and a templates plugin may make sense. Other plugins could include configs returned from URLs, etc.

The reason it would be 2.0 is the interface would be more of a deferred type interface. I believe we've taken the "everything loads on first tick" approach to the end of it's useful life. Issues like templating, URL and DB based configs, etc. don't work well in this environment, and more folks are warming up to the concepts of deferred and promises.

[markstos]

I thought some about how to make this bit pluggable. Here are the ideas I considered:

1. emit an event when the final config structure made, so that something could listen for that event and further modify it. The async nature of this approach might be an issue in some cases, if something tries to use the configuration before the event listener is finished.
2. Use a generic method-modifier library that can register other methods to run 'before' and 'afternativeconfig` methods. I've familiar with a few of these libraries in Perl, but I haven't found the standard pattern for doing this in JavaScript / node.js.
3. Explicitly set up our own 'hook' points and have plugins register on them. When each hook point is reach, run the list of registered events in order.

I used the third approach in CGI::Application and it worked pretty well.

I haven't looked a lot how the plugin pattern is implemented in Node.js projects but I agree it's a good idea-- keep a small simple core and make it pluggable. It's a big benefit to remove the core maintainers as a bottleneck. It also lets plugin authors experiment and innovate without further permission, while sharing the maintenance load.

I think I'll proceed with my original 'deferred' plan for as a potential built-in feature, and then we'll have it as active case to consider on how to make it pluggable in 2.0.

I did look at potentially using a 'deferred' or 'promise' library instead of rolling my own version of the concept. In particular I look at Bluebird. It didn't look like a good fit, nor did any searches on npmjs.org turn up a good fit. However, it was just a few lines of code to roll-my-own, so I went with that.

[lorenwest]

I have used q.js, and have moved to bluebird on another project, and am enjoying the results.

My suggestion is that you don't use the deferred/promose semantics unless it can be used like deferred/promise.

[markstos]

Good suggestion on the semantics.