From af669297efd775016725ee39318cdb391cf00a21 Mon Sep 17 00:00:00 2001 From: Zen <843968788@qq.com> Date: Fri, 24 Apr 2020 05:59:35 +0800 Subject: [PATCH] Fix evaluation order with object spread (#11412) * Fix object spread runtime semantics * limit this deoptimization * optimize empty objects * declare variables --- .../src/index.js | 51 ++++++++++++++++++- .../object-spread/assignment/input.js | 4 ++ .../object-spread/assignment/output.js | 3 ++ .../object-spread/expression/input.js | 7 +++ .../object-spread/expression/output.js | 7 +++ .../loose-mode-builtins/input.js | 4 ++ .../loose-mode-builtins/output.js | 3 ++ .../object-spread/loose-mode/input.js | 4 ++ .../object-spread/loose-mode/output.js | 3 ++ .../object-spread/side-effect/exec.js | 4 ++ .../object-spread/side-effect/input.js | 16 ++++++ .../object-spread/side-effect/output.js | 39 ++++++++++++++ 12 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/side-effect/exec.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/side-effect/input.js create mode 100644 packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/side-effect/output.js 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 7c25ac399242..5899233567c4 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/src/index.js +++ b/packages/babel-plugin-proposal-object-rest-spread/src/index.js @@ -541,6 +541,9 @@ export default declare((api, opts) => { ObjectExpression(path, file) { if (!hasSpread(path.node)) return; + const { scope } = path; + + // a non-SpreadElement and SpreadElement striped array const args = []; let props = []; @@ -579,8 +582,54 @@ export default declare((api, opts) => { helper = file.addHelper("objectSpread"); } } + // We cannot call _objectSpread with more than two elements directly, since any element could cause side effects. For + // example: + // var k = { a: 1, b: 2 }; + // var o = { a: 3, ...k, b: k.a++ }; + // // expected: { a: 1, b: 1 } + // If we translate the above to `_objectSpread({ a: 3 }, k, { b: k.a++ })`, the `k.a++` will evaluate before + // `k` is spread and we end up with `{ a: 2, b: 1 }`. + // adapted from https://github.com/microsoft/TypeScript/blob/eb105efdcd6db8a73f5b983bf329cb7a5eee55e1/src/compiler/transformers/es2018.ts#L272 + const chunks = []; + let currentChunk = []; + for (let i = 0; i < args.length; i++) { + currentChunk.push(args[i]); + const isCurrentChunkEmptyObject = + currentChunk.length === 1 && + t.isObjectExpression(args[i]) && + args[i].properties.length === 0; + const isNextArgEffectful = + i < args.length - 1 && !scope.isPure(args[i + 1]); + // prevent current chunk from pollution unless current chunk is an empty object + if (!isCurrentChunkEmptyObject && isNextArgEffectful) { + chunks.push(currentChunk); + currentChunk = []; + } + } + + if (currentChunk.length) { + chunks.push(currentChunk); + currentChunk = []; + } + + let exp = t.callExpression(helper, chunks[0]); + let nthArg = chunks[0].length; + for (let i = 1; i < chunks.length; i++) { + // reference: packages/babel-helpers/src/helpers.js#objectSpread2 + if (nthArg % 2) { + exp = t.callExpression(helper, [exp, ...chunks[i]]); + } else { + exp = t.callExpression(helper, [ + exp, + t.objectExpression([]), + ...chunks[i], + ]); + } + + nthArg += chunks[i].length; + } - path.replaceWith(t.callExpression(helper, args)); + path.replaceWith(exp); }, }, }; diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/assignment/input.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/assignment/input.js index 6f0367fb7c70..093a8e9f9351 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/assignment/input.js +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/assignment/input.js @@ -1,3 +1,7 @@ +var x; +var y; +var z; + z = { x, ...y }; z = { x, w: { ...y } }; diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/assignment/output.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/assignment/output.js index a1f2970a3264..9f13316b8658 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/assignment/output.js +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/assignment/output.js @@ -4,6 +4,9 @@ function _objectSpread(target) { for (var i = 1; i < arguments.length; i++) { va function _defineProperty(obj, key, value) { if (key in obj) { Object.defineProperty(obj, key, { value: value, enumerable: true, configurable: true, writable: true }); } else { obj[key] = value; } return obj; } +var x; +var y; +var z; z = _objectSpread({ x }, y); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/expression/input.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/expression/input.js index a20f61241b3c..da4e7b689505 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/expression/input.js +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/expression/input.js @@ -1,3 +1,10 @@ +var a; +var b; +var c; +var d; +var x; +var y; + ({ x, ...y, a, ...b, c }); ({ ...Object.prototype }); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/expression/output.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/expression/output.js index a172df60bd16..b846f4badffe 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/expression/output.js +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/expression/output.js @@ -4,6 +4,13 @@ function _objectSpread(target) { for (var i = 1; i < arguments.length; i++) { va function _defineProperty(obj, key, value) { if (key in obj) { Object.defineProperty(obj, key, { value: value, enumerable: true, configurable: true, writable: true }); } else { obj[key] = value; } return obj; } +var a; +var b; +var c; +var d; +var x; +var y; + _objectSpread({ x }, y, { 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-mode-builtins/input.js index 6f0367fb7c70..093a8e9f9351 100644 --- 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-mode-builtins/input.js @@ -1,3 +1,7 @@ +var x; +var y; +var z; + z = { x, ...y }; z = { x, w: { ...y } }; 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-mode-builtins/output.js index 457fa6a6069d..8e9602898ed6 100644 --- 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-mode-builtins/output.js @@ -1,3 +1,6 @@ +var x; +var y; +var z; z = Object.assign({ x }, y); 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-mode/input.js index 07beed075a6c..f05d6d6b8917 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-mode/input.js @@ -1,3 +1,7 @@ +var x; +var y; +var z; + z = { x, ...y }; z = { x, w: { ...y } }; 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-mode/output.js index 1d477e3f1bb8..d33583d41054 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-mode/output.js @@ -2,6 +2,9 @@ function _objectWithoutPropertiesLoose(source, excluded) { if (source == null) r 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; +var y; +var z; z = _extends({ x }, y); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/side-effect/exec.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/side-effect/exec.js new file mode 100644 index 000000000000..efb73e7a98ca --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/side-effect/exec.js @@ -0,0 +1,4 @@ +var k = { a: 1, b: 2 }; +var o = { a: 3, ...k, b: k.a++ }; + +expect(o).toEqual({a: 1, b: 1}); diff --git a/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/side-effect/input.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/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/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/side-effect/output.js b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/side-effect/output.js new file mode 100644 index 000000000000..19ec3bc08cbe --- /dev/null +++ b/packages/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/side-effect/output.js @@ -0,0 +1,39 @@ +function ownKeys(object, enumerableOnly) { var keys = Object.keys(object); if (Object.getOwnPropertySymbols) { var symbols = Object.getOwnPropertySymbols(object); if (enumerableOnly) symbols = symbols.filter(function (sym) { return Object.getOwnPropertyDescriptor(object, sym).enumerable; }); keys.push.apply(keys, symbols); } return keys; } + +function _objectSpread(target) { for (var i = 1; i < arguments.length; i++) { var source = arguments[i] != null ? arguments[i] : {}; if (i % 2) { ownKeys(Object(source), true).forEach(function (key) { _defineProperty(target, key, source[key]); }); } else if (Object.getOwnPropertyDescriptors) { Object.defineProperties(target, Object.getOwnPropertyDescriptors(source)); } else { ownKeys(Object(source)).forEach(function (key) { Object.defineProperty(target, key, Object.getOwnPropertyDescriptor(source, key)); }); } } return target; } + +function _defineProperty(obj, key, value) { if (key in obj) { Object.defineProperty(obj, key, { value: value, enumerable: true, configurable: true, writable: true }); } else { obj[key] = value; } return obj; } + +var k = { + a: 1, + b: 2 +}; + +var o = _objectSpread(_objectSpread({ + a: 3 +}, k), {}, { + b: k.a++ +}); + +var pureA = {}; +var pureB = {}; +var pureC = {}; +var pureD = {}; +var pureE = {}; + +function impureFunc() { + console.log('hello'); +} + +var output = _objectSpread(_objectSpread(_objectSpread({}, pureA), {}, { + get foo() {}, + + get bar() {} + +}, pureB, {}, pureC, {}), impureFunc(), {}, pureD, { + pureD +}); + +var simpleOutput = _objectSpread({}, pureA, { + test: '1' +}, pureB);