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 the support of arguments in private get/set method #16307

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
67 changes: 51 additions & 16 deletions packages/babel-helper-create-class-features-plugin/src/fields.ts
Expand Up @@ -1261,19 +1261,6 @@ function buildPrivateMethodDeclaration(
const isGetter = getId && !getterDeclared && params.length === 0;
const isSetter = setId && !setterDeclared && params.length > 0;

if (
(process.env.BABEL_8_BREAKING || newHelpers(file)) &&
(isGetter || isSetter) &&
!privateFieldsAsProperties
) {
const thisArg = prop.get("body").scope.generateUidIdentifier("this");
// eslint-disable-next-line @typescript-eslint/no-use-before-define
prop.traverse(thisContextVisitor, {
thisRef: thisArg,
});
params.unshift(t.cloneNode(thisArg));
}

let declId = methodId;

if (isGetter) {
Expand All @@ -1294,6 +1281,43 @@ function buildPrivateMethodDeclaration(
declId = id;
}

if (
(process.env.BABEL_8_BREAKING || newHelpers(file)) &&
(isGetter || isSetter) &&
!privateFieldsAsProperties
) {
const thisArg = prop.get("body").scope.generateUidIdentifier("this");
const state: ReplaceThisState = {
thisPaths: [],
};
// eslint-disable-next-line @typescript-eslint/no-use-before-define
prop.traverse(thisContextVisitor, state);
if (state.hasArguments) {
return t.variableDeclaration("var", [
t.variableDeclarator(
t.cloneNode(declId),
t.callExpression(file.addHelper("curryThis"), [
inheritPropComments(
t.functionExpression(
t.cloneNode(declId),
// @ts-expect-error params for ClassMethod has TSParameterProperty
params,
body,
generator,
async,
),
prop,
),
]),
),
]);
} else {
for (const path of state.thisPaths) {
path.replaceWith(t.cloneNode(thisArg));
}
params.unshift(t.cloneNode(thisArg));
}
}
return inheritPropComments(
t.functionDeclaration(
t.cloneNode(declId),
Expand All @@ -1308,7 +1332,9 @@ function buildPrivateMethodDeclaration(
}

type ReplaceThisState = {
thisRef: t.Identifier;
thisRef?: t.Identifier;
thisPaths?: NodePath<t.ThisExpression>[];
hasArguments?: boolean;
needsClassRef?: boolean;
innerBinding?: t.Identifier | null;
};
Expand All @@ -1317,6 +1343,11 @@ type ReplaceInnerBindingReferenceState = ReplaceThisState;

const thisContextVisitor = traverse.visitors.merge<ReplaceThisState>([
{
Identifier(path, state) {
if (path.node.name === "arguments") {
state.hasArguments = true;
}
},
UnaryExpression(path) {
// Replace `delete this` with `true`
const { node } = path;
Expand All @@ -1329,7 +1360,11 @@ const thisContextVisitor = traverse.visitors.merge<ReplaceThisState>([
},
ThisExpression(path, state) {
state.needsClassRef = true;
path.replaceWith(t.cloneNode(state.thisRef));
if (state.thisPaths) {
state.thisPaths.push(path);
} else {
path.replaceWith(t.cloneNode(state.thisRef));
}
},
MetaProperty(path) {
const { node, scope } = path;
Expand Down Expand Up @@ -1456,7 +1491,7 @@ export function buildFieldsInitNodes(
const instanceNodes: t.ExpressionStatement[] = [];
let lastInstanceNodeReturnsThis = false;
// These nodes are pure and can be moved to the closest statement position
const pureStaticNodes: t.FunctionDeclaration[] = [];
const pureStaticNodes: (t.FunctionDeclaration | t.VariableDeclaration)[] = [];
let classBindingNode: t.ExpressionStatement | null = null;

const getSuperRef = t.isIdentifier(superRef)
Expand Down
Expand Up @@ -269,7 +269,7 @@ export function createClassFeaturePlugin({
staticNodes: t.Statement[],
instanceNodes: t.ExpressionStatement[],
lastInstanceNodeReturnsThis: boolean,
pureStaticNodes: t.FunctionDeclaration[],
pureStaticNodes: (t.FunctionDeclaration | t.VariableDeclaration)[],
classBindingNode: t.Statement | null,
wrapClass: (path: NodePath<t.Class>) => NodePath;

Expand Down
5 changes: 5 additions & 0 deletions packages/babel-helpers/src/helpers-generated.ts
Expand Up @@ -108,6 +108,11 @@ export default Object.freeze({
"7.0.0-beta.0",
'import setPrototypeOf from"setPrototypeOf";import isNativeReflectConstruct from"isNativeReflectConstruct";export default function _construct(t,e,r){if(isNativeReflectConstruct())return Reflect.construct.apply(null,arguments);var o=[null];o.push.apply(o,e);var p=new(t.bind.apply(t,o));return r&&setPrototypeOf(p,r.prototype),p}',
),
// size: 103, gzip size: 103
curryThis: helper(
"7.24.0",
"export default function _curryThis(r){return function(n){return r.apply(n,[].slice.call(arguments,1))}}",
),
// size: 130, gzip size: 130
defineAccessor: helper(
"7.20.7",
Expand Down
7 changes: 7 additions & 0 deletions packages/babel-helpers/src/helpers/curryThis.ts
@@ -0,0 +1,7 @@
/* @minVersion 7.24.0 */

export default function _curryThis(fn: Function) {
return function (thisArg: any) {
return fn.apply(thisArg, [].slice.call(arguments, 1));
};
}

This comment was marked as outdated.

@@ -0,0 +1,32 @@
class Cl {
#privateField = "top secret string";

constructor() {
this.publicField = "not secret string";
}

get #privateFieldValue() {
expect(arguments[0]).toBeUndefined();
liuxingbaoyu marked this conversation as resolved.
Show resolved Hide resolved
return this.#privateField;
}

set #privateFieldValue(newValue) {
this.#privateField = arguments[0];
}

publicGetPrivateField() {
return this.#privateFieldValue;
}

publicSetPrivateField(newValue) {
this.#privateFieldValue = newValue;
}
}

const cl = new Cl();

expect(cl.publicGetPrivateField()).toEqual("top secret string");

cl.publicSetPrivateField("new secret string");
expect(cl.publicGetPrivateField()).toEqual("new secret string");

@@ -0,0 +1,23 @@
class Cl {
#privateField = "top secret string";

constructor() {
this.publicField = "not secret string";
}

get #privateFieldValue() {
return this.#privateField;
Copy link
Member

Choose a reason for hiding this comment

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

Use arguments somewhere here maybe, to show the outputs for getters too.

}

set #privateFieldValue(newValue) {
this.#privateField = arguments[0];
}

publicGetPrivateField() {
return this.#privateFieldValue;
}

publicSetPrivateField(newValue) {
this.#privateFieldValue = newValue;
}
}
@@ -0,0 +1,21 @@
var _privateField = /*#__PURE__*/new WeakMap();
var _Cl_brand = /*#__PURE__*/new WeakSet();
class Cl {
constructor() {
babelHelpers.classPrivateMethodInitSpec(this, _Cl_brand);
babelHelpers.classPrivateFieldInitSpec(this, _privateField, "top secret string");
this.publicField = "not secret string";
}
publicGetPrivateField() {
return babelHelpers.classPrivateGetter(_Cl_brand, this, _get_privateFieldValue);
}
publicSetPrivateField(newValue) {
babelHelpers.classPrivateSetter(_Cl_brand, _set_privateFieldValue, this, newValue);
}
}
function _get_privateFieldValue(_this) {
return babelHelpers.classPrivateFieldGet2(_privateField, _this);
}
var _set_privateFieldValue = babelHelpers.curryThis(function _set_privateFieldValue(newValue) {
babelHelpers.classPrivateFieldSet2(_privateField, this, arguments[0]);
});
9 changes: 9 additions & 0 deletions packages/babel-runtime-corejs2/package.json
Expand Up @@ -189,6 +189,15 @@
"./helpers/construct.js"
],
"./helpers/esm/construct": "./helpers/esm/construct.js",
"./helpers/curryThis": [
{
"node": "./helpers/curryThis.js",
"import": "./helpers/esm/curryThis.js",
"default": "./helpers/curryThis.js"
},
"./helpers/curryThis.js"
],
"./helpers/esm/curryThis": "./helpers/esm/curryThis.js",
"./helpers/defineAccessor": [
{
"node": "./helpers/defineAccessor.js",
Expand Down
9 changes: 9 additions & 0 deletions packages/babel-runtime-corejs3/package.json
Expand Up @@ -188,6 +188,15 @@
"./helpers/construct.js"
],
"./helpers/esm/construct": "./helpers/esm/construct.js",
"./helpers/curryThis": [
{
"node": "./helpers/curryThis.js",
"import": "./helpers/esm/curryThis.js",
"default": "./helpers/curryThis.js"
},
"./helpers/curryThis.js"
],
"./helpers/esm/curryThis": "./helpers/esm/curryThis.js",
"./helpers/defineAccessor": [
{
"node": "./helpers/defineAccessor.js",
Expand Down
9 changes: 9 additions & 0 deletions packages/babel-runtime/package.json
Expand Up @@ -188,6 +188,15 @@
"./helpers/construct.js"
],
"./helpers/esm/construct": "./helpers/esm/construct.js",
"./helpers/curryThis": [
{
"node": "./helpers/curryThis.js",
"import": "./helpers/esm/curryThis.js",
"default": "./helpers/curryThis.js"
},
"./helpers/curryThis.js"
],
"./helpers/esm/curryThis": "./helpers/esm/curryThis.js",
"./helpers/defineAccessor": [
{
"node": "./helpers/defineAccessor.js",
Expand Down