From c50c382748608d1f67068b3e0356f131b9715921 Mon Sep 17 00:00:00 2001 From: "nik.savchenko@nielsen.com" Date: Wed, 5 May 2021 16:13:52 +0300 Subject: [PATCH 1/8] test(error-messages): add prototype of test for future fix - first step for fix issue: Output User Error instead of New Issue error for wrong node from user getWindowFromNode(node) addresses issue #911 --- src/__tests__/helpers.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/__tests__/helpers.js b/src/__tests__/helpers.js index eb86a1d1..d6f53db3 100644 --- a/src/__tests__/helpers.js +++ b/src/__tests__/helpers.js @@ -1,3 +1,4 @@ +import {screen} from '../' import { getDocument, getWindowFromNode, @@ -10,6 +11,13 @@ test('returns global document if exists', () => { }) describe('window retrieval throws when given something other than a node', () => { + // we had an issue when user insert screen instead of query + // actually here should be another more clear error output + test('screen as node', () => { + expect(() => getWindowFromNode(screen)).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"`, + ) + }) test('Promise as node', () => { expect(() => getWindowFromNode(new Promise(jest.fn())), From cf822cabc473f9b56e50526227dfb0f38fb62c41 Mon Sep 17 00:00:00 2001 From: "nik.savchenko@nielsen.com" Date: Wed, 5 May 2021 16:33:41 +0300 Subject: [PATCH 2/8] fix(error-messages): issue: Output User Error instead of New Issue error for wrong node from user - by adding additional check and correct error-message according to edge use case from the issue #911 --- src/__tests__/helpers.js | 2 +- src/helpers.js | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/__tests__/helpers.js b/src/__tests__/helpers.js index d6f53db3..cff2a78a 100644 --- a/src/__tests__/helpers.js +++ b/src/__tests__/helpers.js @@ -15,7 +15,7 @@ describe('window retrieval throws when given something other than a node', () => // actually here should be another more clear error output test('screen as node', () => { expect(() => getWindowFromNode(screen)).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"`, + `"It looks like you used 'screen' object instead of one of selectors from screen.selectMethod and it passed a 'screen' instead of a DOM node. Did you do something like \`fireEvent.click(screen, ...\` when you meant to use a \`fireEvent.click(screen.getBy..., \`?"`, ) }) test('Promise as node', () => { diff --git a/src/helpers.js b/src/helpers.js index 4536ddd5..9fe32a9c 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -105,6 +105,13 @@ function getWindowFromNode(node) { throw new Error( `It looks like you passed an Array instead of a DOM node. Did you do something like \`fireEvent.click(screen.getAllBy...\` when you meant to use a \`getBy\` query \`fireEvent.click(screen.getBy...\`?`, ) + } else if ( + node.debug instanceof Function && + node.logTestingPlaygroundURL instanceof Function + ) { + throw new Error( + `It looks like you used 'screen' object instead of one of selectors from screen.selectMethod and it passed a 'screen' instead of a DOM node. Did you do something like \`fireEvent.click(screen, ...\` when you meant to use a \`fireEvent.click(screen.getBy..., \`?`, + ) } else { // The user passed something unusual to a calling function throw new Error( From d6d95acddbeb5591bce646679f1cd25f460f0d2a Mon Sep 17 00:00:00 2001 From: Nik Savchenko Date: Thu, 6 May 2021 09:39:49 +0300 Subject: [PATCH 3/8] Update src/helpers.js Co-authored-by: Sebastian Silbermann --- src/helpers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/helpers.js b/src/helpers.js index 9fe32a9c..7693d799 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -106,7 +106,7 @@ function getWindowFromNode(node) { `It looks like you passed an Array instead of a DOM node. Did you do something like \`fireEvent.click(screen.getAllBy...\` when you meant to use a \`getBy\` query \`fireEvent.click(screen.getBy...\`?`, ) } else if ( - node.debug instanceof Function && + typeof node.debug === 'function' && node.logTestingPlaygroundURL instanceof Function ) { throw new Error( From 08fccd6d8825312450ffd5f6ee300201eb19b989 Mon Sep 17 00:00:00 2001 From: Nik Savchenko Date: Sun, 9 May 2021 08:50:04 +0300 Subject: [PATCH 4/8] Update src/helpers.js Co-authored-by: Sebastian Silbermann --- src/helpers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/helpers.js b/src/helpers.js index 7693d799..9cfc34c3 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -107,7 +107,7 @@ function getWindowFromNode(node) { ) } else if ( typeof node.debug === 'function' && - node.logTestingPlaygroundURL instanceof Function + typeof node.logTestingPlaygroundURL === 'function' ) { throw new Error( `It looks like you used 'screen' object instead of one of selectors from screen.selectMethod and it passed a 'screen' instead of a DOM node. Did you do something like \`fireEvent.click(screen, ...\` when you meant to use a \`fireEvent.click(screen.getBy..., \`?`, From 991241a017780494cc805d6c8f06e6ae66e62c11 Mon Sep 17 00:00:00 2001 From: Nik Savchenko Date: Sun, 9 May 2021 08:50:12 +0300 Subject: [PATCH 5/8] Update src/helpers.js Co-authored-by: Sebastian Silbermann --- src/helpers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/helpers.js b/src/helpers.js index 9cfc34c3..2fc368d1 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -110,7 +110,7 @@ function getWindowFromNode(node) { typeof node.logTestingPlaygroundURL === 'function' ) { throw new Error( - `It looks like you used 'screen' object instead of one of selectors from screen.selectMethod and it passed a 'screen' instead of a DOM node. Did you do something like \`fireEvent.click(screen, ...\` when you meant to use a \`fireEvent.click(screen.getBy..., \`?`, + `It looks like you passed a \`screen\` object. Did you do something like \`fireEvent.click(screen, ...\` when you meant to use a \`fireEvent.click(screen.getBy..., \`?`, ) } else { // The user passed something unusual to a calling function From 72d8f0a6d26560f10088ca18a1b4eecc439d8071 Mon Sep 17 00:00:00 2001 From: Nik Savchenko Date: Mon, 10 May 2021 09:49:05 +0300 Subject: [PATCH 6/8] Update src/helpers.js Co-authored-by: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> --- src/helpers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/helpers.js b/src/helpers.js index 2fc368d1..8cdecc78 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -110,7 +110,7 @@ function getWindowFromNode(node) { typeof node.logTestingPlaygroundURL === 'function' ) { throw new Error( - `It looks like you passed a \`screen\` object. Did you do something like \`fireEvent.click(screen, ...\` when you meant to use a \`fireEvent.click(screen.getBy..., \`?`, + `It looks like you passed a \`screen\` object. Did you do something like \`fireEvent.click(screen, ...\` when you meant to use a query, e.g. \`fireEvent.click(screen.getBy..., \`?`, ) } else { // The user passed something unusual to a calling function From 539c52dcfb873a8ddaa11c317a3089ffaab06acd Mon Sep 17 00:00:00 2001 From: Nik Savchenko Date: Mon, 10 May 2021 09:49:12 +0300 Subject: [PATCH 7/8] Update src/__tests__/helpers.js Co-authored-by: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> --- src/__tests__/helpers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/__tests__/helpers.js b/src/__tests__/helpers.js index cff2a78a..20474019 100644 --- a/src/__tests__/helpers.js +++ b/src/__tests__/helpers.js @@ -15,7 +15,7 @@ describe('window retrieval throws when given something other than a node', () => // actually here should be another more clear error output test('screen as node', () => { expect(() => getWindowFromNode(screen)).toThrowErrorMatchingInlineSnapshot( - `"It looks like you used 'screen' object instead of one of selectors from screen.selectMethod and it passed a 'screen' instead of a DOM node. Did you do something like \`fireEvent.click(screen, ...\` when you meant to use a \`fireEvent.click(screen.getBy..., \`?"`, + `"It looks like you used 'screen' object instead of one of selectors from screen.selectMethod and it passed a 'screen' instead of a DOM node. Did you do something like \`fireEvent.click(screen, ...\` when you meant to use a query, e.g. \`fireEvent.click(screen.getBy..., \`?"`, ) }) test('Promise as node', () => { From 84aaba96b68e7d469f11362754fa9be3abb0500c Mon Sep 17 00:00:00 2001 From: eps1lon Date: Mon, 10 May 2021 10:05:40 +0200 Subject: [PATCH 8/8] Update snapshots --- src/__tests__/helpers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/__tests__/helpers.js b/src/__tests__/helpers.js index 20474019..07b53007 100644 --- a/src/__tests__/helpers.js +++ b/src/__tests__/helpers.js @@ -15,7 +15,7 @@ describe('window retrieval throws when given something other than a node', () => // actually here should be another more clear error output test('screen as node', () => { expect(() => getWindowFromNode(screen)).toThrowErrorMatchingInlineSnapshot( - `"It looks like you used 'screen' object instead of one of selectors from screen.selectMethod and it passed a 'screen' instead of a DOM node. Did you do something like \`fireEvent.click(screen, ...\` when you meant to use a query, e.g. \`fireEvent.click(screen.getBy..., \`?"`, + `"It looks like you passed a \`screen\` object. Did you do something like \`fireEvent.click(screen, ...\` when you meant to use a query, e.g. \`fireEvent.click(screen.getBy..., \`?"`, ) }) test('Promise as node', () => {