Skip to content

Commit 04627b6

Browse files
evanwevanw
committed
fix #4498: async TLA checks need a worklist
1 parent 5c15177 commit 04627b6

4 files changed

Lines changed: 139 additions & 21 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@
3838

3939
This release puts the original behavior back. With this release, esbuild should now actually avoid overwriting input files unless `--allow-overwrite` is explicitly present. This is done by not writing out any files when a build error is encountered.
4040

41+
* Fix incorrect code generated when using top-level await ([#4498](https://github.com/evanw/esbuild/issues/4498))
42+
43+
Previously esbuild could generate code containing a syntax error in complex scenarios involving top-level await used in a dependency cycle. The problem was a missing `async` on one or more module wrapper closures. With this release, esbuild now uses a fixed-point iteration algorithm to correctly annotate all dependencies in the cycle as needing an `async` module wrapper.
44+
4145
* Fix a minification bug with lowered logical assignment operators ([#4508](https://github.com/evanw/esbuild/issues/4508))
4246

4347
This release fixes a bug that could cause esbuild to generate incorrect code for logical assignment operators when lowering them to an older target environment. Specifically the lowering process requires duplicating the left-hand side, but esbuild incorrectly failed to count the duplicate as a new usage when the left-hand side is an identifier. That then caused the minifier to believe that the left-hand side was only used once and could attempt to incorrectly inline an initializer into the first usage. This bug has now been fixed:

‎internal/bundler/bundler.go‎

Lines changed: 58 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,8 @@ type globResolveResult struct {
116116

117117
type tlaCheck struct {
118118
parent ast.Index32
119-
depth uint32
119+
depth ast.Index32
120+
pass uint32
120121
importRecordIndex uint32
121122
}
122123

@@ -2810,47 +2811,87 @@ func (s *scanner) processScannedFiles(entryPointMeta []graph.EntryPoint) []scann
28102811
s.results[sourceIndex] = result
28112812
}
28122813

2814+
// Traverse the graph to check top-level await
2815+
if s.iterativelyValidateTLA() {
2816+
s.reportInvalidTLA()
2817+
}
2818+
28132819
// The linker operates on an array of files, so construct that now. This
28142820
// can't be constructed earlier because we generate new parse results for
28152821
// JavaScript stub files for CSS imports above.
28162822
files := make([]scannerFile, len(s.results))
28172823
for sourceIndex := range s.results {
28182824
if result := &s.results[sourceIndex]; result.ok {
2819-
s.validateTLA(uint32(sourceIndex))
28202825
files[sourceIndex] = result.file
28212826
}
28222827
}
28232828

28242829
return files
28252830
}
28262831

2827-
func (s *scanner) validateTLA(sourceIndex uint32) tlaCheck {
2832+
func (s *scanner) iterativelyValidateTLA() bool {
2833+
pass := uint32(1)
2834+
hasTLA := false
2835+
2836+
// Iterate until a fixed point has been reached to handle graph cycles
2837+
for {
2838+
didChange := false
2839+
for sourceIndex := range s.results {
2840+
s.recursivelyValidateTLA(uint32(sourceIndex), pass, &didChange)
2841+
}
2842+
if !didChange {
2843+
return hasTLA
2844+
}
2845+
pass++
2846+
hasTLA = true
2847+
}
2848+
}
2849+
2850+
func (s *scanner) recursivelyValidateTLA(sourceIndex uint32, pass uint32, didChange *bool) tlaCheck {
28282851
result := &s.results[sourceIndex]
28292852

2830-
if result.ok && result.tlaCheck.depth == 0 {
2853+
// Use a "pass" integer instead of a separate "visited" set
2854+
if result.ok && result.tlaCheck.pass != pass {
2855+
result.tlaCheck.pass = pass
2856+
28312857
if repr, ok := result.file.inputFile.Repr.(*graph.JSRepr); ok {
2832-
result.tlaCheck.depth = 1
2833-
if repr.AST.LiveTopLevelAwaitKeyword.Len > 0 {
2858+
// If this module contains top-level await, set its parent to itself
2859+
if repr.AST.LiveTopLevelAwaitKeyword.Len > 0 && result.tlaCheck.parent.GetIndex() != sourceIndex {
28342860
result.tlaCheck.parent = ast.MakeIndex32(sourceIndex)
2861+
result.tlaCheck.depth = ast.MakeIndex32(1)
2862+
*didChange = true
28352863
}
28362864

2865+
// Check all import statements and require calls (only import statements are valid)
28372866
for importRecordIndex, record := range repr.AST.ImportRecords {
28382867
if record.SourceIndex.IsValid() && (record.Kind == ast.ImportRequire || record.Kind == ast.ImportStmt) {
2839-
parent := s.validateTLA(record.SourceIndex.GetIndex())
2840-
if !parent.parent.IsValid() {
2841-
continue
2842-
}
2868+
parent := s.recursivelyValidateTLA(record.SourceIndex.GetIndex(), pass, didChange)
28432869

2844-
// Follow any import chains
2845-
if record.Kind == ast.ImportStmt && (!result.tlaCheck.parent.IsValid() || parent.depth < result.tlaCheck.depth) {
2846-
result.tlaCheck.depth = parent.depth + 1
2870+
// Track the shallowest top-level await parent (used to report invalid import chains later on)
2871+
if record.Kind == ast.ImportStmt && parent.depth.GetIndex() < result.tlaCheck.depth.GetIndex()-1 {
28472872
result.tlaCheck.parent = record.SourceIndex
2873+
result.tlaCheck.depth = ast.MakeIndex32(parent.depth.GetIndex() + 1)
28482874
result.tlaCheck.importRecordIndex = uint32(importRecordIndex)
2875+
*didChange = true
28492876
continue
28502877
}
2878+
}
2879+
}
2880+
}
2881+
}
2882+
2883+
return result.tlaCheck
2884+
}
28512885

2886+
func (s *scanner) reportInvalidTLA() {
2887+
for sourceIndex := range s.results {
2888+
result := &s.results[sourceIndex]
2889+
2890+
if result.ok && result.tlaCheck.parent.IsValid() {
2891+
if repr, ok := result.file.inputFile.Repr.(*graph.JSRepr); ok {
2892+
for _, record := range repr.AST.ImportRecords {
28522893
// Require of a top-level await chain is forbidden
2853-
if record.Kind == ast.ImportRequire {
2894+
if record.Kind == ast.ImportRequire && record.SourceIndex.IsValid() && s.results[record.SourceIndex.GetIndex()].tlaCheck.parent.IsValid() {
28542895
var notes []logger.MsgData
28552896
var tlaPrettyPaths logger.PrettyPaths
28562897
otherSourceIndex := record.SourceIndex.GetIndex()
@@ -2899,18 +2940,14 @@ func (s *scanner) validateTLA(sourceIndex uint32) tlaCheck {
28992940
s.log.AddErrorWithNotes(&tracker, record.Range, text, notes)
29002941
}
29012942
}
2902-
}
29032943

2904-
// Make sure that if we wrap this module in a closure, the closure is also
2905-
// async. This happens when you call "import()" on this module and code
2906-
// splitting is off.
2907-
if result.tlaCheck.parent.IsValid() {
2944+
// Make sure that if we wrap this module in a closure, the closure is also
2945+
// async. This happens when you call "import()" on this module and code
2946+
// splitting is off.
29082947
repr.Meta.IsAsyncOrHasAsyncDependency = true
29092948
}
29102949
}
29112950
}
2912-
2913-
return result.tlaCheck
29142951
}
29152952

29162953
func DefaultExtensionToLoaderMap() map[string]config.Loader {

‎internal/bundler_tests/bundler_default_test.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4377,6 +4377,39 @@ func TestTopLevelAwaitAllowedImportWithSplitting(t *testing.T) {
43774377
})
43784378
}
43794379

4380+
// https://github.com/evanw/esbuild/issues/4498
4381+
func TestTopLevelAwaitCyclicDependenciesIssue4498(t *testing.T) {
4382+
default_suite.expectBundled(t, bundled{
4383+
files: map[string]string{
4384+
"/entry.mjs": `
4385+
await import("./main.mjs");
4386+
`,
4387+
"/main.mjs": `
4388+
import { a } from "./a.mjs";
4389+
console.log(a());
4390+
`,
4391+
"/a.mjs": `
4392+
import { b } from "./b.mjs";
4393+
import { tla } from "./dep.mjs";
4394+
export function a() { return b() + tla; }
4395+
`,
4396+
"/b.mjs": `
4397+
import { a } from "./a.mjs";
4398+
export function b() { return typeof a; }
4399+
`,
4400+
"/dep.mjs": `
4401+
export const tla = await Promise.resolve("x");
4402+
`,
4403+
},
4404+
entryPaths: []string{"/entry.mjs"},
4405+
options: config.Options{
4406+
Mode: config.ModeBundle,
4407+
OutputFormat: config.FormatESModule,
4408+
AbsOutputDir: "/out",
4409+
},
4410+
})
4411+
}
4412+
43804413
func TestAssignToImport(t *testing.T) {
43814414
default_suite.expectBundled(t, bundled{
43824415
files: map[string]string{

‎internal/bundler_tests/snapshots/snapshots_default.txt‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6897,6 +6897,50 @@ TestTopLevelAwaitCJSDeadBranch
68976897
if (false) foo;
68986898
if (false) for (foo of bar) ;
68996899

6900+
================================================================================
6901+
TestTopLevelAwaitCyclicDependenciesIssue4498
6902+
---------- /out/entry.js ----------
6903+
// b.mjs
6904+
function b() {
6905+
return typeof a;
6906+
}
6907+
var init_b = __esm({
6908+
async "b.mjs"() {
6909+
await init_a();
6910+
}
6911+
});
6912+
6913+
// dep.mjs
6914+
var tla;
6915+
var init_dep = __esm({
6916+
async "dep.mjs"() {
6917+
tla = await Promise.resolve("x");
6918+
}
6919+
});
6920+
6921+
// a.mjs
6922+
function a() {
6923+
return b() + tla;
6924+
}
6925+
var init_a = __esm({
6926+
async "a.mjs"() {
6927+
await init_b();
6928+
await init_dep();
6929+
}
6930+
});
6931+
6932+
// main.mjs
6933+
var main_exports = {};
6934+
var init_main = __esm({
6935+
async "main.mjs"() {
6936+
await init_a();
6937+
console.log(a());
6938+
}
6939+
});
6940+
6941+
// entry.mjs
6942+
await init_main().then(() => main_exports);
6943+
69006944
================================================================================
69016945
TestTopLevelAwaitESM
69026946
---------- /out.js ----------

0 commit comments

Comments
 (0)