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

Implement setPublicClassFields and privateFieldsAsProperties assumptions #12497

Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions packages/babel-core/src/config/validation/options.js
Expand Up @@ -340,6 +340,7 @@ export const assumptionsNames = new Set<string>([
"newableArrowFunctions",
"noDocumentAll",
"objectRestNoSymbols",
"privateFieldsAsProperties",
"pureGetters",
"setClassMethods",
"setComputedProperties",
Expand Down
81 changes: 55 additions & 26 deletions packages/babel-helper-create-class-features-plugin/src/fields.js
Expand Up @@ -35,11 +35,16 @@ export function buildPrivateNamesMap(props) {
return privateNamesMap;
}

export function buildPrivateNamesNodes(privateNamesMap, loose, state) {
export function buildPrivateNamesNodes(
privateNamesMap,
privateFieldsAsProperties,
state,
) {
const initNodes = [];

for (const [name, value] of privateNamesMap) {
// In loose mode, both static and instance fields are transpiled using a
// When the privateFieldsAsProperties assumption is enabled,
// both static and instance fields are transpiled using a
// secret non-enumerable property. Hence, we also need to generate that
// key (using the classPrivateFieldLooseKey helper).
// In spec mode, only instance fields need a "private name" initializer
Expand All @@ -48,7 +53,7 @@ export function buildPrivateNamesNodes(privateNamesMap, loose, state) {
const { static: isStatic, method: isMethod, getId, setId } = value;
const isAccessor = getId || setId;
const id = t.cloneNode(value.id);
if (loose) {
if (privateFieldsAsProperties) {
initNodes.push(
template.statement.ast`
var ${id} = ${state.addHelper("classPrivateFieldLooseKey")}("${name}")
Expand Down Expand Up @@ -149,13 +154,13 @@ const privateInVisitor = privateNameVisitorFactory({
if (operator !== "in") return;
if (!path.get("left").isPrivateName()) return;

const { loose, privateNamesMap, redeclared } = this;
const { privateFieldsAsProperties, privateNamesMap, redeclared } = this;
const { name } = left.id;

if (!privateNamesMap.has(name)) return;
if (redeclared && redeclared.includes(name)) return;

if (loose) {
if (privateFieldsAsProperties) {
const { id } = privateNamesMap.get(name);
path.replaceWith(template.expression.ast`
Object.prototype.hasOwnProperty.call(${right}, ${t.cloneNode(id)})
Expand Down Expand Up @@ -361,13 +366,15 @@ export function transformPrivateNamesUsage(
ref,
path,
privateNamesMap,
loose,
{ privateFieldsAsProperties },
state,
) {
if (!privateNamesMap.size) return;

const body = path.get("body");
const handler = loose ? privateNameHandlerLoose : privateNameHandlerSpec;
const handler = privateFieldsAsProperties
? privateNameHandlerLoose
: privateNameHandlerSpec;

memberExpressionToFunctions(body, privateNameVisitor, {
privateNamesMap,
Expand All @@ -379,7 +386,7 @@ export function transformPrivateNamesUsage(
privateNamesMap,
classRef: ref,
file: state,
loose,
privateFieldsAsProperties,
});
}

Expand Down Expand Up @@ -561,7 +568,11 @@ function buildPrivateStaticMethodInitLoose(ref, prop, state, privateNamesMap) {
`;
}

function buildPrivateMethodDeclaration(prop, privateNamesMap, loose = false) {
function buildPrivateMethodDeclaration(
prop,
privateNamesMap,
privateFieldsAsProperties = false,
) {
const privateName = privateNamesMap.get(prop.node.key.id.name);
const {
id,
Expand Down Expand Up @@ -601,7 +612,7 @@ function buildPrivateMethodDeclaration(prop, privateNamesMap, loose = false) {
t.variableDeclarator(setId, methodValue),
]);
}
if (isStatic && !loose) {
if (isStatic && !privateFieldsAsProperties) {
return t.variableDeclaration("var", [
t.variableDeclarator(
t.cloneNode(id),
Expand Down Expand Up @@ -654,6 +665,8 @@ export function buildFieldsInitNodes(
props,
privateNamesMap,
state,
setPublicClassFields,
privateFieldsAsProperties,
loose,
) {
const staticNodes = [];
Expand All @@ -676,34 +689,34 @@ export function buildFieldsInitNodes(
}

switch (true) {
case isStatic && isPrivate && isField && loose:
case isStatic && isPrivate && isField && privateFieldsAsProperties:
needsClassRef = true;
staticNodes.push(
buildPrivateFieldInitLoose(t.cloneNode(ref), prop, privateNamesMap),
);
break;
case isStatic && isPrivate && isField && !loose:
case isStatic && isPrivate && isField && !privateFieldsAsProperties:
needsClassRef = true;
staticNodes.push(
buildPrivateStaticFieldInitSpec(prop, privateNamesMap),
);
break;
case isStatic && isPublic && isField && loose:
case isStatic && isPublic && isField && setPublicClassFields:
needsClassRef = true;
staticNodes.push(buildPublicFieldInitLoose(t.cloneNode(ref), prop));
break;
case isStatic && isPublic && isField && !loose:
case isStatic && isPublic && isField && !setPublicClassFields:
needsClassRef = true;
staticNodes.push(
buildPublicFieldInitSpec(t.cloneNode(ref), prop, state),
);
break;
case isInstance && isPrivate && isField && loose:
case isInstance && isPrivate && isField && privateFieldsAsProperties:
instanceNodes.push(
buildPrivateFieldInitLoose(t.thisExpression(), prop, privateNamesMap),
);
break;
case isInstance && isPrivate && isField && !loose:
case isInstance && isPrivate && isField && !privateFieldsAsProperties:
instanceNodes.push(
buildPrivateInstanceFieldInitSpec(
t.thisExpression(),
Expand All @@ -712,7 +725,7 @@ export function buildFieldsInitNodes(
),
);
break;
case isInstance && isPrivate && isMethod && loose:
case isInstance && isPrivate && isMethod && privateFieldsAsProperties:
instanceNodes.unshift(
buildPrivateMethodInitLoose(
t.thisExpression(),
Expand All @@ -721,10 +734,14 @@ export function buildFieldsInitNodes(
),
);
staticNodes.push(
buildPrivateMethodDeclaration(prop, privateNamesMap, loose),
buildPrivateMethodDeclaration(
prop,
privateNamesMap,
privateFieldsAsProperties,
),
);
break;
case isInstance && isPrivate && isMethod && !loose:
case isInstance && isPrivate && isMethod && !privateFieldsAsProperties:
instanceNodes.unshift(
buildPrivateInstanceMethodInitSpec(
t.thisExpression(),
Expand All @@ -733,19 +750,27 @@ export function buildFieldsInitNodes(
),
);
staticNodes.push(
buildPrivateMethodDeclaration(prop, privateNamesMap, loose),
buildPrivateMethodDeclaration(
prop,
privateNamesMap,
privateFieldsAsProperties,
),
);
break;
case isStatic && isPrivate && isMethod && !loose:
case isStatic && isPrivate && isMethod && !privateFieldsAsProperties:
needsClassRef = true;
staticNodes.push(
buildPrivateStaticFieldInitSpec(prop, privateNamesMap),
);
staticNodes.unshift(
buildPrivateMethodDeclaration(prop, privateNamesMap, loose),
buildPrivateMethodDeclaration(
prop,
privateNamesMap,
privateFieldsAsProperties,
),
);
break;
case isStatic && isPrivate && isMethod && loose:
case isStatic && isPrivate && isMethod && privateFieldsAsProperties:
needsClassRef = true;
staticNodes.push(
buildPrivateStaticMethodInitLoose(
Expand All @@ -756,13 +781,17 @@ export function buildFieldsInitNodes(
),
);
staticNodes.unshift(
buildPrivateMethodDeclaration(prop, privateNamesMap, loose),
buildPrivateMethodDeclaration(
prop,
privateNamesMap,
privateFieldsAsProperties,
),
);
break;
case isInstance && isPublic && isField && loose:
case isInstance && isPublic && isField && setPublicClassFields:
instanceNodes.push(buildPublicFieldInitLoose(t.thisExpression(), prop));
break;
case isInstance && isPublic && isField && !loose:
case isInstance && isPublic && isField && !setPublicClassFields:
instanceNodes.push(
buildPublicFieldInitSpec(t.thisExpression(), prop, state),
);
Expand Down
42 changes: 40 additions & 2 deletions packages/babel-helper-create-class-features-plugin/src/index.js
Expand Up @@ -36,7 +36,37 @@ export function createClassFeaturePlugin({
feature,
loose,
manipulateOptions,
// TODO(Babel 8): Remove the default falue
JLHwung marked this conversation as resolved.
Show resolved Hide resolved
api = { assumption: () => {} },
}) {
const setPublicClassFields = api.assumption("setPublicClassFields");
Copy link
Contributor

@JLHwung JLHwung Dec 14, 2020

Choose a reason for hiding this comment

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

This will throw on @babel/core 7.12 + @babel/plugin-class-properties 7.13 because api is defined but api.assumption is not provided.
Never mind, I always forget the fact that api.assumption has been provided by helper plugin utils.

const privateFieldsAsProperties = api.assumption("privateFieldsAsProperties");

if (loose) {
const explicit = [];

if (setPublicClassFields !== undefined) {
explicit.push(`"setPublicClassFields"`);
}
if (privateFieldsAsProperties !== undefined) {
explicit.push(`"privateFieldsAsProperties"`);
}
if (explicit.length !== 0) {
console.warn(
`[${name}]: You are using the "loose: true" option and you are` +
` explicitly setting a value for the ${explicit.join(" and ")}` +
` assumption${explicit.length > 1 ? "s" : ""}. The "loose" option` +
` can cause incompatibilities with the other class features` +
` plugins, so it's recommended that you replace it with the` +
` following top-level option:\n` +
`\t"assumptions": {\n` +
`\t\t"setPublicClassFields": true,\n` +
`\t\t"privateFieldsAsProperties": true\n` +
`\t}`,
);
}
}

return {
name,
manipulateOptions,
Expand Down Expand Up @@ -151,11 +181,17 @@ export function createClassFeaturePlugin({
const privateNamesMap = buildPrivateNamesMap(props);
const privateNamesNodes = buildPrivateNamesNodes(
privateNamesMap,
loose,
privateFieldsAsProperties ?? loose,
state,
);

transformPrivateNamesUsage(ref, path, privateNamesMap, loose, state);
transformPrivateNamesUsage(
ref,
path,
privateNamesMap,
{ privateFieldsAsProperties: privateFieldsAsProperties ?? loose },
state,
);

let keysNodes, staticNodes, instanceNodes, wrapClass;

Expand All @@ -175,6 +211,8 @@ export function createClassFeaturePlugin({
props,
privateNamesMap,
state,
setPublicClassFields ?? loose,
privateFieldsAsProperties ?? loose,
loose,
));
}
Expand Down
@@ -0,0 +1,3 @@
class A {
foo;
}
@@ -0,0 +1,9 @@
{
"validateLogs": true,
"plugins": [
["proposal-class-properties", { "loose": true }]
],
"assumptions": {
"setPublicClassFields": true
}
}
@@ -0,0 +1,6 @@
class A {
constructor() {
this.foo = void 0;
}

}
@@ -0,0 +1,5 @@
[proposal-class-properties]: You are using the "loose: true" option and you are explicitly setting a value for the "setPublicClassFields" assumption. The "loose" option can cause incompatibilities with the other class features plugins, so it's recommended that you replace it with the following top-level option:
"assumptions": {
"setPublicClassFields": true,
"privateFieldsAsProperties": true
}
Expand Up @@ -12,6 +12,7 @@ export default declare((api, options) => {
return createClassFeaturePlugin({
name: "proposal-class-properties",

api,
feature: FEATURES.fields,
loose: options.loose,

Expand Down
@@ -0,0 +1,38 @@
const actualOrder = [];

const track = i => {
actualOrder.push(i);
return i;
};

class MyClass {
static [track(1)] = track(10);
[track(2)] = track(13);
get [track(3)]() {
return "foo";
}
set [track(4)](value) {
this.bar = value;
}
[track(5)] = track(14);
static [track(6)] = track(11);
static [track(7)] = track(12);
[track(8)]() {}
[track(9)] = track(15);
}

const inst = new MyClass();

const expectedOrder = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15];
expect(actualOrder).toEqual(expectedOrder);

expect(MyClass[1]).toBe(10);
expect(inst[2]).toBe(13);
expect(inst[3]).toBe("foo");
inst[4] = "baz";
expect(inst.bar).toBe("baz");
expect(inst[5]).toBe(14);
expect(MyClass[6]).toBe(11);
expect(MyClass[7]).toBe(12);
expect(typeof inst[8]).toBe("function");
expect(inst[9]).toBe(15);
@@ -0,0 +1,25 @@
const foo = "foo";
const bar = () => {};
const four = 4;

class MyClass {
static [one()] = "test";
static [2 * 4 + 7] = "247";
static [2 * four + 7] = "247";
static [2 * four + seven] = "247";
[null] = "null";
[undefined] = "undefined";
[void 0] = "void 0";
get ["whatever"]() {}
set ["whatever"](value) {}
get [computed()]() {}
set [computed()](value) {}
["test" + one]() {}
static [10]() {}
[/regex/] = "regex";
[foo] = "foo";
[bar] = "bar";
[baz] = "baz";
[`template`] = "template";
[`template${expression}`] = "template-with-expression";
}