Disallow reinitializing private elements (#13601)

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com> Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
babel · Aug 30, 2021 · fdfe978 · fdfe978
1 parent cb3ebde
commit fdfe978
Show file tree

Hide file tree

Showing 92 changed files with 451 additions and 229 deletions.
diff --git a/packages/babel-helper-create-class-features-plugin/src/fields.ts b/packages/babel-helper-create-class-features-plugin/src/fields.ts
@@ -540,16 +540,31 @@ function buildPrivateInstanceFieldInitSpec(
   ref: t.Expression,
   prop: NodePath<t.ClassPrivateProperty>,
   privateNamesMap: PrivateNamesMap,
+  state,
 ) {
   const { id } = privateNamesMap.get(prop.node.key.id.name);
   const value = prop.node.value || prop.scope.buildUndefinedNode();
 
-  return template.statement.ast`${t.cloneNode(id)}.set(${ref}, {
-    // configurable is always false for private elements
-    // enumerable is always false for private elements
-    writable: true,
-    value: ${value},
-  })`;
+  if (!process.env.BABEL_8_BREAKING) {
+    if (!state.availableHelper("classPrivateFieldInitSpec")) {
+      return template.statement.ast`${t.cloneNode(id)}.set(${ref}, {
+        // configurable is always false for private elements
+        // enumerable is always false for private elements
+        writable: true,
+        value: ${value},
+      })`;
+    }
+  }
+
+  const helper = state.addHelper("classPrivateFieldInitSpec");
+  return template.statement.ast`${helper}(
+    ${t.thisExpression()},
+    ${t.cloneNode(id)},
+    {
+      writable: true,
+      value: ${value}
+    },
+  )`;
 }
 
 function buildPrivateStaticFieldInitSpec(
@@ -632,27 +647,87 @@ function buildPrivateInstanceMethodInitSpec(
   ref: t.Expression,
   prop: NodePath<t.ClassPrivateMethod>,
   privateNamesMap: PrivateNamesMap,
+  state,
 ) {
   const privateName = privateNamesMap.get(prop.node.key.id.name);
-  const { id, getId, setId, initAdded } = privateName;
+  const { getId, setId, initAdded } = privateName;
 
   if (initAdded) return;
 
   const isAccessor = getId || setId;
   if (isAccessor) {
-    privateNamesMap.set(prop.node.key.id.name, {
-      ...privateName,
-      initAdded: true,
-    });
+    return buildPrivateAccessorInitialization(
+      ref,
+      prop,
+      privateNamesMap,
+      state,
+    );
+  }
 
-    return template.statement.ast`
+  return buildPrivateInstanceMethodInitalization(
+    ref,
+    prop,
+    privateNamesMap,
+    state,
+  );
+}
+
+function buildPrivateAccessorInitialization(
+  ref: t.Expression,
+  prop: NodePath<t.ClassPrivateMethod>,
+  privateNamesMap: PrivateNamesMap,
+  state,
+) {
+  const privateName = privateNamesMap.get(prop.node.key.id.name);
+  const { id, getId, setId } = privateName;
+
+  privateNamesMap.set(prop.node.key.id.name, {
+    ...privateName,
+    initAdded: true,
+  });
+
+  if (!process.env.BABEL_8_BREAKING) {
+    if (!state.availableHelper("classPrivateFieldInitSpec")) {
+      return template.statement.ast`
       ${id}.set(${ref}, {
         get: ${getId ? getId.name : prop.scope.buildUndefinedNode()},
         set: ${setId ? setId.name : prop.scope.buildUndefinedNode()}
       });
     `;
+    }
+  }
+
+  const helper = state.addHelper("classPrivateFieldInitSpec");
+  return template.statement.ast`${helper}(
+    ${t.thisExpression()},
+    ${t.cloneNode(id)},
+    {
+      get: ${getId ? getId.name : prop.scope.buildUndefinedNode()},
+      set: ${setId ? setId.name : prop.scope.buildUndefinedNode()}
+    },
+  )`;
+}
+
+function buildPrivateInstanceMethodInitalization(
+  ref: t.Expression,
+  prop: NodePath<t.ClassPrivateMethod>,
+  privateNamesMap: PrivateNamesMap,
+  state,
+) {
+  const privateName = privateNamesMap.get(prop.node.key.id.name);
+  const { id } = privateName;
+
+  if (!process.env.BABEL_8_BREAKING) {
+    if (!state.availableHelper("classPrivateMethodInitSpec")) {
+      return template.statement.ast`${id}.add(${ref})`;
+    }
   }
-  return template.statement.ast`${id}.add(${ref})`;
+
+  const helper = state.addHelper("classPrivateMethodInitSpec");
+  return template.statement.ast`${helper}(
+    ${t.thisExpression()},
+    ${t.cloneNode(id)}
+  )`;
 }
 
 function buildPublicFieldInitLoose(
@@ -957,6 +1032,7 @@ export function buildFieldsInitNodes(
             // @ts-expect-error checked in switch
             prop,
             privateNamesMap,
+            state,
           ),
         );
         break;
@@ -985,6 +1061,7 @@ export function buildFieldsInitNodes(
             // @ts-expect-error checked in switch
             prop,
             privateNamesMap,
+            state,
           ),
         );
         pureStaticNodes.push(

diff --git a/...eate-class-features-plugin/test/fixtures/field-reinitialization/private-accessor/input.js b/...eate-class-features-plugin/test/fixtures/field-reinitialization/private-accessor/input.js
@@ -0,0 +1,19 @@
+class Base {
+  constructor(obj) {
+    return obj;
+  }
+}
+
+class Derived extends Base {
+  get #foo() {
+    return 'bar';
+  }
+
+  set #foo(value) {
+    this.#foo = value;
+  }
+
+  static get(obj) {
+    return obj.#foo();
+  }
+}
diff --git a/...-class-features-plugin/test/fixtures/field-reinitialization/private-accessor/options.json b/...-class-features-plugin/test/fixtures/field-reinitialization/private-accessor/options.json
@@ -0,0 +1,6 @@
+{
+  "presets": [["env", { "shippedProposals": true }]],
+  "targets": {
+    "chrome": "75"
+  }
+}
diff --git a/...ate-class-features-plugin/test/fixtures/field-reinitialization/private-accessor/output.js b/...ate-class-features-plugin/test/fixtures/field-reinitialization/private-accessor/output.js
@@ -0,0 +1,31 @@
+class Base {
+  constructor(obj) {
+    return obj;
+  }
+
+}
+
+var _foo = /*#__PURE__*/new WeakMap();
+
+class Derived extends Base {
+  constructor(...args) {
+    super(...args);
+    babelHelpers.classPrivateFieldInitSpec(this, _foo, {
+      get: _get_foo,
+      set: _set_foo
+    });
+  }
+
+  static get(obj) {
+    return babelHelpers.classPrivateFieldGet(obj, _foo).call(obj);
+  }
+
+}
+
+function _get_foo() {
+  return 'bar';
+}
+
+function _set_foo(value) {
+  babelHelpers.classPrivateFieldSet(this, _foo, value);
+}
diff --git a/...-create-class-features-plugin/test/fixtures/field-reinitialization/private-field/input.js b/...-create-class-features-plugin/test/fixtures/field-reinitialization/private-field/input.js
@@ -0,0 +1,13 @@
+class Base {
+  constructor(obj) {
+    return obj;
+  }
+}
+
+let counter = 0;
+class Derived extends Base {
+  #foo = ++counter;
+  static get(obj) {
+    return obj.#foo;
+  }
+}
diff --git a/...ate-class-features-plugin/test/fixtures/field-reinitialization/private-field/options.json b/...ate-class-features-plugin/test/fixtures/field-reinitialization/private-field/options.json
@@ -0,0 +1,6 @@
+{
+  "presets": [["env", { "shippedProposals": true }]],
+  "targets": {
+    "chrome": "75"
+  }
+}
diff --git a/...create-class-features-plugin/test/fixtures/field-reinitialization/private-field/output.js b/...create-class-features-plugin/test/fixtures/field-reinitialization/private-field/output.js
@@ -0,0 +1,25 @@
+class Base {
+  constructor(obj) {
+    return obj;
+  }
+
+}
+
+let counter = 0;
+
+var _foo = /*#__PURE__*/new WeakMap();
+
+class Derived extends Base {
+  constructor(...args) {
+    super(...args);
+    babelHelpers.classPrivateFieldInitSpec(this, _foo, {
+      writable: true,
+      value: ++counter
+    });
+  }
+
+  static get(obj) {
+    return babelHelpers.classPrivateFieldGet(obj, _foo);
+  }
+
+}
diff --git a/...create-class-features-plugin/test/fixtures/field-reinitialization/private-method/input.js b/...create-class-features-plugin/test/fixtures/field-reinitialization/private-method/input.js
@@ -0,0 +1,15 @@
+class Base {
+  constructor(obj) {
+    return obj;
+  }
+}
+
+class Derived extends Base {
+  #foo() {
+    return 'bar';
+  }
+
+  static get(obj) {
+    return obj.#foo();
+  }
+}
diff --git a/...te-class-features-plugin/test/fixtures/field-reinitialization/private-method/options.json b/...te-class-features-plugin/test/fixtures/field-reinitialization/private-method/options.json
@@ -0,0 +1,6 @@
+{
+  "presets": [["env", { "shippedProposals": true }]],
+  "targets": {
+    "chrome": "75"
+  }
+}
diff --git a/...reate-class-features-plugin/test/fixtures/field-reinitialization/private-method/output.js b/...reate-class-features-plugin/test/fixtures/field-reinitialization/private-method/output.js
@@ -0,0 +1,24 @@
+class Base {
+  constructor(obj) {
+    return obj;
+  }
+
+}
+
+var _foo = /*#__PURE__*/new WeakSet();
+
+class Derived extends Base {
+  constructor(...args) {
+    super(...args);
+    babelHelpers.classPrivateMethodInitSpec(this, _foo);
+  }
+
+  static get(obj) {
+    return babelHelpers.classPrivateMethodGet(obj, _foo, _foo2).call(obj);
+  }
+
+}
+
+function _foo2() {
+  return 'bar';
+}
diff --git a/...class-features-plugin/test/fixtures/plugin-proposal-private-methods/loose-false/output.js b/...class-features-plugin/test/fixtures/plugin-proposal-private-methods/loose-false/output.js
@@ -2,7 +2,7 @@ var _privateMethod = /*#__PURE__*/new WeakSet();
 
 class X {
   constructor() {
-    _privateMethod.add(this);
+    babelHelpers.classPrivateMethodInitSpec(this, _privateMethod);
   }
 
 }

diff --git a/...s/babel-helper-create-class-features-plugin/test/fixtures/replace-supers/method/output.js b/...s/babel-helper-create-class-features-plugin/test/fixtures/replace-supers/method/output.js
@@ -3,8 +3,7 @@ var _foo = /*#__PURE__*/new WeakSet();
 class A extends B {
   constructor(...args) {
     super(...args);
-
-    _foo.add(this);
+    babelHelpers.classPrivateMethodInitSpec(this, _foo);
   }
 
 }

diff --git a/packages/babel-helpers/src/helpers.ts b/packages/babel-helpers/src/helpers.ts
@@ -2022,6 +2022,32 @@ helpers.classPrivateMethodGet = helper("7.1.6")`
   }
 `;
 
+helpers.checkPrivateRedeclaration = helper("7.14.1")`
+  export default function _checkPrivateRedeclaration(obj, privateCollection) {
+    if (privateCollection.has(obj)) {
+      throw new TypeError("Cannot initialize the same private elements twice on an object");
+    }
+  }
+`;
+
+helpers.classPrivateFieldInitSpec = helper("7.14.1")`
+  import checkPrivateRedeclaration from "checkPrivateRedeclaration";
+
+  export default function _classPrivateFieldInitSpec(obj, privateMap, value) {
+    checkPrivateRedeclaration(obj, privateMap);
+    privateMap.set(obj, value);
+  }
+`;
+
+helpers.classPrivateMethodInitSpec = helper("7.14.1")`
+  import checkPrivateRedeclaration from "checkPrivateRedeclaration";
+
+  export default function _classPrivateMethodInitSpec(obj, privateSet) {
+    checkPrivateRedeclaration(obj, privateSet);
+    privateSet.add(obj);
+  }
+`;
+
 if (!process.env.BABEL_8_BREAKING) {
   // Use readOnlyError instead
   helpers.classPrivateMethodSet = helper("7.1.6")`

diff --git a/...ional-chaining/test/fixtures/basic/class-private-integration-optional-chaining/output.mjs b/...ional-chaining/test/fixtures/basic/class-private-integration-optional-chaining/output.mjs
@@ -4,11 +4,10 @@ class C {
   constructor() {
     var _babelHelpers$classPr;
 
-    _m.set(this, {
+    babelHelpers.classPrivateFieldInitSpec(this, _m, {
       writable: true,
       value: void 0
     });
-
     const o = null;
     const n = this;
     const p = o === null || o === void 0 ? void 0 : babelHelpers.classPrivateFieldGet(o, _m).call(o, ...c, 1);

diff --git a/...-parameters-in-optional-chaining/test/fixtures/basic/class-private-integration/output.mjs b/...-parameters-in-optional-chaining/test/fixtures/basic/class-private-integration/output.mjs
@@ -4,11 +4,10 @@ class C {
   constructor() {
     var _babelHelpers$classPr;
 
-    _m.set(this, {
+    babelHelpers.classPrivateFieldInitSpec(this, _m, {
       writable: true,
       value: void 0
     });
-
     const o = null;
     const n = this;
     const p = o === null || o === void 0 ? void 0 : babelHelpers.classPrivateFieldGet(o, _m).call(o, ...c, 1);

diff --git a/...l-async-generator-functions/test/fixtures/async-generators/class-private-method/output.js b/...l-async-generator-functions/test/fixtures/async-generators/class-private-method/output.js
@@ -2,7 +2,7 @@ var _g = /*#__PURE__*/new WeakSet();
 
 class C {
   constructor() {
-    _g.add(this);
+    babelHelpers.classPrivateMethodInitSpec(this, _g);
   }
 
 }