[docs] Update "jsdom" guides for v10 and later #921
Conversation
docs/guides/jsdom.md
Outdated
| /* setup.js */ | ||
|
|
||
| var { JSDOM } = require('jsdom'); | ||
| var jsdom = new JSDOM(''); |
There was a problem hiding this comment.
nits chage though. the latter parts is the same, so maybe better way to slim up?
There was a problem hiding this comment.
I don't think this is enough, either. With the proposed doc changes, I see the same errors as reported here: jsdom/jsdom#1827.
docs/guides/jsdom.md
Outdated
| var { JSDOM } = require('jsdom'); | ||
| var jsdom = new JSDOM(''); | ||
|
|
||
| global.document = jsdom; |
There was a problem hiding this comment.
Assuming the above (var jsdom = new JSDOM('');), this should be:
global.document = jsdom.window.document;There was a problem hiding this comment.
Thank you for reviewing.
I fixed the doc changes with 35c2f5e
I created a working sample repository with jsdom ^10.1.0.
https://github.com/kenju/enzyme-draftjs-sample/blob/master/test/setup.js
docs/guides/jsdom.md
Outdated
| global.window = window; | ||
| global.document = window.document; | ||
|
|
||
| Object.keys(window).forEach((property) => { |
There was a problem hiding this comment.
this should use Object.getOwnPropertyDescriptors (or Object.getOwnPropertyNames), not Object.keys, to ensure non-enumerables are grabbed as well.
There was a problem hiding this comment.
Thanks for the advise, that way would be better!
Fixed with: 57dc37a
docs/guides/jsdom.md
Outdated
|
|
||
| Object.keys(window).forEach((property) => { | ||
| if (typeof global[property] === 'undefined') { | ||
| global[property] = window[property]; |
There was a problem hiding this comment.
this should use defineProperty with a property descriptor to maintain nonenumerability and nonwritability when applicable.
|
|
||
| function copyProps(src, target) { | ||
| const props = Object.getOwnPropertyNames(src) | ||
| .filter(prop => typeof target[prop] === 'undefined') |
There was a problem hiding this comment.
i think you want to keep the ones where it's not undefined, no?
There was a problem hiding this comment.
no, actually I meant to filter out ones which are undefined at the target object.
this is equivalent with below code (would you prefer this style?):
Object.getOwnPropertyNames(src).forEach((prop) => {
if (typeof target[prop] === 'undefined') { // if not defined yet as the 'target's props
Object.getOwnPropertyDescriptor(src, prop);
}
});There was a problem hiding this comment.
Ah, you're right, thanks for clarifying :-)
|
I meant to create a minimal test case to replicate, but haven't had a chance yet. In the meantime, however, I wanted to report that with this latest version of the documentation, I get |
|
Does your code premise that
Does attaching global.XMLHttpRequest = jsdom.window.XMLHttpRequest;or, global.XMLHttpRequest = require('xmlhttprequest').XMLHttpRequest; |
|
It's a reasonable assumption for browser code to treat all |
|
@kenju would you mind rebasing this? |
|
what is the version of your const props = Object.getOwnPropertyNames(src)
.filter(prop => typeof target[prop] === 'undefined')
.map(prop => {
if (/XMLHttpRequest/.test(prop)) { // checks if 'XMLHttpRequest'-like props found or not
console.log(`${prop} found`);
}
return Object.getOwnPropertyDescriptor(src, prop);
}); |
|
@kenju it'll need another rebase; see "This branch is out-of-date with the base branch" below |
|
@ljharb Oops I have missed that, now updated with the latest master branch |
|
It looks like while this is merged, it has yet to be released - is that correct? The docs still look old - http://airbnb.io/enzyme/docs/guides/jsdom.html. I ran into an issue getting Enzyme + ReactResponsive to work in a test. Glad I came across this PR -nice work, the updated JSDOM setup fixed the issue I was having. |
|
Yes, that's correct. |
What
Updated "jsdom" guides which is based on "jsdom" v10 and later API
Why
jsdomhas recently updated to v10, but the enzyme's guides is based on "old-api".jsdomsays it supports the old api yet, so I did not delete the old code block but instead add a new code block.Fixes #942.