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

chai4 regression: instanceOf validation fails for native HTMLElement instances on IE11 #1000

Closed
AviVahl opened this issue Jun 29, 2017 · 3 comments

Comments

@AviVahl
Copy link

AviVahl commented Jun 29, 2017

Starting with chai v4, instanceOf now validates the target as a constructor function.
The assertion resides in: https://github.com/chaijs/chai/blob/master/lib/chai/core/assertions.js#L1604

    var validInstanceOfTarget = constructor === Object(constructor) && (
        typeof constructor === 'function' ||
        (typeof Symbol !== 'undefined' &&
         typeof Symbol.hasInstance !== 'undefined' &&
         Symbol.hasInstance in constructor)
    );

This validation fails on IE11, IE10, and Safari 7 when the constructor is HTMLInputElement.
I'm guessing it's broken for other HTMLElement constructors, and other (old) environments as well.

Sample code which shows the failure:
https://github.com/wix/wix-react-components/blob/master/test/components/number-input.spec.tsx#L17

Failure log:
https://travis-ci.org/wix/wix-react-components/builds/248446200

Notes:

  • typeof HTMLInputElement === "object" on IE11.
  • <number-input-dom-instance> instanceof HTMLInputElement === true on IE11
  • I'm loading core-js/shim to the client, but I've tested the validation a a clean IE11 (about:blank) and it fails even without the shim.

Same assertion worked with chai 3, as there was no validation.

@meeber
Copy link
Contributor

meeber commented Jul 15, 2017

@AviVahl Thanks for the report! I agree that this is a regression in Chai v4 for at least one browser that we actively support: IE11. It's unfortunate that the spec isn't being followed in this case, but I think Chai needs to honor it.

The PR that added this regression is #899, in response to #893. I suspect it's possible to fix this regression while still resolving #893.

Thoughts @lucasfcosta @vieiralucas @shvaikalesh @keithamus?

@shvaikalesh
Copy link
Contributor

We can either special-case HTML constructors or reconsider wrapping instanceof in try/catch and improving error message.

@AviVahl
Copy link
Author

AviVahl commented Jul 26, 2017

Thanks for the fix guys!

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

3 participants