Class Private Static Accessors (#10217)

* Remove error for static private accessors * Private static accessors strict * Add loose mode support * Move `value` decl for early return * Reuse getter/setter template * Reuse getter/setter templates * Use `buildUndefinedNode` in accessor templates * Extract `isAccessor` variable
babel · Sep 6, 2019 · 3e4889d · 3e4889d
1 parent da0af5f
commit 3e4889d
Show file tree

Hide file tree

Showing 35 changed files with 749 additions and 90 deletions.
diff --git a/packages/babel-helper-create-class-features-plugin/src/features.js b/packages/babel-helper-create-class-features-plugin/src/features.js
@@ -66,12 +66,6 @@ export function verifyUsedFeatures(path, file) {
     if (!hasFeature(file, FEATURES.privateMethods)) {
       throw path.buildCodeFrameError("Class private methods are not enabled.");
     }
-
-    if (path.node.static && path.node.kind !== "method") {
-      throw path.buildCodeFrameError(
-        "@babel/plugin-class-features doesn't support class static private accessors yet.",
-      );
-    }
   }
 
   if (

diff --git a/packages/babel-helper-create-class-features-plugin/src/fields.js b/packages/babel-helper-create-class-features-plugin/src/fields.js
@@ -44,14 +44,15 @@ export function buildPrivateNamesNodes(privateNamesMap, loose, state) {
     // because static fields are directly assigned to a variable in the
     // buildPrivateStaticFieldInitSpec function.
     const { id, static: isStatic, method: isMethod, getId, setId } = value;
+    const isAccessor = getId || setId;
     if (loose) {
       initNodes.push(
         template.statement.ast`
           var ${id} = ${state.addHelper("classPrivateFieldLooseKey")}("${name}")
         `,
       );
     } else if (isMethod && !isStatic) {
-      if (getId || setId) {
+      if (isAccessor) {
         initNodes.push(template.statement.ast`var ${id} = new WeakMap();`);
       } else {
         initNodes.push(template.statement.ast`var ${id} = new WeakSet();`);
@@ -140,11 +141,13 @@ const privateNameHandlerSpec = {
       getId,
       setId,
     } = privateNamesMap.get(name);
+    const isAccessor = getId || setId;
 
     if (isStatic) {
-      const helperName = isMethod
-        ? "classStaticPrivateMethodGet"
-        : "classStaticPrivateFieldSpecGet";
+      const helperName =
+        isMethod && !isAccessor
+          ? "classStaticPrivateMethodGet"
+          : "classStaticPrivateFieldSpecGet";
 
       return t.callExpression(file.addHelper(helperName), [
         this.receiver(member),
@@ -154,7 +157,7 @@ const privateNameHandlerSpec = {
     }
 
     if (isMethod) {
-      if (getId || setId) {
+      if (isAccessor) {
         return t.callExpression(file.addHelper("classPrivateFieldGet"), [
           this.receiver(member),
           t.cloneNode(id),
@@ -180,12 +183,15 @@ const privateNameHandlerSpec = {
       static: isStatic,
       method: isMethod,
       setId,
+      getId,
     } = privateNamesMap.get(name);
+    const isAccessor = getId || setId;
 
     if (isStatic) {
-      const helperName = isMethod
-        ? "classStaticPrivateMethodSet"
-        : "classStaticPrivateFieldSpecSet";
+      const helperName =
+        isMethod && !isAccessor
+          ? "classStaticPrivateMethodSet"
+          : "classStaticPrivateFieldSpecSet";
 
       return t.callExpression(file.addHelper(helperName), [
         this.receiver(member),
@@ -300,9 +306,30 @@ function buildPrivateInstanceFieldInitSpec(ref, prop, privateNamesMap) {
 }
 
 function buildPrivateStaticFieldInitSpec(prop, privateNamesMap) {
-  const { id } = privateNamesMap.get(prop.node.key.id.name);
-  const value = prop.node.value || prop.scope.buildUndefinedNode();
+  const privateName = privateNamesMap.get(prop.node.key.id.name);
+  const { id, getId, setId, initAdded } = privateName;
+  const isAccessor = getId || setId;
 
+  if (!prop.isProperty() && (initAdded || !isAccessor)) return;
+
+  if (isAccessor) {
+    privateNamesMap.set(prop.node.key.id.name, {
+      ...privateName,
+      initAdded: true,
+    });
+
+    return template.statement.ast`
+      var ${id.name} = {
+        // configurable is false by default
+        // enumerable is false by default
+        // writable is false by default
+        get: ${getId ? getId.name : prop.scope.buildUndefinedNode()},
+        set: ${setId ? setId.name : prop.scope.buildUndefinedNode()}
+      }
+    `;
+  }
+
+  const value = prop.node.value || prop.scope.buildUndefinedNode();
   return template.statement.ast`
     var ${id} = {
       // configurable is false by default
@@ -328,76 +355,44 @@ function buildPrivateMethodInitLoose(ref, prop, privateNamesMap) {
         });
       `;
   }
-
-  if (getId || setId) {
+  const isAccessor = getId || setId;
+  if (isAccessor) {
     privateNamesMap.set(prop.node.key.id.name, {
       ...privateName,
       initAdded: true,
     });
 
-    if (getId && setId) {
-      return template.statement.ast`
-        Object.defineProperty(${ref}, ${id}, {
-          // configurable is false by default
-          // enumerable is false by default
-          // writable is false by default
-          get: ${getId.name},
-          set: ${setId.name}
-        });
-      `;
-    } else if (getId && !setId) {
-      return template.statement.ast`
-        Object.defineProperty(${ref}, ${id}, {
-          // configurable is false by default
-          // enumerable is false by default
-          // writable is false by default
-          get: ${getId.name}
-        });
-      `;
-    } else if (!getId && setId) {
-      return template.statement.ast`
-        Object.defineProperty(${ref}, ${id}, {
-          // configurable is false by default
-          // enumerable is false by default
-          // writable is false by default
-          set: ${setId.name}
-        });
-      `;
-    }
+    return template.statement.ast`
+      Object.defineProperty(${ref}, ${id}, {
+        // configurable is false by default
+        // enumerable is false by default
+        // writable is false by default
+        get: ${getId ? getId.name : prop.scope.buildUndefinedNode()},
+        set: ${setId ? setId.name : prop.scope.buildUndefinedNode()}
+      });
+    `;
   }
 }
 
 function buildPrivateInstanceMethodInitSpec(ref, prop, privateNamesMap) {
   const privateName = privateNamesMap.get(prop.node.key.id.name);
   const { id, getId, setId, initAdded } = privateName;
+
   if (initAdded) return;
 
-  if (getId || setId) {
+  const isAccessor = getId || setId;
+  if (isAccessor) {
     privateNamesMap.set(prop.node.key.id.name, {
       ...privateName,
       initAdded: true,
     });
 
-    if (getId && setId) {
-      return template.statement.ast`
-        ${id}.set(${ref}, {
-          get: ${getId.name},
-          set: ${setId.name}
-        });
-      `;
-    } else if (getId && !setId) {
-      return template.statement.ast`
-        ${id}.set(${ref}, {
-          get: ${getId.name}
-        });
-      `;
-    } else if (!getId && setId) {
-      return template.statement.ast`
-        ${id}.set(${ref}, {
-          set: ${setId.name}
-        });
-      `;
-    }
+    return template.statement.ast`
+      ${id}.set(${ref}, {
+        get: ${getId ? getId.name : prop.scope.buildUndefinedNode()},
+        set: ${setId ? setId.name : prop.scope.buildUndefinedNode()}
+      });
+    `;
   }
   return template.statement.ast`${id}.add(${ref})`;
 }
@@ -429,7 +424,29 @@ function buildPublicFieldInitSpec(ref, prop, state) {
 }
 
 function buildPrivateStaticMethodInitLoose(ref, prop, state, privateNamesMap) {
-  const { id, methodId } = privateNamesMap.get(prop.node.key.id.name);
+  const privateName = privateNamesMap.get(prop.node.key.id.name);
+  const { id, methodId, getId, setId, initAdded } = privateName;
+
+  if (initAdded) return;
+
+  const isAccessor = getId || setId;
+  if (isAccessor) {
+    privateNamesMap.set(prop.node.key.id.name, {
+      ...privateName,
+      initAdded: true,
+    });
+
+    return template.statement.ast`
+      Object.defineProperty(${ref}, ${id}, {
+        // configurable is false by default
+        // enumerable is false by default
+        // writable is false by default
+        get: ${getId ? getId.name : prop.scope.buildUndefinedNode()},
+        set: ${setId ? setId.name : prop.scope.buildUndefinedNode()}
+      })
+    `;
+  }
+
   return template.statement.ast`
     Object.defineProperty(${ref}, ${id}, {
       // configurable is false by default
@@ -615,14 +632,14 @@ export function buildFieldsInitNodes(
       case isStatic && isPrivate && isMethod && !loose:
         needsClassRef = true;
         staticNodes.push(
+          buildPrivateStaticFieldInitSpec(prop, privateNamesMap),
+        );
+        staticNodes.unshift(
           buildPrivateMethodDeclaration(prop, privateNamesMap, loose),
         );
         break;
       case isStatic && isPrivate && isMethod && loose:
         needsClassRef = true;
-        staticNodes.push(
-          buildPrivateMethodDeclaration(prop, privateNamesMap, loose),
-        );
         staticNodes.push(
           buildPrivateStaticMethodInitLoose(
             t.cloneNode(ref),
@@ -631,6 +648,9 @@ export function buildFieldsInitNodes(
             privateNamesMap,
           ),
         );
+        staticNodes.unshift(
+          buildPrivateMethodDeclaration(prop, privateNamesMap, loose),
+        );
         break;
       case isInstance && isPublic && isField && loose:
         instanceNodes.push(buildPublicFieldInitLoose(t.thisExpression(), prop));
@@ -646,7 +666,7 @@ export function buildFieldsInitNodes(
   }
 
   return {
-    staticNodes,
+    staticNodes: staticNodes.filter(Boolean),
     instanceNodes: instanceNodes.filter(Boolean),
     wrapClass(path) {
       for (const prop of props) {

diff --git a/packages/babel-helpers/src/helpers.js b/packages/babel-helpers/src/helpers.js
@@ -1177,6 +1177,9 @@ helpers.classStaticPrivateFieldSpecGet = helper("7.0.2")`
     if (receiver !== classConstructor) {
       throw new TypeError("Private static access of wrong provenance");
     }
+    if (descriptor.get) {
+      return descriptor.get.call(receiver);
+    }
     return descriptor.value;
   }
 `;
@@ -1186,13 +1189,18 @@ helpers.classStaticPrivateFieldSpecSet = helper("7.0.2")`
     if (receiver !== classConstructor) {
       throw new TypeError("Private static access of wrong provenance");
     }
-    if (!descriptor.writable) {
-      // This should only throw in strict mode, but class bodies are
-      // always strict and private fields can only be used inside
-      // class bodies.
-      throw new TypeError("attempted to set read only private field");
+    if (descriptor.set) {
+      descriptor.set.call(receiver, value);
+    } else {
+      if (!descriptor.writable) {
+        // This should only throw in strict mode, but class bodies are
+        // always strict and private fields can only be used inside
+        // class bodies.
+        throw new TypeError("attempted to set read only private field");
+      }
+      descriptor.value = value;
     }
-    descriptor.value = value;
+
     return value;
   }
 `;

diff --git a/...l-plugin-proposal-private-methods/test/fixtures/accessors-loose/get-only-setter/output.js b/...l-plugin-proposal-private-methods/test/fixtures/accessors-loose/get-only-setter/output.js
@@ -1,6 +1,7 @@
 class Cl {
   constructor() {
     Object.defineProperty(this, _privateFieldValue, {
+      get: void 0,
       set: _set_privateFieldValue
     });
     Object.defineProperty(this, _privateField, {

diff --git a/...l-plugin-proposal-private-methods/test/fixtures/accessors-loose/set-only-getter/output.js b/...l-plugin-proposal-private-methods/test/fixtures/accessors-loose/set-only-getter/output.js
@@ -1,7 +1,8 @@
 class Cl {
   constructor() {
     Object.defineProperty(this, _privateFieldValue, {
-      get: _get_privateFieldValue
+      get: _get_privateFieldValue,
+      set: void 0
     });
     Object.defineProperty(this, _privateField, {
       writable: true,

diff --git a/...s/babel-plugin-proposal-private-methods/test/fixtures/accessors/get-only-setter/output.js b/...s/babel-plugin-proposal-private-methods/test/fixtures/accessors/get-only-setter/output.js
@@ -1,6 +1,7 @@
 class Cl {
   constructor() {
     _privateFieldValue.set(this, {
+      get: void 0,
       set: _set_privateFieldValue
     });
 

diff --git a/...s/babel-plugin-proposal-private-methods/test/fixtures/accessors/set-only-getter/output.js b/...s/babel-plugin-proposal-private-methods/test/fixtures/accessors/set-only-getter/output.js
@@ -1,7 +1,8 @@
 class Cl {
   constructor() {
     _privateFieldValue.set(this, {
-      get: _get_privateFieldValue
+      get: _get_privateFieldValue,
+      set: void 0
     });
 
     _privateField.set(this, {

diff --git a/...-plugin-proposal-private-methods/test/fixtures/private-static-method-loose/this/output.js b/...-plugin-proposal-private-methods/test/fixtures/private-static-method-loose/this/output.js
@@ -20,18 +20,17 @@ var _getA = babelHelpers.classPrivateFieldLooseKey("getA");
 
 var _getB = babelHelpers.classPrivateFieldLooseKey("getB");
 
+var _getB2 = function _getB2() {
+  return this.b;
+};
+
 var _getA2 = function _getA2() {
   return A.a;
 };
 
 Object.defineProperty(B, _getA, {
   value: _getA2
 });
-
-var _getB2 = function _getB2() {
-  return this.b;
-};
-
 Object.defineProperty(B, _getB, {
   value: _getB2
 });

diff --git a/.../babel-plugin-proposal-private-methods/test/fixtures/private-static-method/this/output.js b/.../babel-plugin-proposal-private-methods/test/fixtures/private-static-method/this/output.js
@@ -16,12 +16,12 @@ class B extends A {
 
 }
 
-var _getA = function _getA() {
-  return babelHelpers.get(babelHelpers.getPrototypeOf(B), "a", this);
-};
-
 var _getB = function _getB() {
   return this.b;
 };
 
+var _getA = function _getA() {
+  return babelHelpers.get(babelHelpers.getPrototypeOf(B), "a", this);
+};
+
 var [getA, getB] = B.extract();