Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 4 commits into from Aug 4, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
545c410
test: update regression/10162
JLHwung Aug 3, 2022
2fd0724
fix: global variables must not be global types
JLHwung Aug 3, 2022
96103fe
refactor: let GLOBAL_TYPES be a map<scope, set>
JLHwung Aug 3, 2022
e400e27
log warning messages to test fixtures
JLHwung Aug 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 14 additions & 15 deletions packages/babel-plugin-transform-typescript/src/index.ts
View file
Open in desktop
Expand Up @@ -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";
Expand All @@ -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` +
Expand All @@ -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 */
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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() ||
Expand All @@ -344,7 +343,7 @@ export default declare((api, opts: Options) => {
stmt.get("id").isIdentifier())
) {
registerGlobalType(
programNode,
programScope,
//@ts-expect-error
stmt.node.id.name,
);
Expand Down
2 changes: 2 additions & 0 deletions packages/babel-plugin-transform-typescript/test/fixtures/exports/export-const-enums/input.ts
View file
Open in desktop
@@ -0,0 +1,2 @@
const enum None {};
export type { None };
5 changes: 5 additions & 0 deletions ...s/babel-plugin-transform-typescript/test/fixtures/exports/export-const-enums/options.json
View file
Open in desktop
@@ -0,0 +1,5 @@
{
"plugins": ["transform-typescript"],
"sourceType": "module",
"validateLogs": true
}
6 changes: 6 additions & 0 deletions ...ges/babel-plugin-transform-typescript/test/fixtures/exports/export-const-enums/output.mjs
View file
Open in desktop
@@ -0,0 +1,6 @@
var None;

(function (None) {})(None || (None = {}));

;
export {};
1 change: 1 addition & 0 deletions ...babel-plugin-transform-typescript/test/fixtures/exports/export-context-variables/input.ts
View file
Open in desktop
@@ -0,0 +1 @@
export default undefined;
5 changes: 5 additions & 0 deletions ...l-plugin-transform-typescript/test/fixtures/exports/export-context-variables/options.json
View file
Open in desktop
@@ -0,0 +1,5 @@
{
"plugins": ["transform-typescript"],
"sourceType": "module",
"validateLogs": true
}
1 change: 1 addition & 0 deletions ...bel-plugin-transform-typescript/test/fixtures/exports/export-context-variables/output.mjs
View file
Open in desktop
@@ -0,0 +1 @@
export default undefined;
1 change: 1 addition & 0 deletions packages/babel-plugin-transform-typescript/test/fixtures/exports/export-globals/input.ts
View file
Open in desktop
@@ -0,0 +1 @@
export default Math;
5 changes: 5 additions & 0 deletions packages/babel-plugin-transform-typescript/test/fixtures/exports/export-globals/options.json
View file
Open in desktop
@@ -0,0 +1,5 @@
{
"plugins": ["transform-typescript"],
"sourceType": "module",
"validateLogs": true
}
1 change: 1 addition & 0 deletions packages/babel-plugin-transform-typescript/test/fixtures/exports/export-globals/output.mjs
View file
Open in desktop
@@ -0,0 +1 @@
export default Math;
5 changes: 5 additions & 0 deletions ...pescript/test/fixtures/optimize-const-enums/export-const-enum-type-and-value/options.json
View file
Open in desktop
@@ -0,0 +1,5 @@
{
"plugins": [["transform-typescript", { "optimizeConstEnums": true }]],
"sourceType": "module",
"validateLogs": true
}
5 changes: 5 additions & 0 deletions ...ypescript/test/fixtures/optimize-const-enums/export-const-enum-type-no-deopt/options.json
View file
Open in desktop
@@ -0,0 +1,5 @@
{
"plugins": [["transform-typescript", { "optimizeConstEnums": true }]],
"sourceType": "module",
"validateLogs": true
}
16 changes: 16 additions & 0 deletions ...-typescript/test/fixtures/optimize-const-enums/export-const-enum-type-no-deopt/stderr.txt
View file
Open in desktop
@@ -0,0 +1,16 @@
The exported identifier "WhitespaceFlag" is not declared in Babel's scope tracker
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test case is

const enum WhitespaceFlag {
  before = 1 << 0,
  after = 1 << 1,
}

export type { WhitespaceFlag as WF1 };
export { type WhitespaceFlag as WF2 };

export const before = WhitespaceFlag.before;

We first remove the enum declaration WhitespaceFlag, so the referenced WhitespaceFlag becomes invalid and the warning is triggered. In a way it is similar to #14818 where we removed the scope binding info but didn't fix its now-dangling referenced paths.

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)".
3 changes: 2 additions & 1 deletion packages/babel-plugin-transform-typescript/test/fixtures/regression/10162/options.json
View file
Open in desktop
@@ -1,4 +1,5 @@
{
"plugins": ["./plugin.js", "transform-typescript"],
"sourceType": "module"
"sourceType": "module",
"validateLogs": true
}
1 change: 0 additions & 1 deletion packages/babel-plugin-transform-typescript/test/fixtures/regression/10162/output.mjs
View file
Open in desktop
@@ -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)\"."];
17 changes: 1 addition & 16 deletions packages/babel-plugin-transform-typescript/test/fixtures/regression/10162/plugin.js
View file
Open in desktop
@@ -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`
Expand Down
8 changes: 8 additions & 0 deletions packages/babel-plugin-transform-typescript/test/fixtures/regression/10162/stderr.txt
View file
Open in desktop
@@ -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)".