Review feedback

babel · May 10, 2023 · 71bbfa6 · 71bbfa6
1 parent 5f89c24
commit 71bbfa6
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 17 deletions.
diff --git a/babel.config.js b/babel.config.js
@@ -236,6 +236,14 @@ module.exports = function (api) {
             "flag-IS_STANDALONE",
           ],
 
+          [
+            pluginToggleBooleanFlag,
+            {
+              name: "process.env.IS_PUBLISH",
+              value: bool(process.env.IS_PUBLISH),
+            },
+          ],
+
           process.env.STRIP_BABEL_8_FLAG && [
             pluginToggleBooleanFlag,
             {

diff --git a/packages/babel-generator/src/generators/modules.ts b/packages/babel-generator/src/generators/modules.ts
@@ -66,12 +66,23 @@ export function ExportNamespaceSpecifier(
   this.print(node.exported, node);
 }
 
+let warningShown = false;
+
 export function _printAttributes(
   this: Printer,
   node: Extract<t.Node, { attributes?: t.ImportAttribute[] }>,
 ) {
-  if (node.attributes && !this.format.importAttributesKeyword) {
-    this.format.importAttributesKeyword = "auto-legacy";
+  const { importAttributesKeyword } = this.format;
+  const { attributes, assertions } = node;
+
+  if (
+    attributes &&
+    !importAttributesKeyword &&
+    // In the production build only show the warning once.
+    // We want to show it per-usage locally for tests.
+    (!process.env.IS_PUBLISH || !warningShown)
+  ) {
+    warningShown = true;
     console.warn(`\
 You are using import attributes, without specifying the desired output syntax.
 Please specify the "importAttributesKeyword" generator option, whose value can be one of:
@@ -80,26 +91,23 @@ Please specify the "importAttributesKeyword" generator option, whose value can b
  - "with-legacy" : \`import { a } from "b" with type: "json";\`
 `);
   }
-  this.word(
-    !this.format.importAttributesKeyword ||
-      this.format.importAttributesKeyword === "assert" ||
-      (this.format.importAttributesKeyword === "auto-legacy" && node.assertions)
-      ? "assert"
-      : "with",
-  );
+
+  const useAssertKeyword =
+    importAttributesKeyword === "assert" ||
+    (!importAttributesKeyword && assertions);
+
+  this.word(useAssertKeyword ? "assert" : "with");
   this.space();
 
-  if (
-    this.format.importAttributesKeyword === "with-legacy" ||
-    (this.format.importAttributesKeyword === "auto-legacy" && !node.assertions)
-  ) {
-    this.printList(node.attributes || node.assertions, node);
+  if (!useAssertKeyword && importAttributesKeyword !== "with") {
+    // with-legacy
+    this.printList(attributes || assertions, node);
     return;
   }
 
   this.token("{");
   this.space();
-  this.printList(node.attributes || node.assertions, node);
+  this.printList(attributes || assertions, node);
   this.space();
   this.token("}");
 }

diff --git a/packages/babel-generator/src/printer.ts b/packages/babel-generator/src/printer.ts
@@ -18,7 +18,7 @@ import type { Opts as jsescOptions } from "jsesc";
 import * as generatorFunctions from "./generators";
 import type SourceMap from "./source-map";
 import * as charCodes from "charcodes";
-import { type TraceMap } from "@jridgewell/trace-mapping";
+import type { TraceMap } from "@jridgewell/trace-mapping";
 
 const SCIENTIFIC_NOTATION = /e/i;
 const ZERO_DECIMAL_INTEGER = /\.0+$/;
@@ -85,7 +85,7 @@ export type Format = {
    * - "auto-legacy" : "assert" if the AST node has an .assertions property,
    *                   "with-legacy" if it has an .attributes property.
    */
-  importAttributesKeyword?: "with" | "assert" | "with-legacy" | "auto-legacy";
+  importAttributesKeyword?: "with" | "assert" | "with-legacy";
 };
 
 interface AddNewlinesOptions {