Skip to content

Commit

Permalink
Fix evaluation order with object spread (#11412)
Browse files Browse the repository at this point in the history
* Fix object spread runtime semantics

* limit this deoptimization

* optimize empty objects

* declare variables
  • Loading branch information
Zzzen committed Apr 23, 2020
1 parent 5f299ba commit af66929
Show file tree
Hide file tree
Showing 12 changed files with 144 additions and 1 deletion.
51 changes: 50 additions & 1 deletion packages/babel-plugin-proposal-object-rest-spread/src/index.js
Expand Up @@ -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 = [];

Expand Down Expand Up @@ -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);
},
},
};
Expand Down
@@ -1,3 +1,7 @@
var x;
var y;
var z;

z = { x, ...y };

z = { x, w: { ...y } };
Expand Up @@ -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);
Expand Down
@@ -1,3 +1,10 @@
var a;
var b;
var c;
var d;
var x;
var y;

({ x, ...y, a, ...b, c });

({ ...Object.prototype });
Expand Down
Expand Up @@ -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, {
Expand Down
@@ -1,3 +1,7 @@
var x;
var y;
var z;

z = { x, ...y };

z = { x, w: { ...y } };
@@ -1,3 +1,6 @@
var x;
var y;
var z;
z = Object.assign({
x
}, y);
Expand Down
@@ -1,3 +1,7 @@
var x;
var y;
var z;

z = { x, ...y };

z = { x, w: { ...y } };
Expand Down
Expand Up @@ -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);
Expand Down
@@ -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});
@@ -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, }
@@ -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);

0 comments on commit af66929

Please sign in to comment.