Skip to content

Commit

Permalink
feat: change error comparison algorithm again (#59)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: As described in GH Issue #58, the previous change
to the error comparison algorithm isn't compatible with IE and
Safari due to those browsers adding extra enumerable properties
onto `Error` objects. This commit causes `Error` objects to only
include their `name`, `message`, and `code` properties in the
comparison, regardless of enumerability.
  • Loading branch information
meeber authored and keithamus committed Oct 5, 2018
1 parent 62e2d51 commit 50f229d
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 27 deletions.
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,13 @@ The primary export of `deep-eql` is function that can be given two objects to co
- All own and inherited enumerable properties are considered:
- `eql(Object.create({ foo: { a: 1 } }), Object.create({ foo: { a: 1 } })).should.be.true;`
- `eql(Object.create({ foo: { a: 1 } }), Object.create({ foo: { a: 2 } })).should.be.false;`
- When comparing `Error` objects, `name` and `message` properties are also considered, regardless of enumerability:
- When comparing `Error` objects, only `name`, `message`, and `code` properties are considered, regardless of enumerability:
- `eql(Error('foo'), Error('foo')).should.be.true;`
- `eql(Error('foo'), Error('bar')).should.be.false;`
- `eql(Error('foo'), TypeError('foo')).should.be.false;`
- `eql(Object.assign(Error('foo'), { code: 42 }), Object.assign(Error('foo'), { code: 42 })).should.be.true;`
- `eql(Object.assign(Error('foo'), { code: 42 }), Object.assign(Error('foo'), { code: 13 })).should.be.false;`
- `eql(Object.assign(Error('foo'), { otherProp: 42 }), Object.assign(Error('foo'), { otherProp: 13 })).should.be.true;`
- Arguments are not Arrays:
- `eql([], arguments).should.be.false;`
- `eql([], Array.prototype.slice.call(arguments)).should.be.true;`
29 changes: 5 additions & 24 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ function extensiveDeepEqualByType(leftHandOperand, rightHandOperand, leftHandTyp
case 'WeakMap':
case 'WeakSet':
return leftHandOperand === rightHandOperand;
case 'Error':
return keysEqual(leftHandOperand, rightHandOperand, [ 'name', 'message', 'code' ], options);
case 'Arguments':
case 'Int8Array':
case 'Uint8Array':
Expand All @@ -233,7 +235,7 @@ function extensiveDeepEqualByType(leftHandOperand, rightHandOperand, leftHandTyp
case 'Map':
return entriesEqual(leftHandOperand, rightHandOperand, options);
default:
return objectEqual(leftHandOperand, rightHandOperand, leftHandType, options);
return objectEqual(leftHandOperand, rightHandOperand, options);
}
}

Expand Down Expand Up @@ -377,21 +379,6 @@ function getEnumerableKeys(target) {
return keys;
}

/*!
* Adds `Error`-specific keys to an array of object keys. We want to include these keys when comparing `Error` objects
* despite these keys being non-enumerable by default (and thus not added by `getEnumerableKeys`).
*
* @param {Array} keys An array of keys
*/
function addExtraErrorKeys(keys) {
if (keys.indexOf('name') === -1) { // eslint-disable-line no-magic-numbers
keys.push('name');
}
if (keys.indexOf('message') === -1) { // eslint-disable-line no-magic-numbers
keys.push('message');
}
}

/*!
* Determines if two objects have matching values, given a set of keys. Defers to deepEqual for the equality check of
* each key. If any value of the given key is not equal, the function will return false (early).
Expand All @@ -417,22 +404,16 @@ function keysEqual(leftHandOperand, rightHandOperand, keys, options) {

/*!
* Recursively check the equality of two Objects. Once basic sameness has been established it will defer to `deepEqual`
* for each enumerable key in the object. For `Error` objects, also compare `name` and `message` keys, regardless of
* enumerability.
* for each enumerable key in the object.
*
* @param {Mixed} leftHandOperand
* @param {Mixed} rightHandOperand
* @param {String} type of leftHandOperand
* @param {Object} [options] (Optional)
* @return {Boolean} result
*/
function objectEqual(leftHandOperand, rightHandOperand, leftHandType, options) {
function objectEqual(leftHandOperand, rightHandOperand, options) {
var leftHandKeys = getEnumerableKeys(leftHandOperand);
var rightHandKeys = getEnumerableKeys(rightHandOperand);
if (leftHandType === 'Error') {
addExtraErrorKeys(leftHandKeys);
addExtraErrorKeys(rightHandKeys);
}
if (leftHandKeys.length && leftHandKeys.length === rightHandKeys.length) {
leftHandKeys.sort();
rightHandKeys.sort();
Expand Down
13 changes: 11 additions & 2 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ describe('Generic', function () {
'eql(Error("foo"), Object.assign(Error("foo"), { name: "TypeError" })) === false');
});

it('returns true for errors with same custom property', function () {
it('returns true for errors with same code', function () {
var err1 = Error('foo');
var err2 = Error('foo');
err1.code = 42;
Expand All @@ -423,7 +423,7 @@ describe('Generic', function () {
'eql(Object.assign(Error("foo"), { code: 42 }), Object.assign(Error("foo"), { code: 42 }))');
});

it('returns false for errors with different custom property', function () {
it('returns false for errors with different code', function () {
var err1 = Error('foo');
var err2 = Error('foo');
err1.code = 42;
Expand All @@ -432,6 +432,15 @@ describe('Generic', function () {
'eql(Object.assign(new Error("foo"), { code: 42 }), Object.assign(new Error("foo"), { code: 13 })) === false');
});

it('returns true for errors with same name and message despite different otherProp', function () {
var err1 = Error('foo');
var err2 = Error('foo');
err1.otherProp = 42;
err2.otherProp = 13;
assert(eql(err1, err2),
'eql(Object.assign(Error("foo"), { otherProp: 42 }), Object.assign(Error("foo"), { otherProp: 13 }))');
});

});

});
Expand Down

0 comments on commit 50f229d

Please sign in to comment.