-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
support serializing DocumentFragment
#6705
Conversation
@@ -308,6 +308,45 @@ Testing.`; | |||
); | |||
}); | |||
|
|||
it('supports fragment node', () => { | |||
const fragment = document.createDocumentFragment(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -37,17 +37,23 @@ type Comment = { | |||
data: string, | |||
nodeType: 8, | |||
}; | |||
type Fragment = { | |||
children: Array<Element>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how important typing this is? It's not an Array
, it's an HTMLCollection
, but the other typing here is pretty bad anyways
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using the types from flow makes the other stuff fail here since we can't use instanceof
checks and not explode outside of jsdom 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, flow passes with this diff
diff --git i/packages/pretty-format/src/plugins/dom_element.js w/packages/pretty-format/src/plugins/dom_element.js
index 26f0801f6..59957aa54 100644
--- i/packages/pretty-format/src/plugins/dom_element.js
+++ w/packages/pretty-format/src/plugins/dom_element.js
@@ -18,30 +18,6 @@ import {
printText,
} from './lib/markup';
-type Attribute = {
- name: string,
- value: string,
-};
-
-type Element = {
- attributes: Array<Attribute>,
- childNodes: Array<Element | Text | Comment>,
- nodeType: 1,
- tagName: string,
-};
-type Text = {
- data: string,
- nodeType: 3,
-};
-type Comment = {
- data: string,
- nodeType: 8,
-};
-type DocumentFragment = {
- children: Array<Element>,
- nodeType: 11,
-};
-
const ELEMENT_NODE = 1;
const TEXT_NODE = 3;
const COMMENT_NODE = 8;
@@ -76,17 +52,17 @@ export const serialize = (
refs: Refs,
printer: Printer,
): string => {
- if (node.nodeType === TEXT_NODE) {
+ if (node instanceof Text) {
return printText(node.data, config);
}
- if (node.nodeType === COMMENT_NODE) {
+ if (node instanceof Comment) {
return printComment(node.data, config);
}
let type;
- if (node.nodeType === FRAGMENT_NODE) {
+ if (node instanceof DocumentFragment) {
type = `Fragment`;
} else {
type = node.tagName.toLowerCase();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, maybe it's worth to use those?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It only works with instanceof
checks, which means it doesn't work in node, just jsdom
3f439ed
to
fa16957
Compare
Codecov Report
@@ Coverage Diff @@
## master #6705 +/- ##
==========================================
+ Coverage 63.56% 63.56% +<.01%
==========================================
Files 235 235
Lines 9026 9027 +1
Branches 3 3
==========================================
+ Hits 5737 5738 +1
Misses 3288 3288
Partials 1 1
Continue to review full report at Codecov.
|
let type; | ||
|
||
if (node.nodeType === FRAGMENT_NODE) { | ||
type = `Fragment`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do browsers show this type of node?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So maybe a DocumentFragment? Fragment and <> look like from React
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
* upstream/master: (122 commits) fix: don't report promises as open handles (jestjs#6716) support serializing `DocumentFragment` (jestjs#6705) Allow test titles to include array index (jestjs#6414) fix `toContain` suggest to contain equal message (jestjs#6810) fix: testMatch not working with negations (jestjs#6648) Add test cases for jestjs#6744 (jestjs#6772) print stack trace on calls to process.exit (jestjs#6714) Updates SnapshotTesting.md to provide more info. on snapshot scope (jestjs#6735) Mark snapshots as obsolete when moved to an inline snapshot (jestjs#6773) [Docs] Clarified the use of literal values as property matchers in toMatchSnapshot() (jestjs#6807) Update CHANGELOG.md (jestjs#6799) fix changelog entry that is not in 23.4.2 (jestjs#6796) Fix --coverage with --findRelatedTests overwriting collectCoverageFrom options (jestjs#6736) Update testURL default value from about:blank to localhost (jestjs#6792) fix: `matchInlineSnapshot` when prettier dependencies are mocked (jestjs#6776) Fix retryTimes and add e2e regression test (jestjs#6762) Release v23.4.2 Docs/ExpectAPI: Correct docs for `objectContaining` (jestjs#6754) chore(packages/babel-jest) readme (jestjs#6746) docs: noted --coverage aliased by --collectCoverage (jestjs#6741) ...
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
Useful when you have some markup you want to snapshot.
Prints:
Open to changing
<Fragment>
to something else. Just<>
perhaps?Test plan
Test added