From 0dae54afb4d37bde7c87b05830fd0b2953f2a703 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 14 Jun 2020 20:59:06 -0400 Subject: [PATCH 1/2] Added explicit error message for Promises passed to getWindowFromNode --- src/__tests__/helpers.js | 17 ++++++++++++++++- src/helpers.js | 7 +++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/__tests__/helpers.js b/src/__tests__/helpers.js index 717fd776..6bc26109 100644 --- a/src/__tests__/helpers.js +++ b/src/__tests__/helpers.js @@ -1,9 +1,24 @@ -import {getDocument, checkContainerType} from '../helpers' +import {getDocument, getWindowFromNode, checkContainerType} from '../helpers' test('returns global document if exists', () => { expect(getDocument()).toBe(document) }) +describe('window retrieval throws when given something other than a node', () => { + test('Promise as node', () => { + expect(() => + getWindowFromNode(new Promise(jest.fn())), + ).toThrowErrorMatchingInlineSnapshot( + `"It looks like you passed a Promise object instead of a DOM node. Did you use a \\"find\\" search instead of a \\"get\\" or \\"query\\"?"`, + ) + }) + test('unknown as node', () => { + expect(() => getWindowFromNode({})).toThrowErrorMatchingInlineSnapshot( + `"Unable to find the \\"window\\" object for the given node. Please file an issue with the code that's causing you to see this error: https://github.com/testing-library/dom-testing-library/issues/new"`, + ) + }) +}) + describe('query container validation throws when validation fails', () => { test('undefined as container', () => { expect(() => diff --git a/src/helpers.js b/src/helpers.js index 64031562..23160760 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -47,7 +47,6 @@ function getDocument() { return window.document } function getWindowFromNode(node) { - // istanbul ignore next I'm not sure what could cause the final else so we'll leave it uncovered. if (node.defaultView) { // node is document return node.defaultView @@ -57,8 +56,12 @@ function getWindowFromNode(node) { } else if (node.window) { // node is window return node.window + } else if (node.then instanceof Function) { + throw new Error( + `It looks like you passed a Promise object instead of a DOM node. Did you use a "find" search instead of a "get" or "query"?`, + ) } else { - // no idea... + // The user passed something unusual to a calling function throw new Error( `Unable to find the "window" object for the given node. Please file an issue with the code that's causing you to see this error: https://github.com/testing-library/dom-testing-library/issues/new`, ) From 2a42a496fa54bc3d5dae363ae9f5a154460ef98a Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 15 Jun 2020 00:50:52 -0400 Subject: [PATCH 2/2] Applied suggestion --- src/__tests__/helpers.js | 2 +- src/helpers.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/__tests__/helpers.js b/src/__tests__/helpers.js index 6bc26109..83d45052 100644 --- a/src/__tests__/helpers.js +++ b/src/__tests__/helpers.js @@ -9,7 +9,7 @@ describe('window retrieval throws when given something other than a node', () => expect(() => getWindowFromNode(new Promise(jest.fn())), ).toThrowErrorMatchingInlineSnapshot( - `"It looks like you passed a Promise object instead of a DOM node. Did you use a \\"find\\" search instead of a \\"get\\" or \\"query\\"?"`, + `"It looks like you passed a Promise object instead of a DOM node. Did you do something like \`fireEvent.click(screen.findBy...\` when you meant to do \`fireEvent.click(await screen.getBy...\`?"`, ) }) test('unknown as node', () => { diff --git a/src/helpers.js b/src/helpers.js index 23160760..567f2791 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -58,7 +58,7 @@ function getWindowFromNode(node) { return node.window } else if (node.then instanceof Function) { throw new Error( - `It looks like you passed a Promise object instead of a DOM node. Did you use a "find" search instead of a "get" or "query"?`, + `It looks like you passed a Promise object instead of a DOM node. Did you do something like \`fireEvent.click(screen.findBy...\` when you meant to do \`fireEvent.click(await screen.getBy...\`?`, ) } else { // The user passed something unusual to a calling function