From 2e30f52a8ed2b097f0ae44450c4691049ef022ce Mon Sep 17 00:00:00 2001 From: Wei-An Yen Date: Thu, 20 Aug 2020 02:17:36 +0800 Subject: [PATCH] Fix diffing object contain readonly symbol key object (#10414) --- CHANGELOG.md | 1 + .../__snapshots__/domDiffing.test.ts.snap | 51 +++++++++++++++++++ e2e/__tests__/domDiffing.test.ts | 16 ++++++ e2e/dom-diffing/__tests__/dom.test.js | 19 +++++++ e2e/dom-diffing/package.json | 3 ++ .../jest-matcher-utils/src/Replaceable.ts | 17 ++++--- .../src/__tests__/Replaceable.test.ts | 24 +++++++++ .../printDiffOrStringify.test.ts.snap | 13 +++++ .../deepCyclicCopyReplaceable.test.ts | 30 ++++++++++- .../deepCyclicCopyReplaceableDom.test.ts | 29 +++++++++++ .../__tests__/printDiffOrStringify.test.ts | 23 +++++++++ .../src/deepCyclicCopyReplaceable.ts | 34 +++++++++---- 12 files changed, 241 insertions(+), 19 deletions(-) create mode 100644 e2e/__tests__/__snapshots__/domDiffing.test.ts.snap create mode 100644 e2e/__tests__/domDiffing.test.ts create mode 100644 e2e/dom-diffing/__tests__/dom.test.js create mode 100644 e2e/dom-diffing/package.json create mode 100644 packages/jest-matcher-utils/src/__tests__/deepCyclicCopyReplaceableDom.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index dd0ebbcf853a..d9fc181407cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixes +- `[jest-matcher-utils]` Fix diffing object contain readonly symbol key object ([#10414](https://github.com/facebook/jest/pull/10414)) - `[jest-reporters]` Fixes notify reporter on Linux (using notify-send) ([#10393](https://github.com/facebook/jest/pull/10400)) - `[jest-snapshot]` Correctly handles arrays and property matchers in snapshots ([#10404](https://github.com/facebook/jest/pull/10404)) diff --git a/e2e/__tests__/__snapshots__/domDiffing.test.ts.snap b/e2e/__tests__/__snapshots__/domDiffing.test.ts.snap new file mode 100644 index 000000000000..a99d8acde447 --- /dev/null +++ b/e2e/__tests__/__snapshots__/domDiffing.test.ts.snap @@ -0,0 +1,51 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`should work without error 1`] = ` +"FAIL __tests__/dom.test.js + ✕ use toBe compare two div (<>) + ✕ compare span and div (<>) + + ● use toBe compare two div + + expect(received).toBe(expected) // Object.is equality + + If it should pass with deep equality, replace \\"toBe\\" with \\"toStrictEqual\\" + + Expected:
+ Received: serializes to the same string + + 12 | const div1 = document.createElement('div'); + 13 | const div2 = document.createElement('div'); + > 14 | expect(div1).toBe(div2); + | ^ + 15 | }); + 16 | + 17 | test('compare span and div', () => { + + at Object.toBe (__tests__/dom.test.js:14:16) + + ● compare span and div + + expect(received).toBe(expected) // Object.is equality + + - Expected - 1 + + Received + 1 + + - + +
+ + 16 | + 17 | test('compare span and div', () => { + > 18 | expect(document.createElement('div')).toBe(document.createElement('span')); + | ^ + 19 | }); + 20 | + + at Object.toBe (__tests__/dom.test.js:18:41) + +Test Suites: 1 failed, 1 total +Tests: 2 failed, 2 total +Snapshots: 0 total +Time: <> +Ran all test suites." +`; diff --git a/e2e/__tests__/domDiffing.test.ts b/e2e/__tests__/domDiffing.test.ts new file mode 100644 index 000000000000..64d4622ac156 --- /dev/null +++ b/e2e/__tests__/domDiffing.test.ts @@ -0,0 +1,16 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +import runJest from '../runJest'; +import {replaceTime} from '../Utils'; + +test('should work without error', () => { + const output = runJest('dom-diffing'); + expect(output.failed).toBe(true); + expect(replaceTime(output.stderr)).toMatchSnapshot(); +}); diff --git a/e2e/dom-diffing/__tests__/dom.test.js b/e2e/dom-diffing/__tests__/dom.test.js new file mode 100644 index 000000000000..9d87fc11b60b --- /dev/null +++ b/e2e/dom-diffing/__tests__/dom.test.js @@ -0,0 +1,19 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +/* eslint-env browser */ + +test('use toBe compare two div', () => { + const div1 = document.createElement('div'); + const div2 = document.createElement('div'); + expect(div1).toBe(div2); +}); + +test('compare span and div', () => { + expect(document.createElement('div')).toBe(document.createElement('span')); +}); diff --git a/e2e/dom-diffing/package.json b/e2e/dom-diffing/package.json new file mode 100644 index 000000000000..586d4ca6b75c --- /dev/null +++ b/e2e/dom-diffing/package.json @@ -0,0 +1,3 @@ +{ + "jest": {} +} diff --git a/packages/jest-matcher-utils/src/Replaceable.ts b/packages/jest-matcher-utils/src/Replaceable.ts index 8f518fef1494..07f26d6cd89a 100644 --- a/packages/jest-matcher-utils/src/Replaceable.ts +++ b/packages/jest-matcher-utils/src/Replaceable.ts @@ -32,12 +32,17 @@ export default class Replaceable { forEach(cb: ReplaceableForEachCallBack): void { if (this.type === 'object') { - Object.entries(this.object).forEach(([key, value]) => { - cb(value, key, this.object); - }); - Object.getOwnPropertySymbols(this.object).forEach(key => { - cb(this.object[key], key, this.object); - }); + const descriptors = Object.getOwnPropertyDescriptors(this.object); + [ + ...Object.keys(descriptors), + ...Object.getOwnPropertySymbols(descriptors), + ] + //@ts-expect-error because typescript do not support symbol key in object + //https://github.com/microsoft/TypeScript/issues/1863 + .filter(key => descriptors[key].enumerable) + .forEach(key => { + cb(this.object[key], key, this.object); + }); } else { this.object.forEach(cb); } diff --git a/packages/jest-matcher-utils/src/__tests__/Replaceable.test.ts b/packages/jest-matcher-utils/src/__tests__/Replaceable.test.ts index 890f1ba38b96..ef82ba3772ea 100644 --- a/packages/jest-matcher-utils/src/__tests__/Replaceable.test.ts +++ b/packages/jest-matcher-utils/src/__tests__/Replaceable.test.ts @@ -132,6 +132,30 @@ describe('Replaceable', () => { expect(cb.mock.calls[0]).toEqual([1, 'a', map]); expect(cb.mock.calls[1]).toEqual([2, 'b', map]); }); + + test('forEach should ignore nonenumerable property', () => { + const symbolKey = Symbol('jest'); + const symbolKey2 = Symbol('awesome'); + const object = {a: 1, [symbolKey]: 3}; + Object.defineProperty(object, 'b', { + configurable: true, + enumerable: false, + value: 2, + writable: true, + }); + Object.defineProperty(object, symbolKey2, { + configurable: true, + enumerable: false, + value: 4, + writable: true, + }); + const replaceable = new Replaceable(object); + const cb = jest.fn(); + replaceable.forEach(cb); + expect(cb).toHaveBeenCalledTimes(2); + expect(cb.mock.calls[0]).toEqual([1, 'a', object]); + expect(cb.mock.calls[1]).toEqual([3, symbolKey, object]); + }); }); describe('isReplaceable', () => { diff --git a/packages/jest-matcher-utils/src/__tests__/__snapshots__/printDiffOrStringify.test.ts.snap b/packages/jest-matcher-utils/src/__tests__/__snapshots__/printDiffOrStringify.test.ts.snap index 7e0674650dc6..a35b44743030 100644 --- a/packages/jest-matcher-utils/src/__tests__/__snapshots__/printDiffOrStringify.test.ts.snap +++ b/packages/jest-matcher-utils/src/__tests__/__snapshots__/printDiffOrStringify.test.ts.snap @@ -198,3 +198,16 @@ exports[`printDiffOrStringify has no common after clean up chaff one-line 1`] = Expected: "delete" Received: "insert" `; + +exports[`printDiffOrStringify object contain readonly symbol key object 1`] = ` +- Expected - 1 ++ Received + 1 + + Object { +- "b": 2, ++ "b": 1, + Symbol(key): Object { + "a": 1, + }, + } +`; diff --git a/packages/jest-matcher-utils/src/__tests__/deepCyclicCopyReplaceable.test.ts b/packages/jest-matcher-utils/src/__tests__/deepCyclicCopyReplaceable.test.ts index 63af3a7dce3d..3e647b4de61d 100644 --- a/packages/jest-matcher-utils/src/__tests__/deepCyclicCopyReplaceable.test.ts +++ b/packages/jest-matcher-utils/src/__tests__/deepCyclicCopyReplaceable.test.ts @@ -43,13 +43,20 @@ test('convert accessor descriptor into value descriptor', () => { }); }); -test('skips non-enumerables', () => { +test('shuold not skips non-enumerables', () => { const obj = {}; Object.defineProperty(obj, 'foo', {enumerable: false, value: 'bar'}); const copy = deepCyclicCopyReplaceable(obj); - expect(Object.getOwnPropertyDescriptors(copy)).toEqual({}); + expect(Object.getOwnPropertyDescriptors(copy)).toEqual({ + foo: { + configurable: true, + enumerable: false, + value: 'bar', + writable: true, + }, + }); }); test('copies symbols', () => { @@ -113,3 +120,22 @@ test('return same value for built-in object type except array, map and object', expect(deepCyclicCopyReplaceable(regexp)).toBe(regexp); expect(deepCyclicCopyReplaceable(set)).toBe(set); }); + +test('should copy object symbol key property', () => { + const symbolKey = Symbol.for('key'); + expect(deepCyclicCopyReplaceable({[symbolKey]: 1})).toEqual({[symbolKey]: 1}); +}); + +test('should set writable, configurable to true', () => { + const a = {}; + Object.defineProperty(a, 'key', { + configurable: false, + enumerable: true, + value: 1, + writable: false, + }); + const copied = deepCyclicCopyReplaceable(a); + expect(Object.getOwnPropertyDescriptors(copied)).toEqual({ + key: {configurable: true, enumerable: true, value: 1, writable: true}, + }); +}); diff --git a/packages/jest-matcher-utils/src/__tests__/deepCyclicCopyReplaceableDom.test.ts b/packages/jest-matcher-utils/src/__tests__/deepCyclicCopyReplaceableDom.test.ts new file mode 100644 index 000000000000..31536d9c4372 --- /dev/null +++ b/packages/jest-matcher-utils/src/__tests__/deepCyclicCopyReplaceableDom.test.ts @@ -0,0 +1,29 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @jest-environment jsdom + */ +/* eslint-env browser*/ +import deepCyclicCopyReplaceable from '../deepCyclicCopyReplaceable'; + +test('should copy dom element', () => { + const div = document.createElement('div'); + const copied = deepCyclicCopyReplaceable(div); + expect(copied).toEqual(div); + expect(div === copied).toBe(false); //assert reference is not the same +}); + +test('should copy complex element', () => { + const div = document.createElement('div'); + const span = document.createElement('span'); + div.setAttribute('id', 'div'); + div.innerText = 'this is div'; + div.appendChild(span); + const copied = deepCyclicCopyReplaceable(div); + expect(copied).toEqual(div); + expect(div === copied).toBe(false); //assert reference is not the same + expect(div.children[0] === copied.children[0]).toBe(false); //assert reference is not the same +}); diff --git a/packages/jest-matcher-utils/src/__tests__/printDiffOrStringify.test.ts b/packages/jest-matcher-utils/src/__tests__/printDiffOrStringify.test.ts index 41b61c4b9fdc..874d38ec6068 100644 --- a/packages/jest-matcher-utils/src/__tests__/printDiffOrStringify.test.ts +++ b/packages/jest-matcher-utils/src/__tests__/printDiffOrStringify.test.ts @@ -50,6 +50,29 @@ describe('printDiffOrStringify', () => { expect(testDiffOrStringify(expected, received)).toMatchSnapshot(); }); + test('object contain readonly symbol key object', () => { + const expected = {b: 2}; + const received = {b: 1}; + const symbolKey = Symbol.for('key'); + Object.defineProperty(expected, symbolKey, { + configurable: true, + enumerable: true, + value: { + a: 1, + }, + writable: false, + }); + Object.defineProperty(received, symbolKey, { + configurable: true, + enumerable: true, + value: { + a: 1, + }, + writable: false, + }); + expect(testDiffOrStringify(expected, received)).toMatchSnapshot(); + }); + describe('MAX_DIFF_STRING_LENGTH', () => { const lessChange = INVERTED_COLOR('single '); const less = 'single line'; diff --git a/packages/jest-matcher-utils/src/deepCyclicCopyReplaceable.ts b/packages/jest-matcher-utils/src/deepCyclicCopyReplaceable.ts index 6849735a7f2c..2a22bcfddaf4 100644 --- a/packages/jest-matcher-utils/src/deepCyclicCopyReplaceable.ts +++ b/packages/jest-matcher-utils/src/deepCyclicCopyReplaceable.ts @@ -5,6 +5,8 @@ * LICENSE file in the root directory of this source tree. */ +import {plugins} from 'pretty-format'; + const builtInObject = [ Array, Buffer, @@ -42,6 +44,8 @@ export default function deepCyclicCopyReplaceable( return deepCyclicCopyMap(value, cycles); } else if (isBuiltInObject(value)) { return value; + } else if (plugins.DOMElement.test(value)) { + return (((value as unknown) as Element).cloneNode(true) as unknown) as T; } else { return deepCyclicCopyObject(value, cycles); } @@ -55,25 +59,33 @@ function deepCyclicCopyObject(object: T, cycles: WeakMap): T { cycles.set(object, newObject); - Object.keys(descriptors).forEach(key => { - if (descriptors[key].enumerable) { - descriptors[key] = { + const newDescriptors = [ + ...Object.keys(descriptors), + ...Object.getOwnPropertySymbols(descriptors), + ].reduce( + //@ts-expect-error because typescript do not support symbol key in object + //https://github.com/microsoft/TypeScript/issues/1863 + (newDescriptors: {[x: string]: PropertyDescriptor}, key: string) => { + const enumerable = descriptors[key].enumerable; + + newDescriptors[key] = { configurable: true, - enumerable: true, + enumerable, value: deepCyclicCopyReplaceable( // this accesses the value or getter, depending. We just care about the value anyways, and this allows us to not mess with accessors // it has the side effect of invoking the getter here though, rather than copying it over - (object as Record)[key], + (object as Record)[key], cycles, ), writable: true, }; - } else { - delete descriptors[key]; - } - }); - - return Object.defineProperties(newObject, descriptors); + return newDescriptors; + }, + {}, + ); + //@ts-expect-error because typescript do not support symbol key in object + //https://github.com/microsoft/TypeScript/issues/1863 + return Object.defineProperties(newObject, newDescriptors); } function deepCyclicCopyArray(array: Array, cycles: WeakMap): T {