Skip to content

Commit

Permalink
Fix diffing object contain readonly symbol key object (#10414)
Browse files Browse the repository at this point in the history
  • Loading branch information
WeiAnAn authored Aug 19, 2020
1 parent 200adc0 commit 2e30f52
Show file tree
Hide file tree
Showing 12 changed files with 241 additions and 19 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
51 changes: 51 additions & 0 deletions e2e/__tests__/__snapshots__/domDiffing.test.ts.snap
Original file line number Diff line number Diff line change
@@ -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 (<<REPLACED>>)
✕ compare span and div (<<REPLACED>>)
● use toBe compare two div
expect(received).toBe(expected) // Object.is equality
If it should pass with deep equality, replace \\"toBe\\" with \\"toStrictEqual\\"
Expected: <div />
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
- <span />
+ <div />
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: <<REPLACED>>
Ran all test suites."
`;
16 changes: 16 additions & 0 deletions e2e/__tests__/domDiffing.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
19 changes: 19 additions & 0 deletions e2e/dom-diffing/__tests__/dom.test.js
Original file line number Diff line number Diff line change
@@ -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'));
});
3 changes: 3 additions & 0 deletions e2e/dom-diffing/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"jest": {}
}
17 changes: 11 additions & 6 deletions packages/jest-matcher-utils/src/Replaceable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
24 changes: 24 additions & 0 deletions packages/jest-matcher-utils/src/__tests__/Replaceable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,3 +198,16 @@ exports[`printDiffOrStringify has no common after clean up chaff one-line 1`] =
Expected: <g>"delete"</>
Received: <r>"insert"</>
`;
exports[`printDiffOrStringify object contain readonly symbol key object 1`] = `
<g>- Expected - 1</>
<r>+ Received + 1</>
<d> Object {</>
<g>- "b": 2,</>
<r>+ "b": 1,</>
<d> Symbol(key): Object {</>
<d> "a": 1,</>
<d> },</>
<d> }</>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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},
});
});
Original file line number Diff line number Diff line change
@@ -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
});
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
34 changes: 23 additions & 11 deletions packages/jest-matcher-utils/src/deepCyclicCopyReplaceable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* LICENSE file in the root directory of this source tree.
*/

import {plugins} from 'pretty-format';

const builtInObject = [
Array,
Buffer,
Expand Down Expand Up @@ -42,6 +44,8 @@ export default function deepCyclicCopyReplaceable<T>(
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);
}
Expand All @@ -55,25 +59,33 @@ function deepCyclicCopyObject<T>(object: T, cycles: WeakMap<any, any>): 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<string, unknown>)[key],
(object as Record<string | symbol, unknown>)[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<T>(array: Array<T>, cycles: WeakMap<any, any>): T {
Expand Down

0 comments on commit 2e30f52

Please sign in to comment.