Skip to content

Commit

Permalink
fix #3210: cross-module inlining of string enums
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Jul 12, 2023
1 parent aad8818 commit 33933c4
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 61 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@

Code that uses a newline after `export type` is now allowed starting with this release.

* Fix cross-module inlining of string enums ([#3210](https://github.com/evanw/esbuild/issues/3210))

A refactoring typo in version 0.18.9 accidentally introduced a regression with cross-module inlining of string enums when combined with computed property accesses. This regression has been fixed.

* Rewrite `.js` to `.ts` inside packages with `exports` ([#3201](https://github.com/evanw/esbuild/issues/3201))

Packages with the `exports` field are supposed to disable node's path resolution behavior that allows you to import a file with a different extension than the one in the source code (for example, importing `foo/bar` to get `foo/bar.js`). And TypeScript has behavior where you can import a non-existent `.js` file and you will get the `.ts` file instead. Previously the presence of the `exports` field caused esbuild to disable all extension manipulation stuff which included both node's implicit file extension searching and TypeScript's file extension swapping. However, TypeScript appears to always apply file extension swapping even in this case. So with this release, esbuild will now rewrite `.js` to `.ts` even inside packages with `exports`.
Expand Down
87 changes: 62 additions & 25 deletions internal/bundler_tests/bundler_ts_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2094,18 +2094,35 @@ func TestTSEnumSameModuleInliningAccess(t *testing.T) {
ts_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.ts": `
enum a { x = 123 }
enum b { x = 123 }
enum c { x = 123 }
enum d { x = 123 }
enum e { x = 123 }
console.log([
a.x,
b['x'],
c?.x,
d?.['x'],
e,
])
enum a_num { x = 123 }
enum b_num { x = 123 }
enum c_num { x = 123 }
enum d_num { x = 123 }
enum e_num { x = 123 }
enum a_str { x = 'abc' }
enum b_str { x = 'abc' }
enum c_str { x = 'abc' }
enum d_str { x = 'abc' }
enum e_str { x = 'abc' }
inlined = [
a_num.x,
b_num['x'],
a_str.x,
b_str['x'],
]
not_inlined = [
c_num?.x,
d_num?.['x'],
e_num,
c_str?.x,
d_str?.['x'],
e_str,
]
`,
},
entryPaths: []string{"/entry.ts"},
Expand All @@ -2120,21 +2137,41 @@ func TestTSEnumCrossModuleInliningAccess(t *testing.T) {
ts_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.ts": `
import { a, b, c, d, e } from './enums'
console.log([
a.x,
b['x'],
c?.x,
d?.['x'],
e,
])
import {
a_num, b_num, c_num, d_num, e_num,
a_str, b_str, c_str, d_str, e_str,
} from './enums'
inlined = [
a_num.x,
b_num['x'],
a_str.x,
b_str['x'],
]
not_inlined = [
c_num?.x,
d_num?.['x'],
e_num,
c_str?.x,
d_str?.['x'],
e_str,
]
`,
"/enums.ts": `
export enum a { x = 123 }
export enum b { x = 123 }
export enum c { x = 123 }
export enum d { x = 123 }
export enum e { x = 123 }
export enum a_num { x = 123 }
export enum b_num { x = 123 }
export enum c_num { x = 123 }
export enum d_num { x = 123 }
export enum e_num { x = 123 }
export enum a_str { x = 'abc' }
export enum b_str { x = 'abc' }
export enum c_str { x = 'abc' }
export enum d_str { x = 'abc' }
export enum e_str { x = 'abc' }
`,
},
entryPaths: []string{"/entry.ts"},
Expand Down
106 changes: 72 additions & 34 deletions internal/bundler_tests/snapshots/snapshots_ts.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,27 +301,46 @@ var foo = bar();
TestTSEnumCrossModuleInliningAccess
---------- /out/entry.js ----------
// enums.ts
var c = /* @__PURE__ */ ((c2) => {
c2[c2["x"] = 123] = "x";
return c2;
})(c || {});
var d = /* @__PURE__ */ ((d2) => {
d2[d2["x"] = 123] = "x";
return d2;
})(d || {});
var e = /* @__PURE__ */ ((e2) => {
e2[e2["x"] = 123] = "x";
return e2;
})(e || {});
var c_num = /* @__PURE__ */ ((c_num2) => {
c_num2[c_num2["x"] = 123] = "x";
return c_num2;
})(c_num || {});
var d_num = /* @__PURE__ */ ((d_num2) => {
d_num2[d_num2["x"] = 123] = "x";
return d_num2;
})(d_num || {});
var e_num = /* @__PURE__ */ ((e_num2) => {
e_num2[e_num2["x"] = 123] = "x";
return e_num2;
})(e_num || {});
var c_str = /* @__PURE__ */ ((c_str2) => {
c_str2["x"] = "abc";
return c_str2;
})(c_str || {});
var d_str = /* @__PURE__ */ ((d_str2) => {
d_str2["x"] = "abc";
return d_str2;
})(d_str || {});
var e_str = /* @__PURE__ */ ((e_str2) => {
e_str2["x"] = "abc";
return e_str2;
})(e_str || {});

// entry.ts
console.log([
inlined = [
123 /* x */,
123 /* x */,
c?.x,
d?.["x"],
e
]);
"abc" /* x */,
"abc" /* x */
];
not_inlined = [
c_num?.x,
d_num?.["x"],
e_num,
c_str?.x,
d_str?.["x"],
e_str
];

================================================================================
TestTSEnumCrossModuleInliningDefinitions
Expand Down Expand Up @@ -486,25 +505,44 @@ var x;
TestTSEnumSameModuleInliningAccess
---------- /out/entry.js ----------
// entry.ts
var c = /* @__PURE__ */ ((c2) => {
c2[c2["x"] = 123] = "x";
return c2;
})(c || {});
var d = /* @__PURE__ */ ((d2) => {
d2[d2["x"] = 123] = "x";
return d2;
})(d || {});
var e = /* @__PURE__ */ ((e2) => {
e2[e2["x"] = 123] = "x";
return e2;
})(e || {});
console.log([
var c_num = /* @__PURE__ */ ((c_num2) => {
c_num2[c_num2["x"] = 123] = "x";
return c_num2;
})(c_num || {});
var d_num = /* @__PURE__ */ ((d_num2) => {
d_num2[d_num2["x"] = 123] = "x";
return d_num2;
})(d_num || {});
var e_num = /* @__PURE__ */ ((e_num2) => {
e_num2[e_num2["x"] = 123] = "x";
return e_num2;
})(e_num || {});
var c_str = /* @__PURE__ */ ((c_str2) => {
c_str2["x"] = "abc";
return c_str2;
})(c_str || {});
var d_str = /* @__PURE__ */ ((d_str2) => {
d_str2["x"] = "abc";
return d_str2;
})(d_str || {});
var e_str = /* @__PURE__ */ ((e_str2) => {
e_str2["x"] = "abc";
return e_str2;
})(e_str || {});
inlined = [
123 /* x */,
123 /* x */,
c?.x,
d?.["x"],
e
]);
"abc" /* x */,
"abc" /* x */
];
not_inlined = [
c_num?.x,
d_num?.["x"],
e_num,
c_str?.x,
d_str?.["x"],
e_str
];

================================================================================
TestTSEnumTreeShaking
Expand Down
2 changes: 1 addition & 1 deletion internal/js_printer/js_printer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2516,7 +2516,7 @@ func (p *printer) printExpr(expr js_ast.Expr, level js_ast.L, flags printExprFla

// Inline cross-module TypeScript enum references here
if index, ok := e.Index.Data.(*js_ast.EString); ok {
if value, name, ok := p.tryToGetImportedEnumValueUTF16(e.Target, index.Value); ok && value.String == nil {
if value, name, ok := p.tryToGetImportedEnumValueUTF16(e.Target, index.Value); ok {
if value.String != nil {
p.printQuotedUTF16(value.String, true /* allowBacktick */)
} else {
Expand Down
25 changes: 24 additions & 1 deletion scripts/end-to-end-tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,9 @@ tests.push(
}),
)

// Test TypeScript enum scope merging
// Test TypeScript enum stuff
tests.push(
// Scope merging
test(['entry.ts', '--bundle', '--minify', '--outfile=node.js'], {
'entry.ts': `
const id = x => x
Expand Down Expand Up @@ -156,6 +157,28 @@ tests.push(
if (x()() !== 123) throw 'fail'
`,
}),

// https://github.com/evanw/esbuild/issues/3210
test(['entry.ts', '--bundle', '--outfile=node.js'], {
'entry.ts': `
import { MyEnum } from './enums';
enum MyEnum2 {
'A.A' = 'a',
'aa' = 'aa',
}
if (
MyEnum['A.A'] !== 'a' || MyEnum2['A.A'] !== 'a' ||
MyEnum.aa !== 'aa' || MyEnum2['aa'] !== 'aa' ||
MyEnum['aa'] !== 'aa' || MyEnum2.aa !== 'aa'
) throw 'fail'
`,
'enums.ts': `
export enum MyEnum {
'A.A' = 'a',
'aa' = 'aa',
}
`,
}),
)

// Check "tsconfig.json" behavior
Expand Down

0 comments on commit 33933c4

Please sign in to comment.