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 TypeScript Enum self-references #14093

Merged
merged 6 commits into from Jan 6, 2022
Merged

Fix TypeScript Enum self-references #14093

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1326eaa
Fix TypeScript Enum Reference
magic-akari Dec 31, 2021
83c7fa0
Update enum tests
magic-akari Jan 1, 2022
40c50c2
Update enum tests
magic-akari Jan 1, 2022
933d994
FIX TypeScript Enum
magic-akari Jan 1, 2022
741614d
Refactor TypeScript Enum Reference
magic-akari Jan 1, 2022
2c8362f
fix: resolve shadow binding
magic-akari Jan 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
28 changes: 26 additions & 2 deletions packages/babel-plugin-transform-typescript/src/enum.ts
View file
Open in desktop
@@ -1,7 +1,8 @@
import assert from "assert";
import { template } from "@babel/core";
import type * as t from "@babel/types";
import type { NodePath } from "@babel/traverse";
import traverse from "@babel/traverse";
import type { NodePath, Visitor } from "@babel/traverse";

export default function transpileEnum(path, t) {
const { node } = path;
Expand Down Expand Up @@ -124,7 +125,29 @@ export function translateEnumValues(
value = t.stringLiteral(constValue);
}
} else {
value = initializer;
const IdentifierVisitor: Visitor = {
Copy link
Member

Choose a reason for hiding this comment

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

Please move this visitor to the top-level of the file. We do some processing+validation to visitors, and if it's always the same object this work can be cached. You can get the seen map by using the "state" parameter:

          Identifier(expr, { seen }) {
            // ...

(style nit: we always call non-classes with a name that starts with a lowercase letter. Maybe enumSelfReferenceVisitor.

Identifier(expr) {
if (seen.has(expr.node.name)) {
expr.replaceWith(
t.memberExpression(
t.cloneNode(path.node.id),
t.cloneNode(expr.node),
),
);
expr.skip();
}
},
MemberExpression(expr) {
expr.skip();
},
Copy link
Member

Choose a reason for hiding this comment

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

I think that skipping member expressions breaks transpilation of this code:

enum A {
    a = 0,
    b = 1,
    // @ts-ignore
    x = a.toString(),
}

You can try visiting ReferencedIdentifier instead of Identifier, so that it only visits the "correct" identifiers and you don't need to manually skip member expressions.

};

const exprStmt = t.expressionStatement(initializer);

traverse(t.program([exprStmt]), IdentifierVisitor);

value = exprStmt.expression;
Copy link
Member

Choose a reason for hiding this comment

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

Rather than creating a "fake" program in order to traverse this, you can:

  1. Replace return path.node.members.map(member => { a few lines above with 
    return path.members.map(memberPath => {
    so that you have access to the NodePaths rather than to the raw nodes
  2. Get the initializerPath with memberPath.get("initializer")
  3. Traverse it with 
     if (initializerPath.isIdentifier()) {
   enumSelfReferenceVisitor.Identifier(initializerPath, { seen });
 } else {
   initializerPath.traverse(enumSelfReferenceVisitor, { seen });
 }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, it helps a lot.

I've run into another problem.
After Visitor is used once, the internal ReferencedIdentifier function is not accessible again, which causes me to bring it up in the top scope.

seen.set(name, undefined);
}
} else if (typeof constValue === "number") {
constValue += 1;
Expand All @@ -140,6 +163,7 @@ export function translateEnumValues(
true,
);
value = t.binaryExpression("+", t.numericLiteral(1), lastRef);
seen.set(name, undefined);
}

lastName = name;
Expand Down
14 changes: 14 additions & 0 deletions packages/babel-plugin-transform-typescript/test/fixtures/enum/mix-references/input.ts
View file
Open in desktop
@@ -0,0 +1,14 @@
var x = 10;

enum Foo {
a = 10,
b = a,
c = b + x,
}

enum Bar {
D = Foo.a,
E = D,
F = Math.E,
G = E + Foo.c,
}
17 changes: 17 additions & 0 deletions packages/babel-plugin-transform-typescript/test/fixtures/enum/mix-references/output.js
View file
Open in desktop
@@ -0,0 +1,17 @@
var x = 10;
var Foo;

(function (Foo) {
Foo[Foo["a"] = 10] = "a";
Foo[Foo["b"] = 10] = "b";
Foo[Foo["c"] = Foo.b + x] = "c";
})(Foo || (Foo = {}));

var Bar;

(function (Bar) {
Bar[Bar["D"] = Foo.a] = "D";
Bar[Bar["E"] = Bar.D] = "E";
Bar[Bar["F"] = Math.E] = "F";
Bar[Bar["G"] = Bar.E + Foo.c] = "G";
})(Bar || (Bar = {}));