From c22ec2d89513e15e0ccf0a7719a58e8d4fd36091 Mon Sep 17 00:00:00 2001 From: Martin Feineis Date: Sun, 3 Feb 2019 19:18:41 +0100 Subject: [PATCH 1/7] Check for existence of `Element` when comparing DOM-nody-things (#7786) --- .../__snapshots__/matchers.test.js.snap | 7 +++++++ .../expect/src/__tests__/matchers.test.js | 10 ++++++++++ packages/expect/src/jasmineUtils.ts | 20 +++++++++++-------- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap b/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap index d576cdbad479..81179bd20b95 100644 --- a/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap +++ b/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap @@ -1996,6 +1996,13 @@ Expected: {\\"a\\": 99} Received: {\\"a\\": 99}" `; +exports[`.toEqual() {pass: false} expect({"nodeName": "div", "nodeType": 1}).not.toEqual({"nodeName": "div", "nodeType": 1}) 1`] = ` +"expect(received).not.toEqual(expected) + +Expected: {\\"nodeName\\": \\"div\\", \\"nodeType\\": 1} +Received: {\\"nodeName\\": \\"div\\", \\"nodeType\\": 1}" +`; + exports[`.toEqual() {pass: false} expect({"target": {"nodeType": 1, "value": "a"}}).toEqual({"target": {"nodeType": 1, "value": "b"}}) 1`] = ` "expect(received).toEqual(expected) diff --git a/packages/expect/src/__tests__/matchers.test.js b/packages/expect/src/__tests__/matchers.test.js index 4e2cf535b6b3..3fd55b3b854a 100644 --- a/packages/expect/src/__tests__/matchers.test.js +++ b/packages/expect/src/__tests__/matchers.test.js @@ -537,6 +537,16 @@ describe('.toEqual()', () => { }, }, ], + [ + { + nodeName: 'div', + nodeType: 1, + }, + { + nodeName: 'div', + nodeType: 1, + }, + ], ].forEach(([a, b]) => { test(`{pass: false} expect(${stringify(a)}).not.toEqual(${stringify( b, diff --git a/packages/expect/src/jasmineUtils.ts b/packages/expect/src/jasmineUtils.ts index 11e368d11001..6e1a2ee061c3 100644 --- a/packages/expect/src/jasmineUtils.ts +++ b/packages/expect/src/jasmineUtils.ts @@ -134,14 +134,18 @@ function eq( if (a.isEqualNode) { return a.isEqualNode(b); } - // IE8 doesn't support isEqualNode, try to use outerHTML && innerText - var aIsElement = a instanceof Element; - var bIsElement = b instanceof Element; - if (aIsElement && bIsElement) { - return a.outerHTML == b.outerHTML; - } - if (aIsElement || bIsElement) { - return false; + // In some test environments (e.g. "node") there is no `Element` even though + // we might be comparing things that look like DOM nodes + if (typeof Element !== 'undefined') { + // IE8 doesn't support isEqualNode, try to use outerHTML && innerText + var aIsElement = a instanceof Element; + var bIsElement = b instanceof Element; + if (aIsElement && bIsElement) { + return a.outerHTML == b.outerHTML; + } + if (aIsElement || bIsElement) { + return false; + } } return a.innerText == b.innerText && a.textContent == b.textContent; } From 50da3fc2dd64a9acfe53820133c835eebd48c003 Mon Sep 17 00:00:00 2001 From: Martin Feineis Date: Mon, 4 Feb 2019 20:36:39 +0100 Subject: [PATCH 2/7] Fall back to deep equality for presumed DOM nodes if `Element` is not in scope (#7786) --- .../__tests__/__snapshots__/matchers.test.js.snap | 15 +++++++++++++++ packages/expect/src/__tests__/matchers.test.js | 10 ++++++++++ packages/expect/src/jasmineUtils.ts | 7 ++++--- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap b/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap index 81179bd20b95..19d8645061d3 100644 --- a/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap +++ b/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap @@ -2003,6 +2003,21 @@ Expected: {\\"nodeName\\": \\"div\\", \\"nodeType\\": 1} Received: {\\"nodeName\\": \\"div\\", \\"nodeType\\": 1}" `; +exports[`.toEqual() {pass: false} expect({"nodeName": "div", "nodeType": 1}).toEqual({"nodeName": "p", "nodeType": 1}) 1`] = ` +"expect(received).toEqual(expected) + +Difference: + +- Expected ++ Received + + Object { +- \\"nodeName\\": \\"p\\", ++ \\"nodeName\\": \\"div\\", + \\"nodeType\\": 1, + }" +`; + exports[`.toEqual() {pass: false} expect({"target": {"nodeType": 1, "value": "a"}}).toEqual({"target": {"nodeType": 1, "value": "b"}}) 1`] = ` "expect(received).toEqual(expected) diff --git a/packages/expect/src/__tests__/matchers.test.js b/packages/expect/src/__tests__/matchers.test.js index 3fd55b3b854a..4be1e90b0ac8 100644 --- a/packages/expect/src/__tests__/matchers.test.js +++ b/packages/expect/src/__tests__/matchers.test.js @@ -417,6 +417,16 @@ describe('.toEqual()', () => { }, }, ], + [ + { + nodeName: 'div', + nodeType: 1, + }, + { + nodeName: 'p', + nodeType: 1, + }, + ], ].forEach(([a, b]) => { test(`{pass: false} expect(${stringify(a)}).toEqual(${stringify( b, diff --git a/packages/expect/src/jasmineUtils.ts b/packages/expect/src/jasmineUtils.ts index 6e1a2ee061c3..acc3fc005333 100644 --- a/packages/expect/src/jasmineUtils.ts +++ b/packages/expect/src/jasmineUtils.ts @@ -129,6 +129,7 @@ function eq( var aIsDomNode = isDomNode(a); var bIsDomNode = isDomNode(b); + var hasElementCtor = typeof Element !== 'undefined'; if (aIsDomNode && bIsDomNode) { // At first try to use DOM3 method isEqualNode if (a.isEqualNode) { @@ -136,7 +137,7 @@ function eq( } // In some test environments (e.g. "node") there is no `Element` even though // we might be comparing things that look like DOM nodes - if (typeof Element !== 'undefined') { + if (hasElementCtor) { // IE8 doesn't support isEqualNode, try to use outerHTML && innerText var aIsElement = a instanceof Element; var bIsElement = b instanceof Element; @@ -146,10 +147,10 @@ function eq( if (aIsElement || bIsElement) { return false; } + return a.innerText == b.innerText && a.textContent == b.textContent; } - return a.innerText == b.innerText && a.textContent == b.textContent; } - if (aIsDomNode || bIsDomNode) { + if (hasElementCtor && (aIsDomNode || bIsDomNode)) { return false; } From f47ae5838e39234cad4c77d4cac3644a7af765de Mon Sep 17 00:00:00 2001 From: Martin Feineis Date: Tue, 5 Feb 2019 00:51:39 +0100 Subject: [PATCH 3/7] Remove IE<9 specific fallback DOM node comparison (#7786) --- packages/expect/src/jasmineUtils.ts | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/packages/expect/src/jasmineUtils.ts b/packages/expect/src/jasmineUtils.ts index acc3fc005333..1cdc2016fe8d 100644 --- a/packages/expect/src/jasmineUtils.ts +++ b/packages/expect/src/jasmineUtils.ts @@ -129,28 +129,14 @@ function eq( var aIsDomNode = isDomNode(a); var bIsDomNode = isDomNode(b); - var hasElementCtor = typeof Element !== 'undefined'; - if (aIsDomNode && bIsDomNode) { - // At first try to use DOM3 method isEqualNode - if (a.isEqualNode) { - return a.isEqualNode(b); - } - // In some test environments (e.g. "node") there is no `Element` even though - // we might be comparing things that look like DOM nodes - if (hasElementCtor) { - // IE8 doesn't support isEqualNode, try to use outerHTML && innerText - var aIsElement = a instanceof Element; - var bIsElement = b instanceof Element; - if (aIsElement && bIsElement) { - return a.outerHTML == b.outerHTML; - } - if (aIsElement || bIsElement) { - return false; - } - return a.innerText == b.innerText && a.textContent == b.textContent; - } + // Use DOM3 method isEqualNode (IE>=9) + if (aIsDomNode && a.isEqualNode && bIsDomNode) { + return a.isEqualNode(b); } - if (hasElementCtor && (aIsDomNode || bIsDomNode)) { + // In some test environments (e.g. "node") there is no `Element` even though + // we might be comparing things that look like DOM nodes. In these cases we + // fall back to deep equality because we just don't know better at this point + if (typeof Element !== 'undefined' && (aIsDomNode || bIsDomNode)) { return false; } From 8f3265bb5ed9e99eaccdb8e68eaf91fedfca6c3a Mon Sep 17 00:00:00 2001 From: Martin Feineis Date: Tue, 5 Feb 2019 21:12:07 +0100 Subject: [PATCH 4/7] Check for global DOM `Node` in `isDomNode` (#7786) In a real browser and JSDOM the check will use DOM-Level-3 `Node.isEqualNode` and for everything else a deep equality check should do the trick. --- packages/expect/src/jasmineUtils.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/expect/src/jasmineUtils.ts b/packages/expect/src/jasmineUtils.ts index 1cdc2016fe8d..eff9c38ea9f2 100644 --- a/packages/expect/src/jasmineUtils.ts +++ b/packages/expect/src/jasmineUtils.ts @@ -133,12 +133,6 @@ function eq( if (aIsDomNode && a.isEqualNode && bIsDomNode) { return a.isEqualNode(b); } - // In some test environments (e.g. "node") there is no `Element` even though - // we might be comparing things that look like DOM nodes. In these cases we - // fall back to deep equality because we just don't know better at this point - if (typeof Element !== 'undefined' && (aIsDomNode || bIsDomNode)) { - return false; - } // Assume equality for cyclic structures. The algorithm for detecting cyclic // structures is adapted from ES 5.1 section 15.12.3, abstract operation `JO`. @@ -251,11 +245,15 @@ export function isA(typeName: string, value: unknown) { } function isDomNode(obj: any): obj is Node { + // In some test environments (e.g. "node") there is no `Node` even though + // we might be comparing things that look like DOM nodes. return ( - obj !== null && - typeof obj === 'object' && - typeof obj.nodeType === 'number' && - typeof obj.nodeName === 'string' + typeof Node !== 'undefined' + ? obj instanceof Node + : obj !== null && + typeof obj === 'object' && + typeof obj.nodeType === 'number' && + typeof obj.nodeName === 'string' ); } From aabb6629d42b39b252b20b93b2056611cb2c217d Mon Sep 17 00:00:00 2001 From: Martin Feineis Date: Wed, 6 Feb 2019 21:08:28 +0100 Subject: [PATCH 5/7] Check that `isEqualNode` is a function and remove duck typing from `isDomNode` (#7786) --- packages/expect/src/jasmineUtils.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/packages/expect/src/jasmineUtils.ts b/packages/expect/src/jasmineUtils.ts index eff9c38ea9f2..d87bd502daec 100644 --- a/packages/expect/src/jasmineUtils.ts +++ b/packages/expect/src/jasmineUtils.ts @@ -130,7 +130,7 @@ function eq( var aIsDomNode = isDomNode(a); var bIsDomNode = isDomNode(b); // Use DOM3 method isEqualNode (IE>=9) - if (aIsDomNode && a.isEqualNode && bIsDomNode) { + if (aIsDomNode && typeof a.isEqualNode === 'function' && bIsDomNode) { return a.isEqualNode(b); } @@ -247,14 +247,7 @@ export function isA(typeName: string, value: unknown) { function isDomNode(obj: any): obj is Node { // In some test environments (e.g. "node") there is no `Node` even though // we might be comparing things that look like DOM nodes. - return ( - typeof Node !== 'undefined' - ? obj instanceof Node - : obj !== null && - typeof obj === 'object' && - typeof obj.nodeType === 'number' && - typeof obj.nodeName === 'string' - ); + return typeof Node !== 'undefined' && obj instanceof Node; } export function fnNameFor(func: Function) { From 44309ee0ded56e81ba384b52f7150f22a4b25068 Mon Sep 17 00:00:00 2001 From: Martin Feineis Date: Fri, 1 Mar 2019 19:27:18 +0100 Subject: [PATCH 6/7] Add changelog entry for streamlined DOM node comparisons in `expect` (#7786) --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe3a1932a876..0080c3f99585 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ - `[jest-worker]` Fix `jest-worker` when using pre-allocated jobs ([#7934](https://github.com/facebook/jest/pull/7934)) - `[jest-changed-files]` Fix `getChangedFilesFromRoots` to not return parts of the commit messages as if they were files, when the commit messages contained multiple paragraphs ([#7961](https://github.com/facebook/jest/pull/7961)) - `[static]` Remove console log '-' on the front page +- `[expect]` Remove duck typing and obsolete browser support code when comparing DOM nodes and use DOM-Level-3 API instead ([#7786](https://github.com/facebook/jest/pull/7995)) ### Chore & Maintenance From 79e152d2f776f82e91d5f7f01fd28b1a61fd0e29 Mon Sep 17 00:00:00 2001 From: Mark Pedrotti Date: Fri, 1 Mar 2019 14:21:42 -0500 Subject: [PATCH 7/7] Correct visible PR number in CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 480815530b31..039968ce1d74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,7 +29,7 @@ - `[expect]` Fix non-object received value in toHaveProperty ([#7986](https://github.com/facebook/jest/pull/7986)) - `[jest-jasmine2]`: Throw explicit error when errors happen after test is considered complete ([#8005](https://github.com/facebook/jest/pull/8005)) - `[jest-circus]`: Throw explicit error when errors happen after test is considered complete ([#8005](https://github.com/facebook/jest/pull/8005)) -- `[expect]` Remove duck typing and obsolete browser support code when comparing DOM nodes and use DOM-Level-3 API instead ([#7786](https://github.com/facebook/jest/pull/7995)) +- `[expect]` Remove duck typing and obsolete browser support code when comparing DOM nodes and use DOM-Level-3 API instead ([#7995](https://github.com/facebook/jest/pull/7995)) ### Chore & Maintenance