From f8932f6acb21c0d56e9b4da50588f6501aeae4f6 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Mon, 4 May 2020 17:08:10 -0400 Subject: [PATCH 1/3] Use single spread call when loose transforming object spread --- .../src/index.js | 10 +++++ .../assignment}/input.js | 0 .../assignment}/output.js | 0 .../expression/exec.js | 9 ++++ .../expression/input.js | 16 +++++++ .../expression/output.js | 28 ++++++++++++ .../no-getOwnPropertyDescriptors/exec.js | 3 ++ .../no-object-assign-exec/exec.js | 43 +++++++++++++++++++ .../options.json | 0 .../side-effect/exec.js | 12 ++++++ .../side-effect/input.js | 16 +++++++ .../side-effect/output.js | 30 +++++++++++++ .../string-exec/exec.js | 3 ++ .../variable-declaration/input.js | 1 + .../variable-declaration/output.js | 1 + .../assignment}/input.js | 2 - .../assignment}/output.js | 4 -- .../object-spread-loose/expression/exec.js | 9 ++++ .../object-spread-loose/expression/input.js | 16 +++++++ .../object-spread-loose/expression/output.js | 35 +++++++++++++++ .../no-getOwnPropertyDescriptors/exec.js | 3 ++ .../no-object-assign-exec/exec.js | 43 +++++++++++++++++++ .../fixtures/object-spread-loose/options.json | 5 +++ .../object-spread-loose/side-effect/exec.js | 12 ++++++ .../object-spread-loose/side-effect/input.js | 16 +++++++ .../object-spread-loose/side-effect/output.js | 35 +++++++++++++++ .../object-spread-loose/string-exec/exec.js | 3 ++ .../variable-declaration/input.js | 1 + .../variable-declaration/output.js | 3 ++ .../object-spread/loose-mode/options.json | 3 -- .../no-getOwnPropertyDescriptors/exec.js | 3 -- 31 files changed, 353 insertions(+), 12 deletions(-) rename packages/babel-plugin-proposal-object-rest-spread/test/fixtures/{object-spread/loose-mode-builtins => object-spread-loose-builtins/assignment}/input.js (100%) rename packages/babel-plugin-proposal-object-rest-spread/test/fixtures/{object-spread/loose-mode-builtins => object-spread-loose-builtins/assignment}/output.js (100%) create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/expression/exec.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/expression/input.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/expression/output.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/no-getOwnPropertyDescriptors/exec.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/no-object-assign-exec/exec.js rename packages/babel-plugin-proposal-object-rest-spread/test/fixtures/{object-spread/loose-mode-builtins => object-spread-loose-builtins}/options.json (100%) create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/side-effect/exec.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/side-effect/input.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/side-effect/output.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/string-exec/exec.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/variable-declaration/input.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/variable-declaration/output.js rename packages/babel-plugin-proposal-object-rest-spread/test/fixtures/{object-spread/loose-mode => object-spread-loose/assignment}/input.js (70%) rename packages/babel-plugin-proposal-object-rest-spread/test/fixtures/{object-spread/loose-mode => object-spread-loose/assignment}/output.js (52%) create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/expression/exec.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/expression/input.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/expression/output.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/no-getOwnPropertyDescriptors/exec.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/no-object-assign-exec/exec.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/options.json create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/side-effect/exec.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/side-effect/input.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/side-effect/output.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/string-exec/exec.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/variable-declaration/input.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/variable-declaration/output.js delete mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/loose-mode/options.json diff --git a/packages/babel-plugin-proposal-object-rest-spread/src/index.js b/packages/babel-plugin-proposal-object-rest-spread/src/index.js index 1ad0862a55cd..784a4b67cbdc 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/src/index.js +++ b/packages/babel-plugin-proposal-object-rest-spread/src/index.js @@ -579,6 +579,16 @@ export default declare((api, opts) => { return; } + // In loose mode, we don't want to make multiple calls. We're assuming + // that the spread objects either don't use getters, or that the + // getters are pure and don't depend on the order of evaluation. + if (loose) { + if (hadProps) { + exp.arguments.push(obj); + } + return; + } + exp = t.callExpression(t.cloneNode(helper), [ exp, // If we have static props, we need to insert an empty object diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/loose-mode-builtins/input.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/assignment/input.js similarity index 100% rename from packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/loose-mode-builtins/input.js rename to packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/assignment/input.js diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/loose-mode-builtins/output.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/assignment/output.js similarity index 100% rename from packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/loose-mode-builtins/output.js rename to packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/assignment/output.js diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/expression/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/expression/exec.js new file mode 100644 index 000000000000..b57d8d3222b9 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/expression/exec.js @@ -0,0 +1,9 @@ +var log = []; + +var a = { + ...{ get foo() { log.push(1); } }, + get bar() { log.push(2); } +}; + +// Loose mode uses regular Get, not GetOwnProperty. +expect(log).toEqual([1, 2]); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/expression/input.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/expression/input.js new file mode 100644 index 000000000000..da4e7b689505 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/expression/input.js @@ -0,0 +1,16 @@ +var a; +var b; +var c; +var d; +var x; +var y; + +({ x, ...y, a, ...b, c }); + +({ ...Object.prototype }); + +({ ...{ foo: 'bar' } }); + +({ ...{ foo: 'bar' }, ...{ bar: 'baz' } }); + +({ ...{ get foo () { return 'foo' } } }); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/expression/output.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/expression/output.js new file mode 100644 index 000000000000..d0e6aa43389d --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/expression/output.js @@ -0,0 +1,28 @@ +var a; +var b; +var c; +var d; +var x; +var y; +Object.assign({ + x +}, y, { + a +}, b, { + c +}); +Object.assign({}, Object.prototype); +Object.assign({}, { + foo: 'bar' +}); +Object.assign({}, { + foo: 'bar' +}, { + bar: 'baz' +}); +Object.assign({}, { + get foo() { + return 'foo'; + } + +}); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/no-getOwnPropertyDescriptors/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/no-getOwnPropertyDescriptors/exec.js new file mode 100644 index 000000000000..2bf9a9c4e166 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/no-getOwnPropertyDescriptors/exec.js @@ -0,0 +1,3 @@ +Object.getOwnPropertyDescriptors = null; + +({ ...{ a: 1 }, b: 1, ...{} }); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/no-object-assign-exec/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/no-object-assign-exec/exec.js new file mode 100644 index 000000000000..f7c9991ec034 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/no-object-assign-exec/exec.js @@ -0,0 +1,43 @@ +"use strict"; +Object.defineProperty(Object.prototype, 'NOSET', { + get(value) { + // noop + }, +}); + +Object.defineProperty(Object.prototype, 'NOWRITE', { + writable: false, + value: 'abc', +}); + +const obj = { NOSET: 123 }; +// this won't work as expected if transformed as Object.assign (or equivalent) +// because those trigger object setters (spread don't) +expect(() => { + const objSpread = { ...obj }; +}).toThrow(); + +const obj2 = { NOWRITE: 456 }; +// this throws `TypeError: Cannot assign to read only property 'NOWRITE'` +// if transformed as Object.assign (or equivalent) because those use *assignment* for creating properties +// (spread defines them) +expect(() => { + const obj2Spread = { ...obj2 }; +}).toThrow(); + +const KEY = Symbol('key'); +const obj3Spread = { ...{ get foo () { return 'bar' } }, [KEY]: 'symbol' }; +expect(Object.getOwnPropertyDescriptor(obj3Spread, 'foo').value).toBe('bar'); +expect(Object.getOwnPropertyDescriptor(obj3Spread, KEY).value).toBe('symbol'); + +const obj4Spread = { ...Object.prototype }; +expect(Object.getOwnPropertyDescriptor(obj4Spread, 'hasOwnProperty')).toBeUndefined(); + +expect(() => ({ ...null, ...undefined })).not.toThrow(); + +const o = Object.create(null); +o.a = 'foo'; +o.__proto__ = []; +const o2 = { ...o }; +// Loose will do o2.__proto__ = [] +expect(Array.isArray(Object.getPrototypeOf(o2))).toBe(true); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/loose-mode-builtins/options.json b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/options.json similarity index 100% rename from packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/loose-mode-builtins/options.json rename to packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/options.json diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/side-effect/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/side-effect/exec.js new file mode 100644 index 000000000000..088ceb03bcd6 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/side-effect/exec.js @@ -0,0 +1,12 @@ +var k = { a: 1, b: 2 }; +var o = { a: 3, ...k, b: k.a++ }; +// Loose will evaluate the static `b: k.a++` before spreading `...k`. +// It should be {a: 1, b: 1} +expect(o).toEqual({a: 2, b: 1}); + +var k = { a: 1, get b() { l = { z: 9 }; return 2; } }; +var l = { c: 3 }; +var o = { ...k, ...l }; +// Loose will evaluate the `l` before spreading `...k`. +// It should be {a: 1, b: 2, z: 9} +expect(o).toEqual({ a: 1, b: 2, c: 3 }); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/side-effect/input.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/side-effect/input.js new file mode 100644 index 000000000000..f2c856b9fcd4 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/side-effect/input.js @@ -0,0 +1,16 @@ +var k = { a: 1, b: 2 }; +var o = { a: 3, ...k, b: k.a++ }; + +var pureA = {}; +var pureB = {}; +var pureC = {}; +var pureD = {}; +var pureE = {}; + +function impureFunc() { + console.log('hello') +} + +var output = { ...pureA, get foo() {}, get bar() {}, ...pureB, ...pureC, ...impureFunc(), ...pureD, pureD } + +var simpleOutput = { ...pureA, test: '1', ...pureB, } \ No newline at end of file diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/side-effect/output.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/side-effect/output.js new file mode 100644 index 000000000000..947c4d6bc458 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/side-effect/output.js @@ -0,0 +1,30 @@ +var k = { + a: 1, + b: 2 +}; +var o = Object.assign({ + a: 3 +}, k, { + b: k.a++ +}); +var pureA = {}; +var pureB = {}; +var pureC = {}; +var pureD = {}; +var pureE = {}; + +function impureFunc() { + console.log('hello'); +} + +var output = Object.assign({}, pureA, { + get foo() {}, + + get bar() {} + +}, pureB, pureC, impureFunc(), pureD, { + pureD +}); +var simpleOutput = Object.assign({}, pureA, { + test: '1' +}, pureB); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/string-exec/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/string-exec/exec.js new file mode 100644 index 000000000000..bff6fc07971f --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/string-exec/exec.js @@ -0,0 +1,3 @@ +expect([...'']).toHaveLength(0); +expect([...'abc']).toHaveLength(3); +expect([...'def']).toMatchObject(['d','e','f']); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/variable-declaration/input.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/variable-declaration/input.js new file mode 100644 index 000000000000..a16dd23cb95f --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/variable-declaration/input.js @@ -0,0 +1 @@ +var z = { ...x }; diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/variable-declaration/output.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/variable-declaration/output.js new file mode 100644 index 000000000000..1c9d310a4830 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/variable-declaration/output.js @@ -0,0 +1 @@ +var z = Object.assign({}, x); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/loose-mode/input.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/assignment/input.js similarity index 70% rename from packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/loose-mode/input.js rename to packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/assignment/input.js index f05d6d6b8917..093a8e9f9351 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/loose-mode/input.js +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/assignment/input.js @@ -5,5 +5,3 @@ var z; z = { x, ...y }; z = { x, w: { ...y } }; - -const { q, ...rest } = z; diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/loose-mode/output.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/assignment/output.js similarity index 52% rename from packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/loose-mode/output.js rename to packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/assignment/output.js index d33583d41054..4db6d9f35237 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/loose-mode/output.js +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/assignment/output.js @@ -1,5 +1,3 @@ -function _objectWithoutPropertiesLoose(source, excluded) { if (source == null) return {}; var target = {}; var sourceKeys = Object.keys(source); var key, i; for (i = 0; i < sourceKeys.length; i++) { key = sourceKeys[i]; if (excluded.indexOf(key) >= 0) continue; target[key] = source[key]; } return target; } - function _extends() { _extends = Object.assign || function (target) { for (var i = 1; i < arguments.length; i++) { var source = arguments[i]; for (var key in source) { if (Object.prototype.hasOwnProperty.call(source, key)) { target[key] = source[key]; } } } return target; }; return _extends.apply(this, arguments); } var x; @@ -12,5 +10,3 @@ z = { x, w: _extends({}, y) }; - -const rest = _objectWithoutPropertiesLoose(z, ["q"]); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/expression/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/expression/exec.js new file mode 100644 index 000000000000..b57d8d3222b9 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/expression/exec.js @@ -0,0 +1,9 @@ +var log = []; + +var a = { + ...{ get foo() { log.push(1); } }, + get bar() { log.push(2); } +}; + +// Loose mode uses regular Get, not GetOwnProperty. +expect(log).toEqual([1, 2]); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/expression/input.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/expression/input.js new file mode 100644 index 000000000000..da4e7b689505 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/expression/input.js @@ -0,0 +1,16 @@ +var a; +var b; +var c; +var d; +var x; +var y; + +({ x, ...y, a, ...b, c }); + +({ ...Object.prototype }); + +({ ...{ foo: 'bar' } }); + +({ ...{ foo: 'bar' }, ...{ bar: 'baz' } }); + +({ ...{ get foo () { return 'foo' } } }); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/expression/output.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/expression/output.js new file mode 100644 index 000000000000..bc98a8d781d5 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/expression/output.js @@ -0,0 +1,35 @@ +function _extends() { _extends = Object.assign || function (target) { for (var i = 1; i < arguments.length; i++) { var source = arguments[i]; for (var key in source) { if (Object.prototype.hasOwnProperty.call(source, key)) { target[key] = source[key]; } } } return target; }; return _extends.apply(this, arguments); } + +var a; +var b; +var c; +var d; +var x; +var y; + +_extends({ + x +}, y, { + a +}, b, { + c +}); + +_extends({}, Object.prototype); + +_extends({}, { + foo: 'bar' +}); + +_extends({}, { + foo: 'bar' +}, { + bar: 'baz' +}); + +_extends({}, { + get foo() { + return 'foo'; + } + +}); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/no-getOwnPropertyDescriptors/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/no-getOwnPropertyDescriptors/exec.js new file mode 100644 index 000000000000..2bf9a9c4e166 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/no-getOwnPropertyDescriptors/exec.js @@ -0,0 +1,3 @@ +Object.getOwnPropertyDescriptors = null; + +({ ...{ a: 1 }, b: 1, ...{} }); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/no-object-assign-exec/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/no-object-assign-exec/exec.js new file mode 100644 index 000000000000..f7c9991ec034 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/no-object-assign-exec/exec.js @@ -0,0 +1,43 @@ +"use strict"; +Object.defineProperty(Object.prototype, 'NOSET', { + get(value) { + // noop + }, +}); + +Object.defineProperty(Object.prototype, 'NOWRITE', { + writable: false, + value: 'abc', +}); + +const obj = { NOSET: 123 }; +// this won't work as expected if transformed as Object.assign (or equivalent) +// because those trigger object setters (spread don't) +expect(() => { + const objSpread = { ...obj }; +}).toThrow(); + +const obj2 = { NOWRITE: 456 }; +// this throws `TypeError: Cannot assign to read only property 'NOWRITE'` +// if transformed as Object.assign (or equivalent) because those use *assignment* for creating properties +// (spread defines them) +expect(() => { + const obj2Spread = { ...obj2 }; +}).toThrow(); + +const KEY = Symbol('key'); +const obj3Spread = { ...{ get foo () { return 'bar' } }, [KEY]: 'symbol' }; +expect(Object.getOwnPropertyDescriptor(obj3Spread, 'foo').value).toBe('bar'); +expect(Object.getOwnPropertyDescriptor(obj3Spread, KEY).value).toBe('symbol'); + +const obj4Spread = { ...Object.prototype }; +expect(Object.getOwnPropertyDescriptor(obj4Spread, 'hasOwnProperty')).toBeUndefined(); + +expect(() => ({ ...null, ...undefined })).not.toThrow(); + +const o = Object.create(null); +o.a = 'foo'; +o.__proto__ = []; +const o2 = { ...o }; +// Loose will do o2.__proto__ = [] +expect(Array.isArray(Object.getPrototypeOf(o2))).toBe(true); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/options.json b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/options.json new file mode 100644 index 000000000000..7f3c40b6a6e8 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/options.json @@ -0,0 +1,5 @@ +{ + "plugins": [ + ["proposal-object-rest-spread", { "loose": true }] + ] +} diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/side-effect/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/side-effect/exec.js new file mode 100644 index 000000000000..088ceb03bcd6 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/side-effect/exec.js @@ -0,0 +1,12 @@ +var k = { a: 1, b: 2 }; +var o = { a: 3, ...k, b: k.a++ }; +// Loose will evaluate the static `b: k.a++` before spreading `...k`. +// It should be {a: 1, b: 1} +expect(o).toEqual({a: 2, b: 1}); + +var k = { a: 1, get b() { l = { z: 9 }; return 2; } }; +var l = { c: 3 }; +var o = { ...k, ...l }; +// Loose will evaluate the `l` before spreading `...k`. +// It should be {a: 1, b: 2, z: 9} +expect(o).toEqual({ a: 1, b: 2, c: 3 }); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/side-effect/input.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/side-effect/input.js new file mode 100644 index 000000000000..f2c856b9fcd4 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/side-effect/input.js @@ -0,0 +1,16 @@ +var k = { a: 1, b: 2 }; +var o = { a: 3, ...k, b: k.a++ }; + +var pureA = {}; +var pureB = {}; +var pureC = {}; +var pureD = {}; +var pureE = {}; + +function impureFunc() { + console.log('hello') +} + +var output = { ...pureA, get foo() {}, get bar() {}, ...pureB, ...pureC, ...impureFunc(), ...pureD, pureD } + +var simpleOutput = { ...pureA, test: '1', ...pureB, } \ No newline at end of file diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/side-effect/output.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/side-effect/output.js new file mode 100644 index 000000000000..2d9042115407 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/side-effect/output.js @@ -0,0 +1,35 @@ +function _extends() { _extends = Object.assign || function (target) { for (var i = 1; i < arguments.length; i++) { var source = arguments[i]; for (var key in source) { if (Object.prototype.hasOwnProperty.call(source, key)) { target[key] = source[key]; } } } return target; }; return _extends.apply(this, arguments); } + +var k = { + a: 1, + b: 2 +}; + +var o = _extends({ + a: 3 +}, k, { + b: k.a++ +}); + +var pureA = {}; +var pureB = {}; +var pureC = {}; +var pureD = {}; +var pureE = {}; + +function impureFunc() { + console.log('hello'); +} + +var output = _extends({}, pureA, { + get foo() {}, + + get bar() {} + +}, pureB, pureC, impureFunc(), pureD, { + pureD +}); + +var simpleOutput = _extends({}, pureA, { + test: '1' +}, pureB); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/string-exec/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/string-exec/exec.js new file mode 100644 index 000000000000..bff6fc07971f --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/string-exec/exec.js @@ -0,0 +1,3 @@ +expect([...'']).toHaveLength(0); +expect([...'abc']).toHaveLength(3); +expect([...'def']).toMatchObject(['d','e','f']); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/variable-declaration/input.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/variable-declaration/input.js new file mode 100644 index 000000000000..a16dd23cb95f --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/variable-declaration/input.js @@ -0,0 +1 @@ +var z = { ...x }; diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/variable-declaration/output.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/variable-declaration/output.js new file mode 100644 index 000000000000..73be92643eb7 --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/variable-declaration/output.js @@ -0,0 +1,3 @@ +function _extends() { _extends = Object.assign || function (target) { for (var i = 1; i < arguments.length; i++) { var source = arguments[i]; for (var key in source) { if (Object.prototype.hasOwnProperty.call(source, key)) { target[key] = source[key]; } } } return target; }; return _extends.apply(this, arguments); } + +var z = _extends({}, x); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/loose-mode/options.json b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/loose-mode/options.json deleted file mode 100644 index 036ba18ea68f..000000000000 --- a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/loose-mode/options.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "plugins": [["proposal-object-rest-spread", { "loose": true }]] -} diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/no-getOwnPropertyDescriptors/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/no-getOwnPropertyDescriptors/exec.js index 0a3b32098cde..2bf9a9c4e166 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/no-getOwnPropertyDescriptors/exec.js +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/no-getOwnPropertyDescriptors/exec.js @@ -1,6 +1,3 @@ -const oldGOPDs = Object.getOwnPropertyDescriptors; Object.getOwnPropertyDescriptors = null; ({ ...{ a: 1 }, b: 1, ...{} }); - -Object.getOwnPropertyDescriptors = oldGOPDs; From 64da98dd8298a43cedd01904f4f0cef0acf4d58f Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Wed, 6 May 2020 23:30:12 -0400 Subject: [PATCH 2/3] Undo changes to getOwnPropertyDescriptors tests --- .../no-getOwnPropertyDescriptors/exec.js | 7 ++++++- .../no-getOwnPropertyDescriptors/exec.js | 8 +++++++- .../object-spread/no-getOwnPropertyDescriptors/exec.js | 8 +++++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/no-getOwnPropertyDescriptors/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/no-getOwnPropertyDescriptors/exec.js index 2bf9a9c4e166..d4870569e0c0 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/no-getOwnPropertyDescriptors/exec.js +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/no-getOwnPropertyDescriptors/exec.js @@ -1,3 +1,8 @@ +const oldGOPDs = Object.getOwnPropertyDescriptors; Object.getOwnPropertyDescriptors = null; -({ ...{ a: 1 }, b: 1, ...{} }); +try { + ({ ...{ a: 1 }, b: 1, ...{} }); +} finally { + Object.getOwnPropertyDescriptors = oldGOPDs; +} diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/no-getOwnPropertyDescriptors/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/no-getOwnPropertyDescriptors/exec.js index 2bf9a9c4e166..e4674d812a50 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/no-getOwnPropertyDescriptors/exec.js +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/no-getOwnPropertyDescriptors/exec.js @@ -1,3 +1,9 @@ +const oldGOPDs = Object.getOwnPropertyDescriptors; Object.getOwnPropertyDescriptors = null; -({ ...{ a: 1 }, b: 1, ...{} }); +try { + ({ ...{ a: 1 }, b: 1, ...{} }); +} finally { + Object.getOwnPropertyDescriptors = oldGOPDs; +} + diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/no-getOwnPropertyDescriptors/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/no-getOwnPropertyDescriptors/exec.js index 2bf9a9c4e166..e4674d812a50 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/no-getOwnPropertyDescriptors/exec.js +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/no-getOwnPropertyDescriptors/exec.js @@ -1,3 +1,9 @@ +const oldGOPDs = Object.getOwnPropertyDescriptors; Object.getOwnPropertyDescriptors = null; -({ ...{ a: 1 }, b: 1, ...{} }); +try { + ({ ...{ a: 1 }, b: 1, ...{} }); +} finally { + Object.getOwnPropertyDescriptors = oldGOPDs; +} + From 654e0a3e8fd57873aabad510976e2e11857a11c4 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Wed, 6 May 2020 23:30:58 -0400 Subject: [PATCH 3/3] Use unique object property to avoid polluted contexts --- .../no-object-assign-exec/exec.js | 11 +++++++---- .../no-object-assign-exec/exec.js | 11 +++++++---- .../object-spread/no-object-assign-exec/exec.js | 12 ++++++++---- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/no-object-assign-exec/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/no-object-assign-exec/exec.js index f7c9991ec034..b8efc6d372e1 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/no-object-assign-exec/exec.js +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose-builtins/no-object-assign-exec/exec.js @@ -1,23 +1,26 @@ "use strict"; -Object.defineProperty(Object.prototype, 'NOSET', { +const NOSET = `NOSET${__filename}`; +const NOWRITE = `NOWRITE${__filename}`; + +Object.defineProperty(Object.prototype, NOSET, { get(value) { // noop }, }); -Object.defineProperty(Object.prototype, 'NOWRITE', { +Object.defineProperty(Object.prototype, NOWRITE, { writable: false, value: 'abc', }); -const obj = { NOSET: 123 }; +const obj = { [NOSET]: 123 }; // this won't work as expected if transformed as Object.assign (or equivalent) // because those trigger object setters (spread don't) expect(() => { const objSpread = { ...obj }; }).toThrow(); -const obj2 = { NOWRITE: 456 }; +const obj2 = { [NOWRITE]: 456 }; // this throws `TypeError: Cannot assign to read only property 'NOWRITE'` // if transformed as Object.assign (or equivalent) because those use *assignment* for creating properties // (spread defines them) diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/no-object-assign-exec/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/no-object-assign-exec/exec.js index f7c9991ec034..b8efc6d372e1 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/no-object-assign-exec/exec.js +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread-loose/no-object-assign-exec/exec.js @@ -1,23 +1,26 @@ "use strict"; -Object.defineProperty(Object.prototype, 'NOSET', { +const NOSET = `NOSET${__filename}`; +const NOWRITE = `NOWRITE${__filename}`; + +Object.defineProperty(Object.prototype, NOSET, { get(value) { // noop }, }); -Object.defineProperty(Object.prototype, 'NOWRITE', { +Object.defineProperty(Object.prototype, NOWRITE, { writable: false, value: 'abc', }); -const obj = { NOSET: 123 }; +const obj = { [NOSET]: 123 }; // this won't work as expected if transformed as Object.assign (or equivalent) // because those trigger object setters (spread don't) expect(() => { const objSpread = { ...obj }; }).toThrow(); -const obj2 = { NOWRITE: 456 }; +const obj2 = { [NOWRITE]: 456 }; // this throws `TypeError: Cannot assign to read only property 'NOWRITE'` // if transformed as Object.assign (or equivalent) because those use *assignment* for creating properties // (spread defines them) diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/no-object-assign-exec/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/no-object-assign-exec/exec.js index 29afff994622..e358e69cfeee 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/no-object-assign-exec/exec.js +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/no-object-assign-exec/exec.js @@ -1,20 +1,24 @@ -Object.defineProperty(Object.prototype, 'NOSET', { +"use strict"; +const NOSET = `NOSET${__filename}`; +const NOWRITE = `NOWRITE${__filename}`; + +Object.defineProperty(Object.prototype, NOSET, { set(value) { // noop }, }); -Object.defineProperty(Object.prototype, 'NOWRITE', { +Object.defineProperty(Object.prototype, NOWRITE, { writable: false, value: 'abc', }); -const obj = { NOSET: 123 }; +const obj = { [NOSET]: 123 }; // this wouldn't work as expected if transformed as Object.assign (or equivalent) // because those trigger object setters (spread don't) const objSpread = { ...obj }; -const obj2 = { NOSET: 123, NOWRITE: 456 }; +const obj2 = { NOSET: 123, [NOWRITE]: 456 }; // this line would throw `TypeError: Cannot assign to read only property 'NOWRITE'` // if transformed as Object.assign (or equivalent) because those use *assignment* for creating properties // (spread defines them)