Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Updated Memory.merge to handle null values #63

Merged
merged 2 commits into from

3 participants

@SchoonologyRRL

Previously, if the Memory store was merged with an object containing a null value, the following Error occurred:

TypeError: Object.keys called on non-object
    at Function.keys (native)
    at Memory.merge (/.../node_modules/nconf/lib/nconf/stores/memory.js:199:17)
    at Memory.merge (/.../node_modules/nconf/lib/nconf/stores/memory.js:200:17)
    at Array.every (native)
    at Memory.merge (/.../node_modules/nconf/lib/nconf/stores/memory.js:199:29)
    at common.merge (/.../node_modules/nconf/lib/nconf/common.js:99:13)
    at Array.forEach (native)
    at common.merge (/.../node_modules/nconf/lib/nconf/common.js:98:22)
    at Array.forEach (native)
    at Object.common.merge (/.../node_modules/nconf/lib/nconf/common.js:97:8)

This commit prevents that.

@SchoonologyRRL SchoonologyRRL Updated Memory.merge to handle null values
Previously, if the Memory store was merged with an object containing a null value, the following Error occurred:

TypeError: Object.keys called on non-object
    at Function.keys (native)
    at Memory.merge (/.../node_modules/nconf/lib/nconf/stores/memory.js:199:17)
    at Memory.merge (/.../node_modules/nconf/lib/nconf/stores/memory.js:200:17)
    at Array.every (native)
    at Memory.merge (/.../node_modules/nconf/lib/nconf/stores/memory.js:199:29)
    at common.merge (/.../node_modules/nconf/lib/nconf/common.js:99:13)
    at Array.forEach (native)
    at common.merge (/.../node_modules/nconf/lib/nconf/common.js:98:22)
    at Array.forEach (native)
    at Object.common.merge (/.../node_modules/nconf/lib/nconf/common.js:97:8)

This commit prevents that.
ed41c51
@indexzero
Owner

@SchoonologyRRL Can you provide test coverage for the use case you're fixing? Similar to what I did here in #67

@SchoonologyRRL

Of course. This was a quick request for comment generated from the GitHub web UI. :smile: I'll get a test or three up promptly.

@SchoonologyRRL

@indexzero Test added. When run on master, this test fails. When run on the patch branch, this test passes.

Huzzah!

@indexzero indexzero merged commit 818526c into indexzero:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 27, 2012
  1. @SchoonologyRRL

    Updated Memory.merge to handle null values

    SchoonologyRRL authored
    Previously, if the Memory store was merged with an object containing a null value, the following Error occurred:
    
    TypeError: Object.keys called on non-object
        at Function.keys (native)
        at Memory.merge (/.../node_modules/nconf/lib/nconf/stores/memory.js:199:17)
        at Memory.merge (/.../node_modules/nconf/lib/nconf/stores/memory.js:200:17)
        at Array.every (native)
        at Memory.merge (/.../node_modules/nconf/lib/nconf/stores/memory.js:199:29)
        at common.merge (/.../node_modules/nconf/lib/nconf/common.js:99:13)
        at Array.forEach (native)
        at common.merge (/.../node_modules/nconf/lib/nconf/common.js:98:22)
        at Array.forEach (native)
        at Object.common.merge (/.../node_modules/nconf/lib/nconf/common.js:97:8)
    
    This commit prevents that.
Commits on Dec 21, 2012
  1. @SchoonologyRRL
This page is out of date. Refresh to see the latest.
View
2  lib/nconf/stores/memory.js
@@ -157,7 +157,7 @@ Memory.prototype.merge = function (key, value) {
// If the key is not an `Object` or is an `Array`,
// then simply set it. Merging is for Objects.
//
- if (typeof value !== 'object' || Array.isArray(value)) {
+ if (typeof value !== 'object' || Array.isArray(value) || value === null) {
return this.set(key, value);
}
View
3  test/fixtures/merge/file1.json
@@ -12,5 +12,8 @@
"first": 1,
"second": 2
}
+ },
+ "unicorn": {
+ "exists": true
}
}
View
5 test/fixtures/merge/file2.json
@@ -5,5 +5,6 @@
"something4": true
},
"dates": true,
- "elderberries": true
-}
+ "elderberries": true,
+ "unicorn": null
+}
View
5 test/provider-test.js
@@ -84,6 +84,11 @@ vows.describe('nconf/provider').addBatch({
provider.merge(override);
helpers.assertMerged(null, provider.stores.file.store);
assert.equal(provider.stores.file.store.candy.something, 'file1');
+ },
+ "should merge Objects over null": function (provider) {
+ provider.load();
+ provider.merge(override);
+ assert.equal(provider.stores.file.store.unicorn.exists, true);
}
}
}
Something went wrong with that request. Please try again.