feat: change error comparison algorithm again (#59)

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.

Author: meeber

README.md

@@ -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;`

index.js

@@ -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':
@@ -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);
   }
 }
 
@@ -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).
@@ -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();

test/index.js

@@ -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;
@@ -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;
@@ -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 }))');
+    });
+
   });
 
 });