Class features loose should have precedence over preset-env (#11634)

* Class features loose should have precedence over preset-env

* Comment

* Update packages/babel-helper-create-class-features-plugin/src/features.js

[skip ci]

Co-authored-by: Huáng Jùnliàng <jlhwung@gmail.com>

* Future proof

* Add warning when loose mode changes automatically

* Better message

Co-authored-by: Huáng Jùnliàng <jlhwung@gmail.com>

packages/babel-helper-create-class-features-plugin/src/features.js

@@ -8,6 +8,15 @@ export const FEATURES = Object.freeze({
   privateIn: 1 << 4,
 });
 
+const featuresSameLoose = new Map([
+  [FEATURES.fields, "@babel/plugin-proposal-class-properties"],
+  [FEATURES.privateMethods, "@babel/plugin-proposal-private-methods"],
+  [
+    FEATURES.privateIn,
+    "@babel/plugin-proposal-private-private-property-in-object",
+  ],
+]);
+
 // We can't use a symbol because this needs to always be the same, even if
 // this package isn't deduped by npm. e.g.
 //  - node_modules/
@@ -18,49 +27,81 @@ export const FEATURES = Object.freeze({
 const featuresKey = "@babel/plugin-class-features/featuresKey";
 const looseKey = "@babel/plugin-class-features/looseKey";
 
+// See https://github.com/babel/babel/issues/11622.
+// Since preset-env sets loose for the fields and private methods plugins, it can
+// cause conflicts with the loose mode set by an explicit plugin in the config.
+// To solve this problem, we ignore preset-env's loose mode if another plugin
+// explicitly sets it
+// The code to handle this logic doesn't check that "low priority loose" is always
+// the same. However, it is only set by the preset and not directly by users:
+// unless someone _wants_ to break it, it shouldn't be a problem.
+const looseLowPriorityKey =
+  "@babel/plugin-class-features/looseLowPriorityKey/#__internal__@babel/preset-env__please-overwrite-loose-instead-of-throwing";
+
 export function enableFeature(file, feature, loose) {
   // We can't blindly enable the feature because, if it was already set,
   // "loose" can't be changed, so that
   //   @babel/plugin-class-properties { loose: true }
   //   @babel/plugin-class-properties { loose: false }
   // is transformed in loose mode.
   // We only enabled the feature if it was previously disabled.
-  if (!hasFeature(file, feature)) {
+  if (!hasFeature(file, feature) || canIgnoreLoose(file, feature)) {
     file.set(featuresKey, file.get(featuresKey) | feature);
-    if (loose) file.set(looseKey, file.get(looseKey) | feature);
+    if (
+      loose ===
+      "#__internal__@babel/preset-env__prefer-true-but-false-is-ok-if-it-prevents-an-error"
+    ) {
+      setLoose(file, feature, true);
+      file.set(looseLowPriorityKey, file.get(looseLowPriorityKey) | feature);
+    } else if (
+      loose ===
+      "#__internal__@babel/preset-env__prefer-false-but-true-is-ok-if-it-prevents-an-error"
+    ) {
+      setLoose(file, feature, false);
+      file.set(looseLowPriorityKey, file.get(looseLowPriorityKey) | feature);
+    } else {
+      setLoose(file, feature, loose);
+    }
   }
 
-  if (
-    hasFeature(file, FEATURES.fields) &&
-    hasFeature(file, FEATURES.privateMethods) &&
-    isLoose(file, FEATURES.fields) !== isLoose(file, FEATURES.privateMethods)
-  ) {
-    throw new Error(
-      "'loose' mode configuration must be the same for both @babel/plugin-proposal-class-properties " +
-        "and @babel/plugin-proposal-private-methods",
-    );
-  }
+  let resolvedLoose: void | true | false;
+  let higherPriorityPluginName: void | string;
 
-  if (
-    hasFeature(file, FEATURES.fields) &&
-    hasFeature(file, FEATURES.privateIn) &&
-    isLoose(file, FEATURES.fields) !== isLoose(file, FEATURES.privateIn)
-  ) {
-    throw new Error(
-      "'loose' mode configuration must be the same for both @babel/plugin-proposal-class-properties " +
-        "and @babel/plugin-proposal-private-property-in-object",
-    );
+  for (const [mask, name] of featuresSameLoose) {
+    if (!hasFeature(file, mask)) continue;
+
+    const loose = isLoose(file, mask);
+
+    if (canIgnoreLoose(file, mask)) {
+      continue;
+    } else if (resolvedLoose === !loose) {
+      throw new Error(
+        "'loose' mode configuration must be the same for @babel/plugin-proposal-class-properties, " +
+          "@babel/plugin-proposal-private-methods and " +
+          "@babel/plugin-proposal-private-property-in-object (when they are enabled).",
+      );
+    } else {
+      resolvedLoose = loose;
+      higherPriorityPluginName = name;
+    }
   }
 
-  if (
-    hasFeature(file, FEATURES.privateMethods) &&
-    hasFeature(file, FEATURES.privateIn) &&
-    isLoose(file, FEATURES.privateMethods) !== isLoose(file, FEATURES.privateIn)
-  ) {
-    throw new Error(
-      "'loose' mode configuration must be the same for both @babel/plugin-proposal-private-methods " +
-        "and @babel/plugin-proposal-private-property-in-object",
-    );
+  if (resolvedLoose !== undefined) {
+    for (const [mask, name] of featuresSameLoose) {
+      if (hasFeature(file, mask) && isLoose(file, mask) !== resolvedLoose) {
+        setLoose(file, mask, resolvedLoose);
+        console.warn(
+          `Though the "loose" option was set to "${!resolvedLoose}" in your @babel/preset-env ` +
+            `config, it will not be used for ${name} since the "loose" mode option was set to ` +
+            `"${resolvedLoose}" for ${higherPriorityPluginName}.\nThe "loose" option must be the ` +
+            `same for @babel/plugin-proposal-class-properties, @babel/plugin-proposal-private-methods ` +
+            `and @babel/plugin-proposal-private-property-in-object (when they are enabled): you can ` +
+            `silence this warning by explicitly adding\n` +
+            `\t["${name}", { "loose": ${resolvedLoose} }]\n` +
+            `to the "plugins" section of your Babel config.`,
+        );
+      }
+    }
   }
 }
 
@@ -72,6 +113,17 @@ export function isLoose(file, feature) {
   return !!(file.get(looseKey) & feature);
 }
 
+function setLoose(file, feature, loose) {
+  if (loose) file.set(looseKey, file.get(looseKey) | feature);
+  else file.set(looseKey, file.get(looseKey) & ~feature);
+
+  file.set(looseLowPriorityKey, file.get(looseLowPriorityKey) & ~feature);
+}
+
+function canIgnoreLoose(file, feature) {
+  return !!(file.get(looseLowPriorityKey) & feature);
+}
+
 export function verifyUsedFeatures(path, file) {
   if (hasOwnDecorators(path.node)) {
     if (!hasFeature(file, FEATURES.decorators)) {

packages/babel-preset-env/src/index.js

@@ -301,10 +301,28 @@ export default declare((api, opts) => {
 
   const pluginUseBuiltIns = useBuiltIns !== false;
   const plugins = Array.from(pluginNames)
-    .map(pluginName => [
-      getPlugin(pluginName),
-      { spec, loose, useBuiltIns: pluginUseBuiltIns },
-    ])
+    .map(pluginName => {
+      if (
+        pluginName === "proposal-class-properties" ||
+        pluginName === "proposal-private-methods" ||
+        // This is not included in preset-env yet, but let's keep it here so we
+        // don't forget about it in the future.
+        pluginName === "proposal-private-property-in-object"
+      ) {
+        return [
+          getPlugin(pluginName),
+          {
+            loose: loose
+              ? "#__internal__@babel/preset-env__prefer-true-but-false-is-ok-if-it-prevents-an-error"
+              : "#__internal__@babel/preset-env__prefer-false-but-true-is-ok-if-it-prevents-an-error",
+          },
+        ];
+      }
+      return [
+        getPlugin(pluginName),
+        { spec, loose, useBuiltIns: pluginUseBuiltIns },
+      ];
+    })
     .concat(polyfillPlugins);
 
   if (debug) {

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/methods-loose-preset-not-loose/input.js

@@ -0,0 +1,5 @@
+class A {
+  x = 2;
+
+  #foo() {}
+}

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/methods-loose-preset-not-loose/options.json

@@ -0,0 +1,12 @@
+{
+  "validateLogs": true,
+  "presets": [
+    ["env", {
+      "targets": { "node": 10 },
+      "shippedProposals": true
+    }]
+  ],
+  "plugins": [
+    ["proposal-private-methods", { "loose": true }]
+  ]
+}

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/methods-loose-preset-not-loose/output.js

@@ -0,0 +1,17 @@
+var id = 0;
+
+function _classPrivateFieldLooseKey(name) { return "__private_" + id++ + "_" + name; }
+
+class A {
+  constructor() {
+    Object.defineProperty(this, _foo, {
+      value: _foo2
+    });
+    this.x = 2;
+  }
+
+}
+
+var _foo = _classPrivateFieldLooseKey("foo");
+
+var _foo2 = function _foo2() {};

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/methods-loose-preset-not-loose/stderr.txt

@@ -0,0 +1,4 @@
+Though the "loose" option was set to "false" in your @babel/preset-env config, it will not be used for @babel/plugin-proposal-class-properties since the "loose" mode option was set to "true" for @babel/plugin-proposal-private-methods.
+The "loose" option must be the same for @babel/plugin-proposal-class-properties, @babel/plugin-proposal-private-methods and @babel/plugin-proposal-private-property-in-object (when they are enabled): you can silence this warning by explicitly adding
+	["@babel/plugin-proposal-class-properties", { "loose": true }]
+to the "plugins" section of your Babel config.

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/preset-loose-no-plugins/input.js

@@ -0,0 +1,5 @@
+class A {
+  x = 2;
+
+  #foo() {}
+}

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/preset-loose-no-plugins/options.json

@@ -0,0 +1,10 @@
+{
+  "validateLogs": true,
+  "presets": [
+    ["env", {
+      "targets": { "node": 10 },
+      "shippedProposals": true,
+      "loose": true
+    }]
+  ]
+}

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/preset-loose-no-plugins/output.js

@@ -0,0 +1,17 @@
+var id = 0;
+
+function _classPrivateFieldLooseKey(name) { return "__private_" + id++ + "_" + name; }
+
+class A {
+  constructor() {
+    Object.defineProperty(this, _foo, {
+      value: _foo2
+    });
+    this.x = 2;
+  }
+
+}
+
+var _foo = _classPrivateFieldLooseKey("foo");
+
+var _foo2 = function _foo2() {};

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/preset-not-loose-no-plugins/input.js

@@ -0,0 +1,5 @@
+class A {
+  x = 2;
+
+  #foo() {}
+}

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/preset-not-loose-no-plugins/options.json

@@ -0,0 +1,9 @@
+{
+  "validateLogs": true,
+  "presets": [
+    ["env", {
+      "targets": { "node": 10 },
+      "shippedProposals": true
+    }]
+  ]
+}

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/preset-not-loose-no-plugins/output.js

@@ -0,0 +1,14 @@
+function _defineProperty(obj, key, value) { if (key in obj) { Object.defineProperty(obj, key, { value: value, enumerable: true, configurable: true, writable: true }); } else { obj[key] = value; } return obj; }
+
+class A {
+  constructor() {
+    _foo.add(this);
+
+    _defineProperty(this, "x", 2);
+  }
+
+}
+
+var _foo = new WeakSet();
+
+var _foo2 = function _foo2() {};

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/properties-and-methods-loose-preset-not-loose/input.js

@@ -0,0 +1,5 @@
+class A {
+  x = 2;
+
+  #foo() {}
+}

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/properties-and-methods-loose-preset-not-loose/options.json

@@ -0,0 +1,13 @@
+{
+  "validateLogs": true,
+  "presets": [
+    ["env", {
+      "targets": { "node": 10 },
+      "shippedProposals": true
+    }]
+  ],
+  "plugins": [
+    ["proposal-class-properties", { "loose": true }],
+    ["proposal-private-methods", { "loose": true }]
+  ]
+}

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/properties-and-methods-loose-preset-not-loose/output.js

@@ -0,0 +1,17 @@
+var id = 0;
+
+function _classPrivateFieldLooseKey(name) { return "__private_" + id++ + "_" + name; }
+
+class A {
+  constructor() {
+    Object.defineProperty(this, _foo, {
+      value: _foo2
+    });
+    this.x = 2;
+  }
+
+}
+
+var _foo = _classPrivateFieldLooseKey("foo");
+
+var _foo2 = function _foo2() {};

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/properties-and-methods-not-loose-preset-loose/input.js

@@ -0,0 +1,5 @@
+class A {
+  x = 2;
+
+  #foo() {}
+}

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/properties-and-methods-not-loose-preset-loose/options.json

@@ -0,0 +1,14 @@
+{
+  "validateLogs": true,
+  "presets": [
+    ["env", {
+      "targets": { "node": 10 },
+      "shippedProposals": true,
+      "loose": true
+    }]
+  ],
+  "plugins": [
+    ["proposal-class-properties", { "loose": false }],
+    ["proposal-private-methods", { "loose": false }]
+  ]
+}

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/properties-and-methods-not-loose-preset-loose/output.js

@@ -0,0 +1,14 @@
+function _defineProperty(obj, key, value) { if (key in obj) { Object.defineProperty(obj, key, { value: value, enumerable: true, configurable: true, writable: true }); } else { obj[key] = value; } return obj; }
+
+class A {
+  constructor() {
+    _foo.add(this);
+
+    _defineProperty(this, "x", 2);
+  }
+
+}
+
+var _foo = new WeakSet();
+
+var _foo2 = function _foo2() {};

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/properties-loose-preset-loose/input.js

@@ -0,0 +1,5 @@
+class A {
+  x = 2;
+
+  #foo() {}
+}

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/properties-loose-preset-loose/options.json

@@ -0,0 +1,13 @@
+{
+  "validateLogs": true,
+  "presets": [
+    ["env", {
+      "targets": { "node": 10 },
+      "shippedProposals": true,
+      "loose": true
+    }]
+  ],
+  "plugins": [
+    ["proposal-class-properties", { "loose": true }]
+  ]
+}

packages/babel-preset-env/test/fixtures/loose-class-features-precedence/properties-loose-preset-loose/output.js

@@ -0,0 +1,17 @@
+var id = 0;
+
+function _classPrivateFieldLooseKey(name) { return "__private_" + id++ + "_" + name; }
+
+class A {
+  constructor() {
+    Object.defineProperty(this, _foo, {
+      value: _foo2
+    });
+    this.x = 2;
+  }
+
+}
+
+var _foo = _classPrivateFieldLooseKey("foo");
+
+var _foo2 = function _foo2() {};