Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Computed properties should keep original definition order #15232

Merged
merged 12 commits into from Dec 5, 2022
18 changes: 9 additions & 9 deletions Makefile
Expand Up @@ -189,15 +189,15 @@ test-test262-update-allowlist:


new-version-checklist:
# @echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
# @echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
# @echo "!!!!!! !!!!!!"
# @echo "!!!!!! Add any message here, and UNCOMMENT THESE LINES! !!!!!!"
# @echo "!!!!!! !!!!!!"
# @echo "!!!!!! !!!!!!"
# @echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
# @echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
# @exit 1
@echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
@echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
@echo "!!!!!! !!!!!!"
@echo "!!!!!! Update the minVersion of !!!!!!"
@echo "!!!!!! packages/babel-helpers/src/helpers/defineAccessor.js !!!!!!"
@echo "!!!!!! !!!!!!"
@echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
@echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
@exit 1

new-version:
$(MAKE) new-version-checklist
Expand Down
4 changes: 4 additions & 0 deletions packages/babel-helpers/src/helpers-generated.ts
Expand Up @@ -45,6 +45,10 @@ export default Object.freeze({
"7.20.5",
'export default function _checkInRHS(value){if(Object(value)!==value)throw TypeError("right-hand side of \'in\' should be an object, got "+(null!==value?typeof value:"null"));return value}',
),
defineAccessor: helper(
"7.20.6",
"export default function _defineAccessor(type,obj,key,fn){var desc={configurable:!0,enumerable:!0};return desc[type]=fn,Object.defineProperty(obj,key,desc)}",
),
iterableToArrayLimit: helper(
"7.0.0-beta.0",
'export default function _iterableToArrayLimit(arr,i){var _i=null==arr?null:"undefined"!=typeof Symbol&&arr[Symbol.iterator]||arr["@@iterator"];if(null!=_i){var _s,_e,_x,_r,_arr=[],_n=!0,_d=!1;try{if(_x=(_i=_i.call(arr)).next,0===i){if(Object(_i)!==_i)return;_n=!1}else for(;!(_n=(_s=_x.call(_i)).done)&&(_arr.push(_s.value),_arr.length!==i);_n=!0);}catch(err){_d=!0,_e=err}finally{try{if(!_n&&null!=_i.return&&(_r=_i.return(),Object(_r)!==_r))return}finally{if(_d)throw _e}}return _arr}}',
Expand Down
8 changes: 8 additions & 0 deletions packages/babel-helpers/src/helpers/defineAccessor.js
@@ -0,0 +1,8 @@
/* @minVersion 7.20.6 */
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is the correct minVersion...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need to update it when releasing, could you update the Makefile like I did in 80c6889?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 16758f3.


export default function _defineAccessor(type, obj, key, fn) {
var desc = { configurable: true, enumerable: true };
// type should be "get" or "set"
desc[type] = fn;
return Object.defineProperty(obj, key, desc);
}
79 changes: 25 additions & 54 deletions packages/babel-plugin-transform-computed-properties/src/index.ts
@@ -1,5 +1,6 @@
import { types as t } from "@babel/core";
import type { PluginPass } from "@babel/core";
import { declare } from "@babel/helper-plugin-utils";
import { template, types as t, type PluginPass } from "@babel/core";
import type { Scope } from "@babel/traverse";

export interface Options {
Expand All @@ -12,7 +13,6 @@ type PropertyInfo = {
body: t.Statement[];
computedProps: t.ObjectMember[];
initPropExpression: t.ObjectExpression;
getMutatorId: () => t.Identifier;
state: PluginPass;
};

Expand All @@ -26,11 +26,6 @@ export default declare((api, options: Options) => {
? pushComputedPropsLoose
: pushComputedPropsSpec;

const buildMutatorMapAssign = template.statements(`
MUTATOR_MAP_REF[KEY] = MUTATOR_MAP_REF[KEY] || {};
MUTATOR_MAP_REF[KEY].KIND = VALUE;
`);

/**
* Get value of an object member under object expression.
* Returns a function expression if prop is a ObjectMethod.
Expand Down Expand Up @@ -72,31 +67,34 @@ export default declare((api, options: Options) => {
);
}

function pushMutatorDefine(
{ body, getMutatorId, scope }: PropertyInfo,
function pushAccessorDefine(
{ body, computedProps, initPropExpression, objId, state }: PropertyInfo,
prop: t.ObjectMethod,
) {
let key =
const key =
!prop.computed && t.isIdentifier(prop.key)
? t.stringLiteral(prop.key.name)
: prop.key;

const maybeMemoise = scope.maybeGenerateMemoised(key);
if (maybeMemoise) {
if (computedProps.length === 1) {
return t.callExpression(state.addHelper("defineAccessor"), [
t.stringLiteral(prop.kind),
initPropExpression,
key,
getValue(prop),
]);
} else {
body.push(
t.expressionStatement(t.assignmentExpression("=", maybeMemoise, key)),
t.expressionStatement(
t.callExpression(state.addHelper("defineAccessor"), [
t.stringLiteral(prop.kind),
t.cloneNode(objId),
key,
getValue(prop),
]),
),
);
key = maybeMemoise;
}

body.push(
...buildMutatorMapAssign({
MUTATOR_MAP_REF: getMutatorId(),
KEY: t.cloneNode(key),
VALUE: getValue(prop),
KIND: t.identifier(prop.kind),
}),
);
}

function pushComputedPropsLoose(info: PropertyInfo) {
Expand All @@ -105,7 +103,8 @@ export default declare((api, options: Options) => {
t.isObjectMethod(prop) &&
(prop.kind === "get" || prop.kind === "set")
) {
pushMutatorDefine(info, prop);
const single = pushAccessorDefine(info, prop);
if (single) return single;
} else {
pushAssign(t.cloneNode(info.objId), prop, info.body);
}
Expand All @@ -123,7 +122,8 @@ export default declare((api, options: Options) => {
t.isObjectMethod(prop) &&
(prop.kind === "get" || prop.kind === "set")
) {
pushMutatorDefine(info, prop);
const single = pushAccessorDefine(info, prop);
if (single) return single;
} else {
// the value of ObjectProperty in ObjectExpression must be an expression
const value = getValue(prop) as t.Expression;
Expand Down Expand Up @@ -195,44 +195,15 @@ export default declare((api, options: Options) => {
]),
);

let mutatorRef: t.Identifier;

const getMutatorId = function () {
if (!mutatorRef) {
mutatorRef = scope.generateUidIdentifier("mutatorMap");

body.push(
t.variableDeclaration("var", [
t.variableDeclarator(mutatorRef, t.objectExpression([])),
]),
);
}

return t.cloneNode(mutatorRef);
};

const single = pushComputedProps({
scope,
objId,
body,
computedProps,
initPropExpression,
getMutatorId,
state,
});

if (mutatorRef) {
body.push(
t.expressionStatement(
t.callExpression(
state.addHelper("defineEnumerableProperties"),
[t.cloneNode(objId), t.cloneNode(mutatorRef)],
),
),
);
}

// @ts-expect-error todo(flow->ts) `void` should not be used as variable
if (single) {
path.replaceWith(single);
} else {
Expand Down
@@ -1,10 +1,10 @@
var _foobar, _foobar2, _test, _test2, _obj, _mutatorMap;
var obj = (_obj = {}, _foobar = foobar, _mutatorMap = {}, _mutatorMap[_foobar] = _mutatorMap[_foobar] || {}, _mutatorMap[_foobar].get = function () {
var _obj;
var obj = (_obj = {}, babelHelpers.defineAccessor("get", _obj, foobar, function () {
return "foobar";
}, _foobar2 = foobar, _mutatorMap[_foobar2] = _mutatorMap[_foobar2] || {}, _mutatorMap[_foobar2].set = function (x) {
}), babelHelpers.defineAccessor("set", _obj, foobar, function (x) {
console.log(x);
}, _test = "test", _mutatorMap[_test] = _mutatorMap[_test] || {}, _mutatorMap[_test].get = function () {
}), babelHelpers.defineAccessor("get", _obj, "test", function () {
return "regular getter after computed property";
}, _test2 = "test", _mutatorMap[_test2] = _mutatorMap[_test2] || {}, _mutatorMap[_test2].set = function (x) {
}), babelHelpers.defineAccessor("set", _obj, "test", function (x) {
console.log(x);
}, babelHelpers.defineEnumerableProperties(_obj, _mutatorMap), _obj);
}), _obj);
@@ -0,0 +1,3 @@
var obj = {
get ["x" + foo]() { return "heh"; }
};
@@ -0,0 +1,3 @@
var obj = babelHelpers.defineAccessor("get", {}, "x" + foo, function () {
return "heh";
});
@@ -1,5 +1,5 @@
var _foo, _mutatorMap;
var _foo;
var k = Symbol();
var foo = (_foo = {}, _foo[Symbol.iterator] = "foobar", _mutatorMap = {}, _mutatorMap[k] = _mutatorMap[k] || {}, _mutatorMap[k].get = function () {
var foo = (_foo = {}, _foo[Symbol.iterator] = "foobar", babelHelpers.defineAccessor("get", _foo, k, function () {
return k;
}, babelHelpers.defineEnumerableProperties(_foo, _mutatorMap), _foo);
}), _foo);
@@ -1,10 +1,10 @@
var _foobar, _foobar2, _test, _test2, _obj, _mutatorMap;
var obj = (_obj = {}, _foobar = foobar, _mutatorMap = {}, _mutatorMap[_foobar] = _mutatorMap[_foobar] || {}, _mutatorMap[_foobar].get = function () {
var _obj;
var obj = (_obj = {}, babelHelpers.defineAccessor("get", _obj, foobar, function () {
return "foobar";
}, _foobar2 = foobar, _mutatorMap[_foobar2] = _mutatorMap[_foobar2] || {}, _mutatorMap[_foobar2].set = function (x) {
}), babelHelpers.defineAccessor("set", _obj, foobar, function (x) {
console.log(x);
}, _test = "test", _mutatorMap[_test] = _mutatorMap[_test] || {}, _mutatorMap[_test].get = function () {
}), babelHelpers.defineAccessor("get", _obj, "test", function () {
return "regular getter after computed property";
}, _test2 = "test", _mutatorMap[_test2] = _mutatorMap[_test2] || {}, _mutatorMap[_test2].set = function (x) {
}), babelHelpers.defineAccessor("set", _obj, "test", function (x) {
console.log(x);
}, babelHelpers.defineEnumerableProperties(_obj, _mutatorMap), _obj);
}), _obj);
@@ -0,0 +1,17 @@
var a = {
get ["x"]() { return 0; },
["y"]: 1,
};
expect(Object.keys(a)).toStrictEqual(["x", "y"]);

var b = {
get ["x"]() { return 0; },
["x"]: 1,
};
expect(b.x).toBe(1);

var x = 1;
var y = { x, get x() { return 0; }, x };
expect(y.x).toBe(1);
y.x = 2;
expect(y.x).toBe(2);
@@ -0,0 +1,13 @@
var a = {
get ["x"]() { return 0; },
["y"]: 1,
};

var b = {
get ["x"]() { return 0; },
["x"]: 1,
};

var x = 1;
var y = { x, get x() { return 0; }, x };
y.x = 2;
@@ -0,0 +1,6 @@
{
"plugins": [
"transform-duplicate-keys",
"transform-computed-properties"
]
}
@@ -0,0 +1,14 @@
var _a, _b, _y;
var a = (_a = {}, babelHelpers.defineAccessor("get", _a, "x", function () {
return 0;
}), babelHelpers.defineProperty(_a, "y", 1), _a);
var b = (_b = {}, babelHelpers.defineAccessor("get", _b, "x", function () {
return 0;
}), babelHelpers.defineProperty(_b, "x", 1), _b);
var x = 1;
var y = (_y = {
x
}, babelHelpers.defineAccessor("get", _y, "x", function () {
return 0;
}), babelHelpers.defineProperty(_y, "x", x), _y);
y.x = 2;
@@ -0,0 +1,3 @@
var obj = {
get ["x" + foo]() { return "heh"; }
};
@@ -0,0 +1,3 @@
var obj = babelHelpers.defineAccessor("get", {}, "x" + foo, function () {
return "heh";
});
@@ -1,5 +1,5 @@
var _foo, _mutatorMap;
var _foo;
var k = Symbol();
var foo = (_foo = {}, babelHelpers.defineProperty(_foo, Symbol.iterator, "foobar"), _mutatorMap = {}, _mutatorMap[k] = _mutatorMap[k] || {}, _mutatorMap[k].get = function () {
var foo = (_foo = {}, babelHelpers.defineProperty(_foo, Symbol.iterator, "foobar"), babelHelpers.defineAccessor("get", _foo, k, function () {
return k;
}, babelHelpers.defineEnumerableProperties(_foo, _mutatorMap), _foo);
}), _foo);
9 changes: 9 additions & 0 deletions packages/babel-runtime-corejs2/package.json
Expand Up @@ -90,6 +90,15 @@
"./helpers/checkInRHS.js"
],
"./helpers/esm/checkInRHS": "./helpers/esm/checkInRHS.js",
"./helpers/defineAccessor": [
{
"node": "./helpers/defineAccessor.js",
"import": "./helpers/esm/defineAccessor.js",
"default": "./helpers/defineAccessor.js"
},
"./helpers/defineAccessor.js"
],
"./helpers/esm/defineAccessor": "./helpers/esm/defineAccessor.js",
"./helpers/iterableToArrayLimit": [
{
"node": "./helpers/iterableToArrayLimit.js",
Expand Down
9 changes: 9 additions & 0 deletions packages/babel-runtime-corejs3/package.json
Expand Up @@ -89,6 +89,15 @@
"./helpers/checkInRHS.js"
],
"./helpers/esm/checkInRHS": "./helpers/esm/checkInRHS.js",
"./helpers/defineAccessor": [
{
"node": "./helpers/defineAccessor.js",
"import": "./helpers/esm/defineAccessor.js",
"default": "./helpers/defineAccessor.js"
},
"./helpers/defineAccessor.js"
],
"./helpers/esm/defineAccessor": "./helpers/esm/defineAccessor.js",
"./helpers/iterableToArrayLimit": [
{
"node": "./helpers/iterableToArrayLimit.js",
Expand Down
9 changes: 9 additions & 0 deletions packages/babel-runtime/package.json
Expand Up @@ -89,6 +89,15 @@
"./helpers/checkInRHS.js"
],
"./helpers/esm/checkInRHS": "./helpers/esm/checkInRHS.js",
"./helpers/defineAccessor": [
{
"node": "./helpers/defineAccessor.js",
"import": "./helpers/esm/defineAccessor.js",
"default": "./helpers/defineAccessor.js"
},
"./helpers/defineAccessor.js"
],
"./helpers/esm/defineAccessor": "./helpers/esm/defineAccessor.js",
"./helpers/iterableToArrayLimit": [
{
"node": "./helpers/iterableToArrayLimit.js",
Expand Down