Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Safer and faster shallow handling #343

Merged
merged 3 commits into from
Jan 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 47 additions & 9 deletions lib/applyToDefaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand Down Expand Up @@ -39,17 +39,55 @@ 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();

const copy = Clone(defaults, { shallow: keys });
for (let key of keys) {
key = Array.isArray(key) ? key : key.split('.'); // Pre-split optimization

if (source === true) { // If source is set to true, use defaults
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;
};
17 changes: 13 additions & 4 deletions lib/clone.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';

const Reach = require('./reach');
const Types = require('./types');
const Utils = require('./utils');

Expand Down Expand Up @@ -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);
};
4 changes: 4 additions & 0 deletions lib/merge.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)) ||
Expand Down
45 changes: 0 additions & 45 deletions lib/utils.js
Original file line number Diff line number Diff line change
@@ -1,54 +1,9 @@
'use strict';

const Reach = require('./reach');


const internals = {};


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];
}
};
99 changes: 95 additions & 4 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()', () => {
Expand Down Expand Up @@ -976,8 +989,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);
Expand All @@ -986,12 +999,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);
Expand All @@ -1000,6 +1014,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', () => {
Expand Down Expand Up @@ -1408,6 +1423,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(() => {
Expand Down Expand Up @@ -1439,6 +1478,58 @@ 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]);
});

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()', () => {
Expand Down