Fix: do not report global variables as injected binding (#14827)

JLHwung · web-flow · commit 9fad3a80959f · 2022-08-04T08:50:41.000-04:00
* test: update regression/10162

* fix: global variables must not be global types

* refactor: let GLOBAL_TYPES be a map&lt;scope, set&gt;

* log warning messages to test fixtures
diff --git a/packages/babel-plugin-transform-typescript/src/index.ts b/packages/babel-plugin-transform-typescript/src/index.ts
@@ -2,7 +2,7 @@ import { declare } from "@babel/helper-plugin-utils";
 import syntaxTypeScript from "@babel/plugin-syntax-typescript";
 import { types as t, template } from "@babel/core";
 import { injectInitialization } from "@babel/helper-create-class-features-plugin";
-import type { Binding, NodePath } from "@babel/traverse";
+import type { Binding, NodePath, Scope } from "@babel/traverse";
 import type { Options as SyntaxOptions } from "@babel/plugin-syntax-typescript";
 
 import transpileConstEnum from "./const-enum";
@@ -27,17 +27,16 @@ function isInType(path: NodePath) {
   }
 }
 
-const GLOBAL_TYPES = new WeakMap();
+const GLOBAL_TYPES = new WeakMap<Scope, Set<string>>();
 // Track programs which contain imports/exports of values, so that we can include
 // empty exports for programs that do not, but were parsed as modules. This allows
 // tools to infer unamibiguously that results are ESM.
 const NEEDS_EXPLICIT_ESM = new WeakMap();
 const PARSED_PARAMS = new WeakSet();
 
