Core: Update tested jsdom, drop obsolete workarounds #2154
Conversation
The latest version supporting Node.js is 3.1.2; some workarounds are not needed for this version. For example, in jsdom 3.1.2 a document created via document.implementation.createHTMLDocument( "" ) has a body. Fixes jquerygh-2153 Closes jquerygh-2154
| @@ -46,61 +46,57 @@ define([ | |||
| documentElement.removeChild( container ); | |||
| } | |||
|
|
|||
| // Support: node.js jsdom | |||
| // Don't assume that getComputedStyle is a property of the global object | |||
There was a problem hiding this comment.
This comment didn't make sense even for jsdom 1.5. Were there any other reasons to hide these support tests? Are we worried about other envs that will expose window without getComputedStyle? If so, I'll revert but the comment will need to be changed completely.
There was a problem hiding this comment.
I'm not sure if it comes into play here, but window may be a non-Window global object: https://github.com/mzgol/jquery/blob/jsdom-update/src/intro.js#L38
In any case, the comment probably does need rewriting.
There was a problem hiding this comment.
It used to say getComputedStyle assuming that was in the global scope. So it was more about assuming window and global were equivalent.
There was a problem hiding this comment.
Yup, window is not a global here but what was passed to the factory. Unless the global has a property document when we assume we're in a browser and assign window = current global.
This works without this if in jsdom because its window parameter has getComputedStyle. Do we want to support anything else here?
There was a problem hiding this comment.
I'd like to get input from more people here as I'm a little afraid of this change. cc @jquery/core
There was a problem hiding this comment.
As long as getComputedStyle is available, I'm fine with this.
There was a problem hiding this comment.
As long as getComputedStyle is available, I'm fine with this.
Great. I'd like to have as many people test it as possible since I'm still worried a little so I'd like to get it into the first beta.
There was a problem hiding this comment.
This should be safe, see 3722aef , dc3f7d3 and http://bugs.jquery.com/ticket/12235 for background.
The latest version supporting Node.js is 3.1.2; some workarounds are not needed for this version. For example, in jsdom 3.1.2 a document created via document.implementation.createHTMLDocument( "" ) has a body. Fixes jquerygh-2153 Closes jquerygh-2154
The latest version supporting Node.js is 3.1.2; some workarounds are not needed for this version. For example, in jsdom 3.1.2 a document created via document.implementation.createHTMLDocument( "" ) has a body. Fixes jquerygh-2153 Closes jquerygh-2154
The latest version supporting Node.js is 3.1.2; some workarounds are not needed
for this version. For example, in jsdom 3.1.2 a document created via
document.implementation.createHTMLDocument( "" ) has a body.
Fixes gh-2153