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

fix: resolve the "eventTargets" error in JSDOM by dropping "instanceof" #1813

Merged
merged 4 commits into from
Nov 1, 2023

Conversation

mattcosta7
Copy link
Contributor

@mattcosta7 mattcosta7 commented Oct 31, 2023

fixes #1785
fixes #1803 (I think)

Avoid using instanceof to deal with jest/jsdom potentially overriding the global error

I haven't had a chance to try and validate this assumption, but I am assuming that this check is failing on the instanceof Error and therefore rethrowing instead of being caught.

In the past I've seen this with jest overriding globals such that these instanceof checks don't always do what's expected

I validated this locally by pulling mswjs/examples and modifying the installed version in the with-jest node_modules. Once this was done the tests in that repo pass correctly with msw@2+ whereas without this change they fail

@pord911
Copy link

pord911 commented Oct 31, 2023

@mattcosta7
I can roughly confirm your theory. See highlighted local variable value.

image

Happens when TypeError message is thrown within the following environment:

jest 27
jsdom 16
node 18
msw 2.0.0

Found this on jsdom changelog, even though it is for version 18 of jsdom, but here (in my environemnt) it seems to be the same case.

in some environments, particularly jsdom with jest, Error objects
may not be `instanceof Error` despite being similar and used in the
same way.

To avoid these issues we only need to validate the shape and properties
that might be thrown. In some cases these could false positive, however
it's less likely that they'll false positive, than it is that jest/jsdom
will throw these objects _like_ the errors they represent (guaranteed)
* instances of `Error` or its subclasses, despite being similar
* to them.
*/
export function isNodeExceptionLike(
Copy link
Contributor Author

@mattcosta7 mattcosta7 Nov 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed this function and added some docs that it's very similar to a NodeException but isn't guaranteed to be and instanceof it

@mattcosta7 mattcosta7 changed the title simplify check to avoid instanceof Avoid instanceof in isNodeException check. Rename that function Nov 1, 2023
@kettanaito
Copy link
Member

Thanks for fixing this, @mattcosta7. Yet another day, yet another JSDOM shenanigans. I will give this a look and let's have this merged.

@kettanaito kettanaito changed the title Avoid instanceof in isNodeException check. Rename that function fix: resolve the "eventTargets" error in JSDOM by dropping "instanceof" Nov 1, 2023
Copy link
Member

@kettanaito kettanaito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great.

@kettanaito kettanaito merged commit f6e5112 into main Nov 1, 2023
9 checks passed
@kettanaito kettanaito deleted the mattcosta7-simplify-node-exception-check branch November 1, 2023 11:25
@sauldeleon
Copy link

Thanks @mattcosta7 @kettanaito !!

@kettanaito
Copy link
Member

Released: v2.0.2 🎉

This has been released in v2.0.2!

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants