Skip to content

Commit

Permalink
fix #3329: treat more enum values as strings
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Sep 13, 2023
1 parent 5ecf535 commit 4c5db58
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 7 deletions.
42 changes: 42 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,48 @@

Note that this bug only affected code using the `local-css` loader. It did not affect code using the `css` loader.

* Adjust TypeScript `enum` output to better approximate `tsc` ([#3329](https://github.com/evanw/esbuild/issues/3329))

TypeScript enum values can be either number literals or string literals. Numbers create a bidirectional mapping between the name and the value but strings only create a unidirectional mapping from the name to the value. When the enum value is neither a number literal nor a string literal, TypeScript and esbuild both default to treating it as a number:

```ts
// Original TypeScript code
declare const foo: any
enum Foo {
NUMBER = 1,
STRING = 'a',
OTHER = foo,
}

// Compiled JavaScript code (from "tsc")
var Foo;
(function (Foo) {
Foo[Foo["NUMBER"] = 1] = "NUMBER";
Foo["STRING"] = "a";
Foo[Foo["OTHER"] = foo] = "OTHER";
})(Foo || (Foo = {}));
```

However, TypeScript does constant folding slightly differently than esbuild. For example, it may consider template literals to be string literals in some cases:

```ts
// Original TypeScript code
declare const foo = 'foo'
enum Foo {
PRESENT = `${foo}`,
MISSING = `${bar}`,
}

// Compiled JavaScript code (from "tsc")
var Foo;
(function (Foo) {
Foo["PRESENT"] = "foo";
Foo[Foo["MISSING"] = `${bar}`] = "MISSING";
})(Foo || (Foo = {}));
```

The template literal initializer for `PRESENT` is treated as a string while the template literal initializer for `MISSING` is treated as a number. Previously esbuild treated both of these cases as a number but starting with this release, esbuild will now treat both of these cases as a string. This doesn't exactly match the behavior of `tsc` but in the case where the behavior diverges `tsc` reports a compile error, so this seems like acceptible behavior for esbuild. Note that handling these cases completely correctly would require esbuild to parse type declarations (see the `declare` keyword), which esbuild deliberately doesn't do.

* Ignore case in CSS in more places ([#3316](https://github.com/evanw/esbuild/issues/3316))

This release makes esbuild's CSS support more case-agnostic, which better matches how browsers work. For example:
Expand Down
12 changes: 6 additions & 6 deletions internal/bundler_tests/snapshots/snapshots_ts.txt
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ console.log(
// not-supported.ts
var NonIntegerNumberToString = ((NonIntegerNumberToString2) => {
NonIntegerNumberToString2["SUPPORTED"] = "1";
NonIntegerNumberToString2[NonIntegerNumberToString2["UNSUPPORTED"] = "" + 1.5] = "UNSUPPORTED";
NonIntegerNumberToString2["UNSUPPORTED"] = "" + 1.5;
return NonIntegerNumberToString2;
})(NonIntegerNumberToString || {});
console.log(
Expand All @@ -67,18 +67,18 @@ console.log(
);
var OutOfBoundsNumberToString = ((OutOfBoundsNumberToString2) => {
OutOfBoundsNumberToString2["SUPPORTED"] = "1000000000";
OutOfBoundsNumberToString2[OutOfBoundsNumberToString2["UNSUPPORTED"] = "" + 1e12] = "UNSUPPORTED";
OutOfBoundsNumberToString2["UNSUPPORTED"] = "" + 1e12;
return OutOfBoundsNumberToString2;
})(OutOfBoundsNumberToString || {});
console.log(
"1000000000" /* SUPPORTED */,
OutOfBoundsNumberToString.UNSUPPORTED
);
var TemplateExpressions = ((TemplateExpressions2) => {
TemplateExpressions2[TemplateExpressions2["NULL"] = "" + null] = "NULL";
TemplateExpressions2[TemplateExpressions2["TRUE"] = "" + true] = "TRUE";
TemplateExpressions2[TemplateExpressions2["FALSE"] = "" + false] = "FALSE";
TemplateExpressions2[TemplateExpressions2["BIGINT"] = "" + 123n] = "BIGINT";
TemplateExpressions2["NULL"] = "" + null;
TemplateExpressions2["TRUE"] = "" + true;
TemplateExpressions2["FALSE"] = "" + false;
TemplateExpressions2["BIGINT"] = "" + 123n;
return TemplateExpressions2;
})(TemplateExpressions || {});
console.log(
Expand Down
3 changes: 3 additions & 0 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -10792,6 +10792,9 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_
hasStringValue = true

default:
if js_ast.KnownPrimitiveType(underlyingValue.Data) == js_ast.PrimitiveString {
hasStringValue = true
}
if !js_ast.ExprCanBeRemovedIfUnused(underlyingValue, p.isUnbound) {
allValuesArePure = false
}
Expand Down
2 changes: 1 addition & 1 deletion internal/js_parser/ts_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1495,7 +1495,7 @@ func TestTSEnum(t *testing.T) {
Foo[Foo["D"] = void 0] = "D";
Foo[Foo["E"] = void 0] = "E";
Foo["F"] = `+"`y`"+`;
Foo[Foo["G"] = `+"`${z}`"+`] = "G";
Foo["G"] = `+"`${z}`"+`;
Foo[Foo["H"] = tag`+"``"+`] = "H";
return Foo;
})(Foo || {});
Expand Down

0 comments on commit 4c5db58

Please sign in to comment.