fix: support jsdom detached fragments #288
Conversation
| child => | ||
| child.nodeType === window.Node.TEXT_NODE && Boolean(child.textContent), | ||
| ) | ||
| .filter(child => child.nodeType === TEXT_NODE && Boolean(child.textContent)) |
There was a problem hiding this comment.
Here I opted to just inline the TEXT_NODE constant. Alternatively it looks like this is also available as document.TEXT_NODE although I'm not entirely sure what the browser support for that is. Using the constant seemed sane as it is only in one place so far.
| // node is document | ||
| return node.defaultView | ||
| } else if (node.ownerDocument) { | ||
| } else if (node.ownerDocument && node.ownerDocument.defaultView) { |
There was a problem hiding this comment.
With JSDOM fragments there is still an ownerDocument, but no defaultView.
kentcdodds
left a comment
kentcdodds
left a comment
There was a problem hiding this comment.
Looks pretty good. Just one thing.
| @@ -1,15 +1,11 @@ | |||
| const TEXT_NODE = 3 | |||
There was a problem hiding this comment.
Could we add a link to MDN docs that explain that this is window.Node.TEXT_NODE and the value is 3 (per the spec). Otherwise this is surprising.
There was a problem hiding this comment.
Good idea, updated the pr 😄
|
@all-contributors please add @DylanPiercey for code and tests |
|
I've put up a pull request to add @DylanPiercey! 🎉 |
|
@kentcdodds not sure if you use gitter, but I reached out to you on there with some quick questions regarding adding a testing-library/marko module. Is there an appropriate place other than twitter to have a bit of a back and forth? 😛 |
|
Go ahead and open an issue on dom-testing-library :) |
|
🎉 This PR is included in version 5.2.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
2 participants
What:
This PR builds on top of c4b4eff and adds support for using the
JSDOM.fragmentAPI for server side tests.Why:
I think using the JSDOM.fragment API makes sense for SSR only tests. In theory if there was client side logic you would instead be running these tests in a browser context.
With this fragment API there is no
window, which caused the existing jsdom test to break.My current thinking is that for SSR tests you really shouldn't be testing events and what not, instead use a browser test for that. Using the fragment API outputs a sane error if you try to do so, while still supporting the same API for everything else.
How:
I've made a couple small changes that account for the scenario when the DOM is not in a browser context.
Checklist:
docs site