Private class methods stage 3 (#8654)

* Add private method syntax support * Add private method spec support * Add private method loose support * Throw error if static private method is used * Add more isStatic & isMethod checks * Remove `writable:false` from private method inits `writable` is false by default. * Add private method func obj equality check * Throw if private accessor is used * Add check for fields === private method loose mode * Throw buildCodeFrameErrors instead of Errors * Move obj destructuring inside for loop * Remove "computed" from ClassPrivateMethod type def
babel · Nov 29, 2018 · 0859535 · 0859535
1 parent 6e39b58
commit 0859535
Show file tree

Hide file tree

Showing 40 changed files with 746 additions and 38 deletions.
diff --git a/packages/babel-generator/src/generators/classes.js b/packages/babel-generator/src/generators/classes.js
@@ -140,6 +140,12 @@ export function ClassMethod(node: Object) {
   this.print(node.body, node);
 }
 
+export function ClassPrivateMethod(node: Object) {
+  this._classMethodHead(node);
+  this.space();
+  this.print(node.body, node);
+}
+
 export function _classMethodHead(node) {
   this.printJoin(node.decorators, node);
 

diff --git a/packages/babel-generator/test/fixtures/types/ClassBody-MethodDefinition/input.js b/packages/babel-generator/test/fixtures/types/ClassBody-MethodDefinition/input.js
@@ -5,6 +5,15 @@ class Foo {
   get foo() {}
   set foo(bar) {}
 
+  async #foo() {}
+  #foo() {}
+  get #foo() {}
+  set #foo(bar) {}
+  * #foo() {}
+  async * #foo() {}
+  get #bar() {}
+  set #baz(taz) {}
+
   static async foo() {}
   static foo() {}
   static ["foo"]() {}
@@ -52,4 +61,4 @@ class Foo {
   get
   static
   () {}
-}
+}
diff --git a/packages/babel-generator/test/fixtures/types/ClassBody-MethodDefinition/options.json b/packages/babel-generator/test/fixtures/types/ClassBody-MethodDefinition/options.json
@@ -0,0 +1 @@
+{ "plugins": ["classPrivateMethods", "asyncGenerators"] }
diff --git a/packages/babel-generator/test/fixtures/types/ClassBody-MethodDefinition/output.js b/packages/babel-generator/test/fixtures/types/ClassBody-MethodDefinition/output.js
@@ -9,6 +9,22 @@ class Foo {
 
   set foo(bar) {}
 
+  async #foo() {}
+
+  #foo() {}
+
+  get #foo() {}
+
+  set #foo(bar) {}
+
+  *#foo() {}
+
+  async *#foo() {}
+
+  get #bar() {}
+
+  set #baz(taz) {}
+
   static async foo() {}
 
   static foo() {}

diff --git a/packages/babel-helpers/src/helpers.js b/packages/babel-helpers/src/helpers.js
@@ -1739,3 +1739,18 @@ helpers.decorate = helper("7.1.5")`
     return constructor;
   }
 `;
+
+helpers.classPrivateMethodGet = helper("7.1.6")`
+  export default function _classPrivateMethodGet(receiver, privateSet, fn) {
+    if (!privateSet.has(receiver)) {
+      throw new TypeError("attempted to get private field on non-instance");
+    }
+    return fn;
+  }
+`;
+
+helpers.classPrivateMethodSet = helper("7.1.6")`
+  export default function _classPrivateMethodSet() {
+    throw new TypeError("attempted to reassign private method");
+  }
+`;
diff --git a/packages/babel-plugin-class-features/src/features.js b/packages/babel-plugin-class-features/src/features.js
@@ -39,19 +39,43 @@ export function isLoose(file, feature) {
 }
 
 export function verifyUsedFeatures(path, file) {
+  if (hasDecorators(path) && !hasFeature(file, FEATURES.decorators)) {
+    throw path.buildCodeFrameError("Decorators are not enabled.");
+  }
+
   if (hasFeature(file, FEATURES.decorators)) {
     throw new Error(
       "@babel/plugin-class-features doesn't support decorators yet.",
     );
   }
-  if (hasFeature(file, FEATURES.privateMethods)) {
-    throw new Error(
-      "@babel/plugin-class-features doesn't support private methods yet.",
-    );
+
+  if (path.isClassPrivateMethod()) {
+    if (!hasFeature(file, FEATURES.privateMethods)) {
+      throw path.buildCodeFrameError("Class private methods are not enabled.");
+    }
+
+    if (path.node.static) {
+      throw path.buildCodeFrameError(
+        "@babel/plugin-class-features doesn't support class static private methods yet.",
+      );
+    }
+
+    if (path.node.kind !== "method") {
+      throw path.buildCodeFrameError(
+        "@babel/plugin-class-features doesn't support class private accessors yet.",
+      );
+    }
   }
 
-  if (hasDecorators(path) && !hasFeature(file, FEATURES.decorators)) {
-    throw path.buildCodeFrameError("Decorators are not enabled.");
+  if (
+    hasFeature(file, FEATURES.privateMethods) &&
+    hasFeature(file, FEATURES.fields) &&
+    isLoose(file, FEATURES.privateMethods) !== isLoose(file, FEATURES.fields)
+  ) {
+    throw path.buildCodeFrameError(
+      "'loose' mode configuration must be the same for both @babel/plugin-proposal-class-properties " +
+        "and @babel/plugin-proposal-private-methods",
+    );
   }
 
   if (path.isProperty()) {

diff --git a/packages/babel-plugin-class-features/src/fields.js b/packages/babel-plugin-class-features/src/fields.js
@@ -11,6 +11,10 @@ export function buildPrivateNamesMap(props) {
       privateNamesMap.set(name, {
         id: prop.scope.generateUidIdentifier(name),
         static: !!prop.node.static,
+        method: prop.isClassPrivateMethod(),
+        methodId: prop.isClassPrivateMethod()
+          ? prop.scope.generateUidIdentifier(name)
+          : undefined,
       });
     }
   }
@@ -20,20 +24,22 @@ export function buildPrivateNamesMap(props) {
 export function buildPrivateNamesNodes(privateNamesMap, loose, state) {
   const initNodes = [];
 
-  for (const [name, { id, static: isStatic }] of privateNamesMap) {
-    // In loose mode, both static and instance fields hare transpiled using a
+  for (const [name, value] of privateNamesMap) {
+    // In loose mode, 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 loose mode.
+    // key (using the classPrivateFieldLooseKey helper).
     // In spec mode, only instance fields need a "private name" initializer
-    // (the WeakMap), becase static fields are directly assigned to a variable
-    // in the buildPrivateStaticFieldInitSpec function.
-
+    // because static fields are directly assigned to a variable in the
+    // buildPrivateStaticFieldInitSpec function.
+    const { id, static: isStatic, method: isMethod } = value;
     if (loose) {
       initNodes.push(
         template.statement.ast`
           var ${id} = ${state.addHelper("classPrivateFieldLooseKey")}("${name}")
         `,
       );
+    } else if (isMethod && !isStatic) {
+      initNodes.push(template.statement.ast`var ${id} = new WeakSet();`);
     } else if (!isStatic) {
       initNodes.push(template.statement.ast`var ${id} = new WeakMap();`);
     }
@@ -42,7 +48,7 @@ export function buildPrivateNamesNodes(privateNamesMap, loose, state) {
   return initNodes;
 }
 
-// Traverses the class scope, handling private name references.  If an inner
+// Traverses the class scope, handling private name references. If an inner
 // class redeclares the same private name, it will hand off traversal to the
 // restricted visitor (which doesn't traverse the inner class's inner scope).
 const privateNameVisitor = {
@@ -61,7 +67,9 @@ const privateNameVisitor = {
     const body = path.get("body.body");
 
     for (const prop of body) {
-      if (!prop.isClassPrivateProperty()) continue;
+      if (!prop.isPrivate()) {
+        continue;
+      }
       if (!privateNamesMap.has(prop.node.key.id.name)) continue;
 
       // This class redeclares the private name.
@@ -108,13 +116,24 @@ const privateNameHandlerSpec = {
   get(member) {
     const { classRef, privateNamesMap, file } = this;
     const { name } = member.node.property.id;
-    const { id, static: isStatic } = privateNamesMap.get(name);
-
-    if (isStatic) {
+    const {
+      id,
+      static: isStatic,
+      method: isMethod,
+      methodId,
+    } = privateNamesMap.get(name);
+
+    if (isStatic && !isMethod) {
       return t.callExpression(
         file.addHelper("classStaticPrivateFieldSpecGet"),
         [this.receiver(member), t.cloneNode(classRef), t.cloneNode(id)],
       );
+    } else if (isMethod) {
+      return t.callExpression(file.addHelper("classPrivateMethodGet"), [
+        this.receiver(member),
+        t.cloneNode(id),
+        t.cloneNode(methodId),
+      ]);
     } else {
       return t.callExpression(file.addHelper("classPrivateFieldGet"), [
         this.receiver(member),
@@ -126,13 +145,17 @@ const privateNameHandlerSpec = {
   set(member, value) {
     const { classRef, privateNamesMap, file } = this;
     const { name } = member.node.property.id;
-    const { id, static: isStatic } = privateNamesMap.get(name);
+    const { id, static: isStatic, method: isMethod } = privateNamesMap.get(
+      name,
+    );
 
-    if (isStatic) {
+    if (isStatic && !isMethod) {
       return t.callExpression(
         file.addHelper("classStaticPrivateFieldSpecSet"),
         [this.receiver(member), t.cloneNode(classRef), t.cloneNode(id), value],
       );
+    } else if (isMethod) {
+      return t.callExpression(file.addHelper("classPrivateMethodSet"), []);
     } else {
       return t.callExpression(file.addHelper("classPrivateFieldSet"), [
         this.receiver(member),
@@ -231,6 +254,25 @@ function buildPrivateStaticFieldInitSpec(prop, privateNamesMap) {
   `;
 }
 
+function buildPrivateMethodInitLoose(ref, prop, privateNamesMap) {
+  const { methodId, id } = privateNamesMap.get(prop.node.key.id.name);
+
+  return template.statement.ast`
+    Object.defineProperty(${ref}, ${id}, {
+      // configurable is false by default
+      // enumerable is false by default
+      // writable is false by default
+      value: ${methodId.name}
+    });
+  `;
+}
+
+function buildPrivateInstanceMethodInitSpec(ref, prop, privateNamesMap) {
+  const { id } = privateNamesMap.get(prop.node.key.id.name);
+
+  return template.statement.ast`${id}.add(${ref})`;
+}
+
 function buildPublicFieldInitLoose(ref, prop) {
   const { key, computed } = prop.node;
   const value = prop.node.value || prop.scope.buildUndefinedNode();
@@ -257,6 +299,16 @@ function buildPublicFieldInitSpec(ref, prop, state) {
   );
 }
 
+function buildPrivateInstanceMethodDeclaration(prop, privateNamesMap) {
+  const { methodId } = privateNamesMap.get(prop.node.key.id.name);
+  const { params, body } = prop.node;
+  const methodValue = t.functionExpression(methodId, params, body);
+
+  return t.variableDeclaration("var", [
+    t.variableDeclarator(methodId, methodValue),
+  ]);
+}
+
 export function buildFieldsInitNodes(
   ref,
   props,
@@ -269,34 +321,34 @@ export function buildFieldsInitNodes(
 
   for (const prop of props) {
     const isStatic = prop.node.static;
-    const isPrivate = prop.isPrivate();
+    const isPrivateField = prop.isClassPrivateProperty();
+    const isPrivateMethod = prop.isClassPrivateMethod();
 
-    // Pattern matching please
     switch (true) {
-      case isStatic && isPrivate && loose:
+      case isStatic && isPrivateField && loose:
         staticNodes.push(
           buildPrivateFieldInitLoose(t.cloneNode(ref), prop, privateNamesMap),
         );
         break;
-      case isStatic && isPrivate && !loose:
+      case isStatic && isPrivateField && !loose:
         staticNodes.push(
           buildPrivateStaticFieldInitSpec(prop, privateNamesMap),
         );
         break;
-      case isStatic && !isPrivate && loose:
+      case isStatic && !isPrivateField && loose:
         staticNodes.push(buildPublicFieldInitLoose(t.cloneNode(ref), prop));
         break;
-      case isStatic && !isPrivate && !loose:
+      case isStatic && !isPrivateField && !loose:
         staticNodes.push(
           buildPublicFieldInitSpec(t.cloneNode(ref), prop, state),
         );
         break;
-      case !isStatic && isPrivate && loose:
+      case !isStatic && isPrivateField && loose:
         instanceNodes.push(
           buildPrivateFieldInitLoose(t.thisExpression(), prop, privateNamesMap),
         );
         break;
-      case !isStatic && isPrivate && !loose:
+      case !isStatic && isPrivateField && !loose:
         instanceNodes.push(
           buildPrivateInstanceFieldInitSpec(
             t.thisExpression(),
@@ -305,10 +357,34 @@ export function buildFieldsInitNodes(
           ),
         );
         break;
-      case !isStatic && !isPrivate && loose:
+      case !isStatic && isPrivateMethod && loose:
+        instanceNodes.push(
+          buildPrivateMethodInitLoose(
+            t.thisExpression(),
+            prop,
+            privateNamesMap,
+          ),
+        );
+        staticNodes.push(
+          buildPrivateInstanceMethodDeclaration(prop, privateNamesMap),
+        );
+        break;
+      case !isStatic && isPrivateMethod && !loose:
+        instanceNodes.push(
+          buildPrivateInstanceMethodInitSpec(
+            t.thisExpression(),
+            prop,
+            privateNamesMap,
+          ),
+        );
+        staticNodes.push(
+          buildPrivateInstanceMethodDeclaration(prop, privateNamesMap),
+        );
+        break;
+      case !isStatic && !isPrivateField && loose:
         instanceNodes.push(buildPublicFieldInitLoose(t.thisExpression(), prop));
         break;
-      case !isStatic && !isPrivate && !loose:
+      case !isStatic && !isPrivateField && !loose:
         instanceNodes.push(
           buildPublicFieldInitSpec(t.thisExpression(), prop, state),
         );

diff --git a/packages/babel-plugin-class-features/src/index.js b/packages/babel-plugin-class-features/src/index.js
@@ -75,7 +75,6 @@ export default declare((api, options) => {
         enableFeature(this.file, FEATURES.fields, fields.loose);
       }
       if (privateMethods.enabled) {
-        throw new Error("Private methods are not supported yet");
         enableFeature(this.file, FEATURES.privateMethods);
       }
       if (decorators.enabled) {
@@ -107,7 +106,7 @@ export default declare((api, options) => {
             computedPaths.push(path);
           }
 
-          if (path.isClassPrivateProperty()) {
+          if (path.isPrivate()) {
             const { name } = path.node.key.id;
 
             if (privateNames.has(name)) {
@@ -116,7 +115,7 @@ export default declare((api, options) => {
             privateNames.add(name);
           }
 
-          if (path.isProperty()) {
+          if (path.isProperty() || path.isClassPrivateMethod()) {
             props.push(path);
           } else if (path.isClassMethod({ kind: "constructor" })) {
             constructor = path;
@@ -131,7 +130,6 @@ export default declare((api, options) => {
           nameFunction(path);
           ref = path.scope.generateUidIdentifier("class");
         } else {
-          // path.isClassDeclaration() && path.node.id
           ref = path.node.id;
         }
 

diff --git a/packages/babel-plugin-proposal-private-methods/.npmignore b/packages/babel-plugin-proposal-private-methods/.npmignore
@@ -0,0 +1,3 @@
+src
+test
+*.log