-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
add mutation observer polyfill for jest test env #20996
Conversation
💔 Build Failed |
…mutation_observer
💔 Build Failed |
// https://github.com/aurelia/pal-nodejs/blob/master/src/polyfills/mutation-observer.ts | ||
|
||
/* | ||
The MIT License (MIT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are going to need a mutation observer polyfill for the browser too no? Is there a reason we need one for jest specifically? Why not use one from npm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A polyfill is not needed for the browser since MutationObservser is implemented in IE 11 and has had support in Firefox, Chrome, and Safari for a long time.
We need a polyfill for jest tests because jsdom does not implement MutationObservser and therefore any tests that render a component using MutationObservser will fail to render.
I was using the MutationObservser polyfill that EUI is using. I will see if an npm version exists that we can use.
💔 Build Failed |
💚 Build Succeeded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
src/dev/jest/setup/polyfills.js
Outdated
*/ | ||
|
||
// bluebird < 3.5.0 does not work with MutationObserver polyfill | ||
// when MutationObserver exists, bluebird avoids using node's builtin async schedulers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you link to docs or something for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no docs to link to. @chandlerprall found this while digging around in their code base
src/dev/jest/setup/polyfills.js
Outdated
* under the License. | ||
*/ | ||
|
||
// bluebird < 3.5.0 does not work with MutationObserver polyfill |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, the update that works alongside MutationObserver's polyfill happened in v3.3.5; I don't think that's vital information but wanted to call it out anyway.
💔 Build Failed |
💚 Build Succeeded |
…mutation_observer
💚 Build Succeeded |
* add mutation observer polyfill for jest test env * use polyfill from npm * set bluebird scheduler * use correct version in comment about bluebird
* add mutation observer polyfill for jest test env * use polyfill from npm * set bluebird scheduler * use correct version in comment about bluebird
EUI 3.1.0 updates the EuiPopover component to use MutationObservser. Any jest tests that uses this component and render with
mount
will fail because jsdom does not implement MutationObservser.This PR sets up a MutationObserver polyfill for the jest runtime environment. A polyfill is not needed for the browser since MutationObservser is implemented in IE 11 and has had support in Firefox, Chrome, and Safari for a long time.
bluebird
does not play nice with the MutationObserver polyfill.