-function isGlobalType(path: NodePath, name: string) {
-  const program = path.find(path => path.isProgram()).node;
-  if (path.scope.hasOwnBinding(name)) return false;
-  if (GLOBAL_TYPES.get(program).has(name)) return true;
+function isGlobalType({ scope }: NodePath, name: string) {
+  if (scope.hasBinding(name)) return false;
+  if (GLOBAL_TYPES.get(scope).has(name)) return true;
 
   console.warn(
     `The exported identifier "${name}" is not declared in Babel's scope tracker\n` +
@@ -52,8 +51,8 @@ function isGlobalType(path: NodePath, name: string) {
   return false;
 }
 
-function registerGlobalType(programNode: t.Program, name: string) {
-  GLOBAL_TYPES.get(programNode).add(name);
+function registerGlobalType(programScope: Scope, name: string) {
+  GLOBAL_TYPES.get(programScope).add(name);
 }
 export interface Options extends SyntaxOptions {
   /** @default true */
@@ -213,10 +212,10 @@ export default declare((api, opts: Options) => {
           const { file } = state;
           let fileJsxPragma = null;
           let fileJsxPragmaFrag = null;
-          const programNode = path.node;
+          const programScope = path.scope;
 
-          if (!GLOBAL_TYPES.has(programNode)) {
-            GLOBAL_TYPES.set(programNode, new Set());
+          if (!GLOBAL_TYPES.has(programScope)) {
+            GLOBAL_TYPES.set(programScope, new Set());
           }
 
           if (file.ast.comments) {
@@ -252,7 +251,7 @@ export default declare((api, opts: Options) => {
 
               if (stmt.node.importKind === "type") {
                 for (const specifier of stmt.node.specifiers) {
-                  registerGlobalType(programNode, specifier.local.name);
+                  registerGlobalType(programScope, specifier.local.name);
                 }
                 stmt.remove();
                 continue;
@@ -269,7 +268,7 @@ export default declare((api, opts: Options) => {
                   specifier.type === "ImportSpecifier" &&
                   specifier.importKind === "type"
                 ) {
-                  registerGlobalType(programNode, specifier.local.name);
+                  registerGlobalType(programScope, specifier.local.name);
                   const binding = stmt.scope.getBinding(specifier.local.name);
                   if (binding) {
                     importsToRemove.add(binding.path);
@@ -332,7 +331,7 @@ export default declare((api, opts: Options) => {
 
             if (stmt.isVariableDeclaration({ declare: true })) {
               for (const name of Object.keys(stmt.getBindingIdentifiers())) {
-                registerGlobalType(programNode, name);
+                registerGlobalType(programScope, name);
               }
             } else if (
               stmt.isTSTypeAliasDeclaration() ||
@@ -344,7 +343,7 @@ export default declare((api, opts: Options) => {
                 stmt.get("id").isIdentifier())
             ) {
               registerGlobalType(
-                programNode,
+                programScope,
                 //@ts-expect-error
                 stmt.node.id.name,
               );
diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-const-enums/input.ts b/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-const-enums/input.ts
@@ -0,0 +1,2 @@
+const enum None {};
+export type { None };
diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-const-enums/options.json b/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-const-enums/options.json
@@ -0,0 +1,5 @@
+{
+  "plugins": ["transform-typescript"],
+  "sourceType": "module",
+  "validateLogs": true
+}
diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-const-enums/output.mjs b/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-const-enums/output.mjs
@@ -0,0 +1,6 @@
+var None;
+
+(function (None) {})(None || (None = {}));
+
+;
+export {};
diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-context-variables/input.ts b/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-context-variables/input.ts
@@ -0,0 +1 @@
+export default undefined;
diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-context-variables/options.json b/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-context-variables/options.json
@@ -0,0 +1,5 @@
+{
+  "plugins": ["transform-typescript"],
+  "sourceType": "module",
+  "validateLogs": true
+}
diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-context-variables/output.mjs b/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-context-variables/output.mjs
@@ -0,0 +1 @@
+export default undefined;
diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-globals/input.ts b/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-globals/input.ts
@@ -0,0 +1 @@
+export default Math;
diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-globals/options.json b/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-globals/options.json
@@ -0,0 +1,5 @@
+{
+  "plugins": ["transform-typescript"],
+  "sourceType": "module",
+  "validateLogs": true
+}
diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-globals/output.mjs b/packages/babel-plugin-transform-typescript/test/fixtures/exports/export-globals/output.mjs
@@ -0,0 +1 @@
+export default Math;
diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/optimize-const-enums/export-const-enum-type-and-value/options.json b/packages/babel-plugin-transform-typescript/test/fixtures/optimize-const-enums/export-const-enum-type-and-value/options.json
@@ -0,0 +1,5 @@
+{
+  "plugins": [["transform-typescript", { "optimizeConstEnums": true }]],
+  "sourceType": "module",
+  "validateLogs": true
+}
diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/optimize-const-enums/export-const-enum-type-no-deopt/options.json b/packages/babel-plugin-transform-typescript/test/fixtures/optimize-const-enums/export-const-enum-type-no-deopt/options.json
@@ -0,0 +1,5 @@
+{
+  "plugins": [["transform-typescript", { "optimizeConstEnums": true }]],
+  "sourceType": "module",
+  "validateLogs": true
+}
diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/optimize-const-enums/export-const-enum-type-no-deopt/stderr.txt b/packages/babel-plugin-transform-typescript/test/fixtures/optimize-const-enums/export-const-enum-type-no-deopt/stderr.txt
@@ -0,0 +1,16 @@
+The exported identifier "WhitespaceFlag" is not declared in Babel's scope tracker
+as a JavaScript value binding, and "@babel/plugin-transform-typescript"
+never encountered it as a TypeScript type declaration.
+It will be treated as a JavaScript value.
+
+This problem is likely caused by another plugin injecting
+"WhitespaceFlag" without registering it in the scope tracker. If you are the author
+ of that plugin, please use "scope.registerDeclaration(declarationPath)".
+The exported identifier "WhitespaceFlag" is not declared in Babel's scope tracker
+as a JavaScript value binding, and "@babel/plugin-transform-typescript"
+never encountered it as a TypeScript type declaration.
+It will be treated as a JavaScript value.
+
+This problem is likely caused by another plugin injecting
+"WhitespaceFlag" without registering it in the scope tracker. If you are the author
+ of that plugin, please use "scope.registerDeclaration(declarationPath)".
diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/regression/10162/options.json b/packages/babel-plugin-transform-typescript/test/fixtures/regression/10162/options.json
@@ -1,4 +1,5 @@
 {
   "plugins": ["./plugin.js", "transform-typescript"],
-  "sourceType": "module"
+  "sourceType": "module",
+  "validateLogs": true
 }
diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/regression/10162/output.mjs b/packages/babel-plugin-transform-typescript/test/fixtures/regression/10162/output.mjs
@@ -1,3 +1,2 @@
 const foo = 2;
 export default foo;
-["The exported identifier \"foo\" is not declared in Babel's scope tracker\nas a JavaScript value binding, and \"@babel/plugin-transform-typescript\"\nnever encountered it as a TypeScript type declaration.\nIt will be treated as a JavaScript value.\n\nThis problem is likely caused by another plugin injecting\n\"foo\" without registering it in the scope tracker. If you are the author\n of that plugin, please use \"scope.registerDeclaration(declarationPath)\"."];
diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/regression/10162/plugin.js b/packages/babel-plugin-transform-typescript/test/fixtures/regression/10162/plugin.js
@@ -1,22 +1,7 @@
 "use strict";
 
-module.exports = function({ template, types: t }) {
-  const warnings = [];
-  let consoleWarn;
-
+module.exports = function ({ template, types: t }) {
   return {
-    pre() {
-      consoleWarn = console.warn;
-      console.warn = msg => warnings.push(msg);
-    },
-
-    post({ path }) {
-      console.warn = consoleWarn;
-
-      const stmt = t.expressionStatement(t.valueToNode(warnings));
-      path.pushContainer("body", stmt);
-    },
-
     visitor: {
       ExportDefaultDeclaration(path) {
         path.insertBefore(template.statement.ast`
diff --git a/packages/babel-plugin-transform-typescript/test/fixtures/regression/10162/stderr.txt b/packages/babel-plugin-transform-typescript/test/fixtures/regression/10162/stderr.txt
@@ -0,0 +1,8 @@
+The exported identifier "foo" is not declared in Babel's scope tracker
+as a JavaScript value binding, and "@babel/plugin-transform-typescript"
+never encountered it as a TypeScript type declaration.
+It will be treated as a JavaScript value.
+
+This problem is likely caused by another plugin injecting
+"foo" without registering it in the scope tracker. If you are the author
+ of that plugin, please use "scope.registerDeclaration(declarationPath)".

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+const enum None {};`
	`2`	`+export type { None };`