Skip to content

Commit

Permalink
feat: change error comparison algorithm (#57)
Browse files Browse the repository at this point in the history
* chore: drop support for Node versions older than 6

BREAKING CHANGE: This commit drops support for versions of Node
that are no longer maintained.

* feat: change error comparison algorithm

BREAKING CHANGE: Previously, `Error` objects were compared using
strict equality. This commit causes `Error` objects to be compared
using deep equality instead, but treats them as a special case by
including their `name` and `message` properties in the comparison,
regardless of enumerability.
  • Loading branch information
meeber authored and keithamus committed Sep 10, 2018
1 parent 04d6da6 commit 62e2d51
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 15 deletions.
5 changes: 3 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ cache:
directories:
- node_modules
node_js:
- 4 # to be removed 2018-04-01
- 6 # to be removed 2019-04-01
- 6 # to be removed 2019-04-30
- 8 # to be removed 2019-12-31
- 10 # to be removed 2021-04-30
- lts/* # safety net; don't remove
- node # safety net; don't remove
before_install:
Expand Down
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,11 @@ 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:
- `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: 13 })).should.be.false;`
- Arguments are not Arrays:
- `eql([], arguments).should.be.false;`
- `eql([], Array.prototype.slice.call(arguments)).should.be.true;`
- Error objects are compared by reference (see https://github.com/chaijs/chai/issues/608):
- `eql(new Error('msg'), new Error('msg')).should.be.false;`
- `var err = new Error('msg'); eql(err, err).should.be.true;`
29 changes: 24 additions & 5 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ function extensiveDeepEqualByType(leftHandOperand, rightHandOperand, leftHandTyp
case 'function':
case 'WeakMap':
case 'WeakSet':
case 'Error':
return leftHandOperand === rightHandOperand;
case 'Arguments':
case 'Int8Array':
Expand All @@ -234,7 +233,7 @@ function extensiveDeepEqualByType(leftHandOperand, rightHandOperand, leftHandTyp
case 'Map':
return entriesEqual(leftHandOperand, rightHandOperand, options);
default:
return objectEqual(leftHandOperand, rightHandOperand, options);
return objectEqual(leftHandOperand, rightHandOperand, leftHandType, options);
}
}

Expand Down Expand Up @@ -378,6 +377,21 @@ 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 @@ -403,17 +417,22 @@ 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 each enumerable key in the object. For `Error` objects, also compare `name` and `message` keys, regardless of
* enumerability.
*
* @param {Mixed} leftHandOperand
* @param {Mixed} rightHandOperand
* @param {String} type of leftHandOperand
* @param {Object} [options] (Optional)
* @return {Boolean} result
*/

function objectEqual(leftHandOperand, rightHandOperand, options) {
function objectEqual(leftHandOperand, rightHandOperand, leftHandType, 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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,6 @@
"watchify": "^3.7.0"
},
"engines": {
"node": ">=0.12"
"node": ">=6"
}
}
52 changes: 48 additions & 4 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -379,13 +379,57 @@ describe('Generic', function () {
describe('errors', function () {

it('returns true for same errors', function () {
var error = new Error('foo');
var error = Error('foo');
assert(eql(error, error), 'eql(error, error)');
});

it('returns false for different errors', function () {
assert(eql(new Error('foo'), new Error('foo')) === false,
'eql(new Error("foo"), new Error("foo")) === false');
it('returns true for errors with same name and message', function () {
assert(eql(Error('foo'), Error('foo')),
'eql(Error("foo"), Error("foo"))');
});

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

it('returns false for errors with same name but different messages', function () {
assert(eql(Error('foo'), Error('bar')) === false,
'eql(Error("foo"), Error("bar")) === false');
});

it('returns false for errors with same message but different names', function () {
assert(eql(Error('foo'), TypeError('foo')) === false,
'eql(Error("foo"), TypeError("foo")) === false');
});

it('returns false for errors with same message but different names despite same constructors', function () {
var err1 = Error('foo');
var err2 = Error('foo');
err2.name = 'TypeError';
assert(eql(err1, err2) === false,
'eql(Error("foo"), Object.assign(Error("foo"), { name: "TypeError" })) === false');
});

it('returns true for errors with same custom property', function () {
var err1 = Error('foo');
var err2 = Error('foo');
err1.code = 42;
err2.code = 42;
assert(eql(err1, err2),
'eql(Object.assign(Error("foo"), { code: 42 }), Object.assign(Error("foo"), { code: 42 }))');
});

it('returns false for errors with different custom property', function () {
var err1 = Error('foo');
var err2 = Error('foo');
err1.code = 42;
err2.code = 13;
assert(eql(err1, err2) === false,
'eql(Object.assign(new Error("foo"), { code: 42 }), Object.assign(new Error("foo"), { code: 13 })) === false');
});

});
Expand Down

0 comments on commit 62e2d51

Please sign in to comment.