lorenwest/node-config — pull request #163 by markstos (2014-11-13 16:39:37+00:00)
Title: Some updates to remove assumptions that the data is full of 'data' properties.

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.