From 689e7ec5541fc31f51681b28768e7d081175ab16 Mon Sep 17 00:00:00 2001 From: Gil Pedersen Date: Wed, 16 Oct 2019 18:49:05 +0200 Subject: [PATCH 1/3] Add more tests --- test/index.js | 68 ++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 64 insertions(+), 4 deletions(-) diff --git a/test/index.js b/test/index.js index 516eb846..ffeceffb 100755 --- a/test/index.js +++ b/test/index.js @@ -976,8 +976,8 @@ describe('merge()', () => { it('overrides values in target', () => { - const a = { x: 1, y: 2, z: 3, v: 5, t: 'test', m: 'abc' }; - const b = { x: null, z: 4, v: 0, t: { u: 6 }, m: '123' }; + const a = { x: 1, y: 2, z: 3, v: 5, t: 'test', s: 1, m: 'abc' }; + const b = { x: null, z: 4, v: 0, t: { u: 6 }, s: undefined, m: '123' }; const c = Hoek.merge(a, b); expect(c.x).to.equal(null); @@ -986,12 +986,13 @@ describe('merge()', () => { expect(c.v).to.equal(0); expect(c.m).to.equal('123'); expect(c.t).to.equal({ u: 6 }); + expect(c.s).to.equal(undefined); }); it('overrides values in target (flip)', () => { - const a = { x: 1, y: 2, z: 3, v: 5, t: 'test', m: 'abc' }; - const b = { x: null, z: 4, v: 0, t: { u: 6 }, m: '123' }; + const a = { x: 1, y: 2, z: 3, v: 5, t: 'test', s: 1, m: 'abc' }; + const b = { x: null, z: 4, v: 0, t: { u: 6 }, s: undefined, m: '123' }; const d = Hoek.merge(b, a); expect(d.x).to.equal(1); @@ -1000,6 +1001,7 @@ describe('merge()', () => { expect(d.v).to.equal(5); expect(d.m).to.equal('abc'); expect(d.t).to.equal('test'); + expect(d.s).to.equal(1); }); it('retains Date properties', () => { @@ -1408,6 +1410,30 @@ describe('applyToDefaults()', () => { expect(merged).to.equal(null); }); + it('handles missing shallow key in defaults', () => { + + const defaults = { + a: { + b: 1 + } + }; + + const options = { + a: { + b: 4 + }, + c: { + d: 2 + } + }; + + const merged = Hoek.applyToDefaults(defaults, options, { shallow: ['c'] }); + expect(merged).to.equal({ a: { b: 4 }, c: { d: 2 } }); + expect(merged.c).to.shallow.equal(options.c); + + expect(Hoek.applyToDefaults(defaults, true, { shallow: ['c'] })).to.equal({ a: { b: 1 } }); + }); + it('throws on missing defaults', () => { expect(() => { @@ -1439,6 +1465,40 @@ describe('applyToDefaults()', () => { Hoek.applyToDefaults({}, true, { shallow: 123 }); }).to.throw('Invalid keys'); }); + + it('handles array keys', () => { + + const sym = Symbol(); + + const defaults = { + a: { + b: 5, + e: 3 + }, + c: { + d: 7, + [sym]: { + f: 9 + } + } + }; + + const options = { + a: { + b: 4 + }, + c: { + d: 6, + [sym]: { + g: 1 + } + } + }; + + const merged = Hoek.applyToDefaults(defaults, options, { shallow: [['c', sym]] }); + expect(merged).to.equal({ a: { b: 4, e: 3 }, c: { d: 6, [sym]: { g: 1 } } }); + expect(merged.c[sym]).to.shallow.equal(options.c[sym]); + }); }); describe('deepEqual()', () => { From 0c4938e5508f6bd9241f7671a37338497927db14 Mon Sep 17 00:00:00 2001 From: Gil Pedersen Date: Wed, 16 Oct 2019 19:29:55 +0200 Subject: [PATCH 2/3] Less intrusive shallow clone and applyToDefaults --- lib/applyToDefaults.js | 55 ++++++++++++++++++++++++++++++++++++------ lib/clone.js | 17 ++++++++++--- lib/merge.js | 4 +++ lib/utils.js | 45 ---------------------------------- test/index.js | 31 ++++++++++++++++++++++++ 5 files changed, 96 insertions(+), 56 deletions(-) diff --git a/lib/applyToDefaults.js b/lib/applyToDefaults.js index 79981a47..bb61f3af 100755 --- a/lib/applyToDefaults.js +++ b/lib/applyToDefaults.js @@ -3,7 +3,7 @@ const Assert = require('./assert'); const Clone = require('./clone'); const Merge = require('./merge'); -const Utils = require('./utils'); +const Reach = require('./reach'); const internals = {}; @@ -42,14 +42,55 @@ internals.applyToDefaultsWithShallow = function (defaults, source, options) { options = Object.assign({}, options); options.shallow = false; - const copy = Clone(defaults, { shallow: keys }); + const seen = new Map(); + const merge = source === true ? null : new Set(); - if (source === true) { // If source is set to true, use defaults + for (let key of keys) { + key = Array.isArray(key) ? key : key.split('.'); // Pre-split optimization + + const ref = Reach(defaults, key); + if (ref && typeof ref === 'object') { + seen.set(ref, merge ? Reach(source, key) : ref); + } + else if (merge) { + merge.add(key); + } + } + + const copy = Clone(defaults, {}, seen); + + if (!merge) { return copy; } - const storage = Utils.store(source, keys); // Move shallow copy items to storage - Merge(copy, source, { mergeArrays: false, nullOverride: false }); // Deep copy the rest - Utils.restore(copy, source, storage); // Shallow copy the stored items and restore - return copy; + for (const key of merge) { + internals.reachCopy(copy, source, key); + } + + return Merge(copy, source, { mergeArrays: false, nullOverride: false }); +}; + + +internals.reachCopy = function (dst, src, path) { + + for (const segment of path) { + if (!(segment in src)) { + return; + } + + src = src[segment]; + } + + const value = src; + let ref = dst; + for (let i = 0; i < path.length - 1; ++i) { + const segment = path[i]; + if (typeof ref[segment] !== 'object') { + ref[segment] = {}; + } + + ref = ref[segment]; + } + + ref[path[path.length - 1]] = value; }; diff --git a/lib/clone.js b/lib/clone.js index 0fbe8359..b5cacc12 100755 --- a/lib/clone.js +++ b/lib/clone.js @@ -1,5 +1,6 @@ 'use strict'; +const Reach = require('./reach'); const Types = require('./types'); const Utils = require('./utils'); @@ -135,8 +136,16 @@ internals.cloneWithShallow = function (source, options) { options = Object.assign({}, options); options.shallow = false; - const storage = Utils.store(source, keys); // Move shallow copy items to storage - const copy = internals.clone(source, options); // Deep copy the rest - Utils.restore(copy, source, storage); // Shallow copy the stored items and restore - return copy; + const seen = new Map(); + + for (const key of keys) { + const ref = Reach(source, key); + if (typeof ref === 'object' || + typeof ref === 'function') { + + seen.set(ref, ref); + } + } + + return internals.clone(source, options, seen); }; diff --git a/lib/merge.js b/lib/merge.js index abce9924..47a1e1e9 100755 --- a/lib/merge.js +++ b/lib/merge.js @@ -45,6 +45,10 @@ module.exports = internals.merge = function (target, source, options) { if (value && typeof value === 'object') { + if (target[key] === value) { + continue; // Can occur for shallow merges + } + if (!target[key] || typeof target[key] !== 'object' || (Array.isArray(target[key]) !== Array.isArray(value)) || diff --git a/lib/utils.js b/lib/utils.js index bf1758a0..bab1e8c4 100755 --- a/lib/utils.js +++ b/lib/utils.js @@ -1,8 +1,5 @@ 'use strict'; -const Reach = require('./reach'); - - const internals = {}; @@ -10,45 +7,3 @@ exports.keys = function (obj, options = {}) { return options.symbols !== false ? Reflect.ownKeys(obj) : Object.getOwnPropertyNames(obj); // Defaults to true }; - - -exports.store = function (source, keys) { - - const storage = new Map(); - for (let i = 0; i < keys.length; ++i) { - const key = keys[i]; - const value = Reach(source, key); - if (typeof value === 'object' || - typeof value === 'function') { - - storage.set(key, value); - internals.reachSet(source, key, undefined); - } - } - - return storage; -}; - - -exports.restore = function (copy, source, storage) { - - for (const [key, value] of storage) { - internals.reachSet(copy, key, value); - internals.reachSet(source, key, value); - } -}; - - -internals.reachSet = function (obj, key, value) { - - const path = Array.isArray(key) ? key : key.split('.'); - let ref = obj; - for (let i = 0; i < path.length; ++i) { - const segment = path[i]; - if (i + 1 === path.length) { - ref[segment] = value; - } - - ref = ref[segment]; - } -}; diff --git a/test/index.js b/test/index.js index ffeceffb..7cfc0461 100755 --- a/test/index.js +++ b/test/index.js @@ -762,6 +762,19 @@ describe('clone()', () => { expect(copy.a).to.shallow.equal(obj.a); expect(copy.x).to.shallow.equal(obj); }); + + it('does not invoke setter when shallow cloning', () => { + + const obj = {}; + + Object.defineProperty(obj, 'a', { enumerable: true, value: {} }); + Object.defineProperty(obj, 'b', { enumerable: true, value: {} }); + + const copy = Hoek.clone(obj, { shallow: ['a'] }); + + expect(copy).equal({ a: {}, b: {} }); + expect(copy.a).to.shallow.equal(obj.a); + }); }); describe('merge()', () => { @@ -1499,6 +1512,24 @@ describe('applyToDefaults()', () => { expect(merged).to.equal({ a: { b: 4, e: 3 }, c: { d: 6, [sym]: { g: 1 } } }); expect(merged.c[sym]).to.shallow.equal(options.c[sym]); }); + + it('does not modify shallow entries in source', () => { + + const defaults = { + a: { + b: 5 + } + }; + + const source = {}; + + Object.defineProperty(source, 'a', { value: { b: 4 } }); + + const merged = Hoek.applyToDefaults(defaults, source, { shallow: ['a'] }); + expect(merged).to.equal({ a: { b: 4 } }); + expect(merged.a).to.equal(source.a); + expect(merged.a).to.not.equal(defaults.a); + }); }); describe('deepEqual()', () => { From 09450e11cad2aae0437a2722c98eb3a2d112dd2e Mon Sep 17 00:00:00 2001 From: Gil Pedersen Date: Thu, 17 Oct 2019 12:28:29 +0200 Subject: [PATCH 3/3] Remove unused code --- lib/applyToDefaults.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/applyToDefaults.js b/lib/applyToDefaults.js index bb61f3af..a43237aa 100755 --- a/lib/applyToDefaults.js +++ b/lib/applyToDefaults.js @@ -39,9 +39,6 @@ internals.applyToDefaultsWithShallow = function (defaults, source, options) { const keys = options.shallow; Assert(Array.isArray(keys), 'Invalid keys'); - options = Object.assign({}, options); - options.shallow = false; - const seen = new Map(); const merge = source === true ? null : new Set();