Skip to content

Fix checkHtmlElement helper #67

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Nov 6, 2018
Merged

Fix checkHtmlElement helper #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ebdbbd8
Fix checkHtmlElement helper
sheerun Oct 17, 2018
3ac158d
Remove checkHtmlText
sheerun Oct 18, 2018
57a32ad
Merge branch 'master' into fix-tests
gnapse Nov 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 45 additions & 1 deletion src/__tests__/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {deprecate} from '../utils'
import {deprecate, checkHtmlElement, HtmlElementTypeError} from '../utils'
import document from './helpers/document'

test('deprecate', () => {
const spy = jest.spyOn(console, 'warn').mockImplementation(() => {})
Expand All @@ -14,3 +15,46 @@ test('deprecate', () => {

spy.mockRestore()
})

describe('checkHtmlElement', () => {
it('does not throw an error for correct html element', () => {
expect(() => {
const element = document.createElement('p')
checkHtmlElement(element, () => {}, {})
}).not.toThrow()
})

it('does not throw an error for correct svg element', () => {
expect(() => {
const element = document.createElementNS(
'http://www.w3.org/2000/svg',
'rect',
)
checkHtmlElement(element, () => {}, {})
}).not.toThrow()
})

it('does not throw for body', () => {
expect(() => {
checkHtmlElement(document.body, () => {}, {})
}).not.toThrow()
})

it('throws for undefined', () => {
expect(() => {
checkHtmlElement(undefined, () => {}, {})
}).toThrow(HtmlElementTypeError)
})

it('throws for document', () => {
expect(() => {
checkHtmlElement(document, () => {}, {})
}).toThrow(HtmlElementTypeError)
})

it('throws for function', () => {
expect(() => {
checkHtmlElement(() => {}, () => {}, {})
}).toThrow(HtmlElementTypeError)
})
})
10 changes: 0 additions & 10 deletions src/to-contain-html.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,8 @@
import {matcherHint, printReceived} from 'jest-matcher-utils'
import {checkHtmlElement} from './utils'

function checkHtmlText(element, htmlText, ...args) {
const DOMParser = element.ownerDocument.defaultView.DOMParser
const htmlElement =
typeof htmlText === 'string'
? new DOMParser().parseFromString(htmlText, 'text/html').body.firstChild
: null
checkHtmlElement(htmlElement, ...args)
}

export function toContainHTML(container, htmlText) {
checkHtmlElement(container, toContainHTML, this)
checkHtmlText(container, htmlText, toContainHTML, this)

return {
pass: container.outerHTML.includes(htmlText),
Expand Down
28 changes: 24 additions & 4 deletions src/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ class HtmlElementTypeError extends Error {
if (Error.captureStackTrace) {
Error.captureStackTrace(this, matcherFn)
}
let withType = ''
try {
withType = printWithType('Received', received, printReceived)
} catch (e) {
// Can throw for Document:
// https://github.com/jsdom/jsdom/issues/2304
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sheerun can you provide more detail about why is this happening?

I'd like to understand the code in the future, and merely visiting the link did not make it evident what's happening here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea why, I know it happens and this try catch is needed for tests to pass

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But what do you mean with "Can throw for Document", and why do you link it to that issue in jsdom?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It happens for this test where I pass Document instance: https://github.com/gnapse/jest-dom/pull/67/files#diff-f00ff29fba28ce3d11d8b6a726c3788eR51

The error I'm seeing when this test fails is the same as in the issue linked. I suspect this is issue with printWithType helper and internals of jsdom but it's not something I can fix easily.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's best if you remove this try catch and see what I mean

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind the error is still shown to the user but is little less descriptive as it doesn't show that Document instance has been passed

this.message = [
matcherHint(
`${context.isNot ? '.not' : ''}.${matcherFn.name}`,
Expand All @@ -27,16 +34,28 @@ class HtmlElementTypeError extends Error {
`${receivedColor(
'received',
)} value must be an HTMLElement or an SVGElement.`,
printWithType('Received', received, printReceived),
withType,
].join('\n')
}
}

function checkHasWindow(htmlElement, ...args) {
if (
!htmlElement ||
!htmlElement.ownerDocument ||
!htmlElement.ownerDocument.defaultView
) {
throw new HtmlElementTypeError(htmlElement, ...args)
}
}

function checkHtmlElement(htmlElement, ...args) {
checkHasWindow(htmlElement, ...args)
const window = htmlElement.ownerDocument.defaultView

if (
!htmlElement.ownerDocument &&
!(htmlElement instanceof htmlElement.ownerDocument.HTMLElement) &&
!(htmlElement instanceof htmlElement.ownerDocument.SVGElement)
!(htmlElement instanceof window.HTMLElement) &&
!(htmlElement instanceof window.SVGElement)
) {
throw new HtmlElementTypeError(htmlElement, ...args)
}
Expand Down Expand Up @@ -115,6 +134,7 @@ function normalize(text) {
}

export {
HtmlElementTypeError,
checkHtmlElement,
checkValidCSS,
deprecate,
Expand Down