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

update jsdom to 12.x #7122

Closed
wants to merge 2 commits into from
Closed

update jsdom to 12.x #7122

wants to merge 2 commits into from

Conversation

43081j
Copy link

@43081j 43081j commented Oct 8, 2018

jsdom 12.x introduces shadow DOM support (finally).

this means we can potentially now test web components and what not, using jest.

edit:

it seems this means we wouldn't be able to support node <8 anymore as jsdom has a requirement on it.

what do you suggest we do about that?

@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@SimenB
Copy link
Member

SimenB commented Oct 8, 2018

it seems this means we wouldn't be able to support node <8 anymore as jsdom has a requirement on it.

Unfortunately, that means we won't land the upgrade for some time. In the meantime you can create your own environment using jsdom 12, similar to what was done before we upgraded to JSDOM 10: https://www.npmjs.com/package/jest-environment-jsdom-latest (happy to link to it from the docs)

@julienw
Copy link

julienw commented Jan 29, 2019

@SimenB Jest 24 seems to depend on sane which doesn't support node < 6, so maybe we'll be able to upgrade jsdom a bit (maybe not to the latest version though)

@thymikee
Copy link
Collaborator

Jest doesn't support Node.js < 6 either and it matches sane@3 we use (and sane@4 as well). So for the time being we need to stick to jsdom@^11 in the core.

@SimenB
Copy link
Member

SimenB commented Jan 29, 2019

wardpeet pushed a commit to gatsbyjs/gatsby that referenced this pull request Mar 27, 2019
## Description

Certain CSS styles have been failing to appear in Jest tests due to an outdated JSDom(and some it's dependencies, one being updated with bugfix in Feb 2019). This was causing false positives for `gatsby-image` tests for a while.

Nothing major, just some CSS props weren't appearing in tests when they technically should have been.

## Details

[My PR](#12468) is failing in the CI but was passing tests locally.

I had been running tests on the `gatsby-image` package on it's own, which uses `react-testing-library: ^5.0.0`, and it failed against the snapshot test. Updated snapshots failed when my PR ran on CircleCI. The main repo root yarn.lock file has `react-testing-library: ^5.2.1` locked down(retrieves 5.2.1 instead of 5.9.0, although is a v6 release now). 

Correcting this didn't help for passing the test suite. [`cssstyle: 1.2.0`](jsdom/cssstyle#74) fixes the [problems with certain styles](jsdom/jsdom#1965 (comment)) being [ignored](testing-library/react-testing-library#214. `transitionDelay`, and certain color values for `backgroundColor`, "red" works, but not "lightgray"). That however [needs `jsdom: ^12.0.0`](jsdom/jsdom@ed11465#diff-b9cfc7f2cdf78a7f4b91a753d10865a2), but [Jest refuses to update as support for Node <8 is dropped](jestjs/jest#8016 (comment))..

I've added a workaround until Jest supports a newer major version of JSDom. Using [`jest-environment-jsdom-fourteen`](https://github.com/ianschmitz/jest-environment-jsdom-fourteen) is the [advised approach](jestjs/jest#7122 (comment)) for the meantime, it'll only impact users running Jest test suite on Node 6, that should be fine for Gatsby? (Node 6 CI test seems to pass)

Only `gatsby-image` tests needed updates. [Node 6 will be EoL in May](https://github.com/nodejs/Release#release-schedule), no idea when Jest will bump JSDom from then, but we could wait a few months if you rather avoid the workaround?

## Related Issues

#12468 (Spotted it while working on this)
LeSuisse added a commit to Enalean/tuleap that referenced this pull request Dec 2, 2019
…r the message can be expanded

Reproduction scenario:
1. Add a project banner with a long message so it needs multiple lines to be
fully displayed
2. Close the project banner
3. Refresh the page
4. Open the banner, with your mouse cursor go on top of the message: it should
be a pointer not a text selector

JSDOM 11 does not implement MutationObserver and Jest currently uses
JSDOM 11 and does not intend to upgrade soon to not break compatibility
with older version of Node.js [0]. In order to make it work we now uses
the latest version of JSDOM for the Tuleap test suites, some tests that were
not compatible has been fixed.

[0] jestjs/jest#7122

Change-Id: If043f4c0428a4abc29ccbd2575306cf88dd673e6
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants