BugFix: deferred values should be treated like atomic values, not objects.

There was an edge case when a deferred function returned an object.

  When extendDeep does its recursion and ran into a deferred value, it would
  continue to recurse into it, potentially doing a deep merge with a child
  config file.

  Because the deferred value did not get /replaced/ by the child object as
  intended, it continued to be an 'instanceof' the DeferredConfig prototype.

  So as a final step, the deferred function was executed to compute a final
  value when it should not have been.

  The result was this the child fails to properly override the deferred
  function with a static value, because deferred function was getting run
  anyway.

  The included test coverage illustrates such a case.

  This fix does not the affect the general utility of extendDeep, as only
  objects that are 'instanceof' the newly-introduced DeferredConfig object
  are effected.
Good catch.

I  had meant to remove that after doing some before/after testing of the
difference.

I've removed the line now and force-pushed a new version of the patch into
the pull request.

Thanks for the quick review.

Also, congratulations on the new job! I see from LinkedIn you changed roles
a few months ago.

On Wed, Nov 19, 2014 at 1:38 PM, Loren West <notifications@github.com>
wrote:

> In lib/config.js:
>
> > @@ -1266,8 +1266,10 @@ util.extendDeep = function(mergeInto) {
> >      // Cycle through each element of the object to merge from
> >      for (var prop in mergeFrom) {
> >
> > -      // Extend recursively if both elements are objects
> > -      if (util.isObject(mergeInto[prop]) && util.isObject(mergeFrom[prop])) {
> > +      // Extend recursively if both elements are objects and target is not really a deferred function
> > +      var isDeferredFunc = mergeInto[prop] instanceof DeferredConfig;
> > +      if (util.isObject(mergeInto[prop]) && util.isObject(mergeFrom[prop]) && !isDeferredFunc) {
> > +      //if (util.isObject(mergeInto[prop]) && util.isObject(mergeFrom[prop])) {
>
> This line should be removed. Git is a better place for keeping history.
>
> --
> Reply to this email directly or view it on GitHub
> <https://github.com/lorenwest/node-config/pull/175/files#r20596494>.
>



-- 
    Mark Stosberg
    Senior Systems Engineer
    RideAmigos