Skip to content

Commit

Permalink
Fix handling of non-enumerable properties
Browse files Browse the repository at this point in the history
- Include non-enumberable properties in clone/merge by default
- Remove hoistProto option
- Add hoistEnumerable option
- Update docs
- Update tests
  • Loading branch information
jhildenbiddle committed Jan 12, 2024
1 parent 79388bb commit 342e38a
Show file tree
Hide file tree
Showing 5 changed files with 207 additions and 248 deletions.
16 changes: 6 additions & 10 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ console.log(mergedObj); // { b: [2, 3] }

**Prototype**

- [hoistProto](#hoistproto)
- [hoistEnumerable](#hoistenumerable)

**Callbacks**

Expand Down Expand Up @@ -494,29 +494,25 @@ console.log(mergedAscending); // { a: [1, 2, 3, 4, 5, 6] }
console.log(mergedDescending); // { a: [6, 5, 4, 3, 2, 1] }
```

### hoistProto()
### hoistEnumerable()

Clone prototype properties as direct properties of merged/cloned object.
Clone enumerable prototype properties as direct properties of merged/cloned object.

- Type: `Boolean`
- Default: `false`

```js
const obj = { a: 1 };
const objProto = Object.getPrototypeOf(obj);

console.log(obj); // { a: 1 }
console.log(obj.b); // 2
console.log(objProto); // { b: 2 }
console.log(obj.hasOwnProperty('a')); // true
console.log(obj.hasOwnProperty('b')); // false
console.log(Object.getPrototypeOf(obj)); // { b: 2 }

const clonedObj = mergician({
hoistProto: true
hoistEnumerable: true
})({}, obj);

console.log(clonedObj); // { a: 1, b: 2 }
console.log(clonedObj.hasOwnProperty('b')); // true
console.log(Object.getPrototypeOf(clonedObj)); // {}
```

### filter()
Expand Down
102 changes: 58 additions & 44 deletions src/index.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const defaults = {
dedupArrays: false,
sortArrays: false,
// Prototype
hoistProto: false,
hoistEnumerable: false,
// Callbacks
filter: Function.prototype,
beforeEach: Function.prototype,
Expand Down Expand Up @@ -63,7 +63,7 @@ const defaults = {
* dedupArrays: false,
* sortArrays: false,
* // Prototype
* hoistProto: false,
* hoistEnumerable: false,
* // Callbacks
* filter({ depth, key, srcObj, srcVal, targetObj, targetVal }) {},
* beforeEach({ depth, key, srcObj, srcVal, targetObj, targetVal }) {},
Expand Down Expand Up @@ -95,8 +95,8 @@ const defaults = {
* values in new merged object
* @param {boolean|function} [options.sortArrays = false] - Sort array values in
* new merged object
* @param {boolean} [options.hoistProto = false] - Clone prototype properties as
* direct properties of merged/cloned object
* @param {boolean} [options.hoistEnumerable = false] - Clone enumerable
* prototype properties as direct properties of merged/cloned object
* @param {function} [options.filter] - Callback used to conditionally merge or
* skip a property. Return a "truthy" value to merge or a "falsy" value to skip.
* Return no value to proceed according to other option values.
Expand Down Expand Up @@ -128,7 +128,7 @@ function mergician(...optionsOrObjects) {
let mergeDepth = 0;

function _getObjectKeys(obj) {
return getObjectKeys(obj, settings.hoistProto);
return getObjectKeys(obj, settings.hoistEnumerable);
}

function _mergician(...objects) {
Expand Down Expand Up @@ -170,27 +170,18 @@ function mergician(...optionsOrObjects) {
const key = keys[i];
const targetVal = targetObj[key];

let isReturnVal = false;
let mergeVal;

if (key in srcObj === false) {
continue;
}

try {
mergeVal = srcObj[key];
}
catch(err) {
console.error(err);
continue;
}
let isReturnVal = false;
let mergeVal = srcObj[key];

const srcDescriptor = Object.getOwnPropertyDescriptor(srcObj, key);
const isSetterOnly = srcDescriptor && typeof srcDescriptor.set === 'function' && typeof srcDescriptor.get !== 'function';

if (isSetterOnly) {
if (!settings.skipSetters) {
srcDescriptor.configurable = true;
Object.defineProperty(targetObj, key, srcDescriptor);
}

Expand Down Expand Up @@ -331,44 +322,67 @@ function mergician(...optionsOrObjects) {
}
}

let mergeDescriptor;

if (isReturnVal) {
if (isPropDescriptor(mergeVal)) {
// Descriptor properties 'configurable', 'enumerable',
// and 'writable' default to 'false' when using
// Object.defineProperty() but to 'true' when using the
// assignment operator (obj.a = 1). It is therefore
// necessary to set 'configurable' and 'writable'
// properties to 'true' to ensure subsequent merge tasks
// complete successfully.
mergeVal.configurable = true;
mergeVal.enumerable = !('enumerable' in mergeVal) ? true : mergeVal.enumerable;

if ('value' in mergeVal && !('writable' in mergeVal)) {
mergeVal.writable = true;
}
mergeDescriptor = isPropDescriptor(mergeVal) ? mergeVal : {
configurable: true,
enumerable: true,
value: mergeVal,
writable: true,
};

Object.defineProperty(targetObj, key, mergeDescriptor);
continue;
}

Object.defineProperty(targetObj, key, mergeVal);
}
else {
targetObj[key] = mergeVal;
}
if (isPropDescriptor(mergeVal)) {
mergeDescriptor = { ...mergeVal };
mergeVal = mergeDescriptor.get ? mergeDescriptor.get() : mergeDescriptor.value;
}
else {
const mergeDescriptor = Object.getOwnPropertyDescriptor(srcObj, key);
mergeDescriptor = {
configurable: true,
enumerable: true,
};
}

if (srcDescriptor) {
// eslint-disable-next-line no-unused-vars
const { configurable, enumerable, get, set, writable} = srcDescriptor;

Object.assign(mergeDescriptor, {
configurable,
enumerable,
});

if (mergeDescriptor && typeof mergeDescriptor.get === 'function' && !settings.invokeGetters) {
if (settings.skipSetters) {
mergeDescriptor.set = undefined;
// Invoke getters
if (typeof get === 'function') {
if (settings.invokeGetters) {
mergeDescriptor.value = get();
}
else {
mergeDescriptor.get = get;
}
}

// Set as configurable for subsequent merges
mergeDescriptor.configurable = true;
Object.defineProperty(targetObj, key, mergeDescriptor);
// Skip setters
if (typeof set === 'function' && !settings.skipSetters && !Object.hasOwnProperty.call(mergeDescriptor, 'value')) {
mergeDescriptor.set = set;
}
else {
targetObj[key] = mergeVal;

// Set writable property if not accessors are defined
if (!mergeDescriptor.get && !mergeDescriptor.set) {
mergeDescriptor.writable = Boolean(writable);
}
}

if (!mergeDescriptor.get && !mergeDescriptor.set && !mergeDescriptor.value) {
mergeDescriptor.value = mergeVal;
mergeDescriptor.writable = true;
}

Object.defineProperty(targetObj, key, mergeDescriptor);
}

return targetObj;
Expand Down
32 changes: 13 additions & 19 deletions src/util.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -88,33 +88,27 @@ function getNotInAll(...arrays) {
}

/**
* Returns array of an object's own keys and (optionally) those from the
* object's prototype chain.
* Returns array of an object's own keys and (optionally) the enumerable keys
* from the object's prototype chain.
*
* @example
* getObjectKeys({ a: 1 }); // ['a']
* getObjectKeys({ a: 1 }, true); // ['a', 'b', 'c', ...]
*
* @param {object} obj - Object to parse
* @param {boolean} includeProto include key/values from prototype
* @param {boolean} enumerable include all enumerable keys
* @return {array} List of keys
*/
function getObjectKeys(obj, includeProto = false) {
const keys = [...Object.getOwnPropertyNames(obj)];

if (includeProto) {
const newProto = Object.getPrototypeOf({});
const newProtoDescriptors = Object.getOwnPropertyDescriptors(newProto);
const objProto = Object.getPrototypeOf(obj);
const objProtoDescriptors = Object.getOwnPropertyDescriptors(objProto);
const objProtoKeys = Object.getOwnPropertyNames(objProtoDescriptors);

// Add unique and skip standard prototype keys
objProtoKeys.forEach(key => {
if (key !== '__proto__' && !(key in newProtoDescriptors)) {
keys.push(key);
}
});
function getObjectKeys(obj, enumerable = false) {
let keys = [];

if (enumerable) {
for (const key in obj) {
keys.push(key);
}
}
else {
keys = Object.getOwnPropertyNames(obj);
}

return keys;
Expand Down
Loading

0 comments on commit 342e38a

Please sign in to comment.