Upgrade to JSDOM@11 #4770
Upgrade to JSDOM@11 #4770
Conversation
| @@ -22,13 +22,14 @@ class JSDOMEnvironment { | |||
| errorEventListener: ?Function; | |||
| moduleMocker: ?ModuleMocker; | |||
|
|
|||
| constructor(config: ProjectConfig): void { | |||
| constructor(config: ProjectConfig) { | |||
| @@ -44,7 +44,6 @@ | |||
| "istanbul-lib-coverage": "^1.0.0", | |||
| "jasmine-reporters": "^2.2.0", | |||
| "jquery": "^3.2.1", | |||
| "jsdom": "^9.12.0", | |||
There was a problem hiding this comment.
any reason for it to be specified down here as well?
There was a problem hiding this comment.
Nope I guess, workspaces should handle this.
There was a problem hiding this comment.
yeah, it's hoisted, and I didn't see any require of it outside of jest-environment-jsdom
There was a problem hiding this comment.
If it works, that's fine. We used to have a test depending on jsdom, but that was all pre workspaces.
|
@SimenB thanks for updating. It's been a long time coming! |
| url: config.testURL, | ||
| }); | ||
| const global = (this.global = this.document.defaultView); | ||
| const global = (this.global = this.document.window.document.defaultView); |
There was a problem hiding this comment.
Oh, I see, this.document is no longer a document, it's a JSDOM instance. That's pretty confusing.
There was a problem hiding this comment.
Which part is confusing? Should this.document be renamed to this.jsdomInstance? I have to admit I floundered a bit here
There was a problem hiding this comment.
Yeah, you shouldn't name a JSDOM instance document (similar to how you shouldn't name it birthdayCake).
We use the variable name dom for short in the JSDOM docs.
|
A note of caution - this particular version of jsdom (11.3.0) includes an SVG-related regression. While it is a spec-compliance improvement (creating instances of There is a new pull request which is about to change this by implementing proper SVG support - jsdom/jsdom#2011 - and it'll be part of the next jsdom release. Since jsdom is a spare time project for everyone involved it is not yet certain when this release will happen, though presumably within the next few weeks. In case the next major Jest release is scheduled to happen soon, I figured I'd make you aware of this. Using 11.2.0 instead might be an option if that's the case. Besides this, it's great to see Jest moving to recent versions of jsdom. |
|
I think we already hit that, fixed in #4645 |
|
@SimenB Hmm, isn't that fix only related to a formatter? The described issue will affect anyone trying to use SVG elements in a jsdom environment. |
|
Ah, good point. I'm not sure when 22 of Jest is scheduled, might make sense to lock it down to 11.2, then. Thanks for pointing it out! (also waiting for jsdom/jsdom#1994 to be merged as well, so we can remove the implementation here) |
|
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. |
Summary
Now that we've dropped node 4, we can finally do this
Test plan
Green CI