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

Build failing due to change in error comparison algo #58

Closed
meeber opened this issue Sep 10, 2018 · 2 comments
Closed

Build failing due to change in error comparison algo #58

meeber opened this issue Sep 10, 2018 · 2 comments

Comments

@meeber
Copy link
Contributor

meeber commented Sep 10, 2018

Looks like #57 broke the build for IE 9 & 11 as well as Safari 10.

Apparently IE 9 & 11 add an enumerable description property to Error objects but not to TypeError objects, causing one of the comparison tests to fail. The description property appears to have the same value as message. (If message and description is empty, then IE 9 & 11 also add an enumerable number property, but that doesn't come into play with any of the tests.)

Apparently Safari 10 adds enumerable line, column, and sourceURL properties to all Error objects. This causes all of the tests to fail that expected two Error objects to be equal, since the line and/or column properties will always be different between Error objects.

What a pain. I'm not sure what the right solution is here.

@keithamus
Copy link
Member

I think providing a custom comparator for Error objects that just checks name, message and perahps while we're at it some node-specific ones like code?

We can just add another case statement to http://github.com/chaijs/deep-eql/blob/62e2d5142921ec2b053b3c681487d99f227df02f/index.js#L197-L238

case 'Error':
  return keysEqual(leftHandOperand, rightHandOperand, ['name', 'message', 'code'], options)

@meeber
Copy link
Contributor Author

meeber commented Sep 11, 2018

Fair suggestion, and that's more or less what lodash and Node are doing, but without checking code. Will sleep on it.

meeber added a commit that referenced this issue Sep 15, 2018
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.
keithamus pushed a commit that referenced this issue Oct 5, 2018
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants