Skip to content

fix: emit async wrapper for TLA modules under onDemandWrapping#10086

Merged
graphite-app[bot] merged 1 commit into
mainfrom
fix/on-demand-wrapping-tla-async-wrapper
Jul 2, 2026
Merged

fix: emit async wrapper for TLA modules under onDemandWrapping#10086
graphite-app[bot] merged 1 commit into
mainfrom
fix/on-demand-wrapping-tla-async-wrapper

Conversation

@IWANABETHATGUY

@IWANABETHATGUY IWANABETHATGUY commented Jul 2, 2026

Copy link
Copy Markdown
Member

What this PR solves

Under experimental.onDemandWrapping, the concatenated module-group wrapper — the single __esm closure holding the bodies of every module in a merged group — was always emitted as a sync arrow. When the group's entry is TLA-tainted (has top-level await, or awaits a TLA importee's init call), those awaits land inside that closure, producing a chunk with await in a non-async function: a SyntaxError at load.

// before
var init_entry = __esm(() => {
  ...
  await init_dep();  // SyntaxError: await outside async function
});

// after
var init_entry = __esm(async () => {
  ...
  await init_dep();
});

How

One render change in render_chunk_content (crates/rolldown/src/ecmascript/format/esm.rs): emit async () => { when metas[group.entry].is_tla_or_contains_tla_dependency. This mirrors the is_async flag already threaded into make_esm_wrapper_stmt for the non-concatenated (single-module) wrapper path — the concatenated group path just never received it.

Tests

Added { "strictExecutionOrder": true, "onDemandWrapping": true } config variants to the TLA fixtures so the emitted syntax is pinned by snapshots (and executed):

  • issues/9083
  • topics/tla/on_demand_await_call
  • topics/tla/transitive_dep
  • function/experimental/on_demand_wrapping/top_level_await_syntax

Each fails on main with the sync closure and passes with this change.

IWANABETHATGUY commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add the label graphite: merge-when-ready to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@graphite-app graphite-app Bot changed the base branch from fix/body-demand-side-effect-inclusion to graphite-base/10086 July 2, 2026 10:58
@graphite-app graphite-app Bot force-pushed the graphite-base/10086 branch from 9897950 to fdb678e Compare July 2, 2026 11:03
@graphite-app graphite-app Bot force-pushed the fix/on-demand-wrapping-tla-async-wrapper branch from 67054c1 to c9f92db Compare July 2, 2026 11:03
@graphite-app graphite-app Bot changed the base branch from graphite-base/10086 to main July 2, 2026 11:04
@graphite-app graphite-app Bot force-pushed the fix/on-demand-wrapping-tla-async-wrapper branch from c9f92db to f084ccc Compare July 2, 2026 11:04
@netlify

netlify Bot commented Jul 2, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit bc6ce75
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a4693ceea7c0d000808ea0e

@IWANABETHATGUY IWANABETHATGUY force-pushed the fix/on-demand-wrapping-tla-async-wrapper branch from f084ccc to 72350c6 Compare July 2, 2026 16:21
@IWANABETHATGUY IWANABETHATGUY marked this pull request as ready for review July 2, 2026 16:28
@codspeed-hq

codspeed-hq Bot commented Jul 2, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing fix/on-demand-wrapping-tla-async-wrapper (72350c6) with main (76593a4)2

Open in CodSpeed

Footnotes

  1. 10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (b19483b) during the generation of this report, so 76593a4 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

hyf0 commented Jul 2, 2026

Copy link
Copy Markdown
Member

Merge activity

### What this PR solves

Under `experimental.onDemandWrapping`, the concatenated module-group wrapper — the single `__esm` closure holding the bodies of every module in a merged group — was always emitted as a sync arrow. When the group's entry is TLA-tainted (has top-level `await`, or awaits a TLA importee's `init` call), those `await`s land inside that closure, producing a chunk with `await` in a non-async function: a SyntaxError at load.

```js
// before
var init_entry = __esm(() => {
  ...
  await init_dep();  // SyntaxError: await outside async function
});

// after
var init_entry = __esm(async () => {
  ...
  await init_dep();
});
```

### How

One render change in `render_chunk_content` (`crates/rolldown/src/ecmascript/format/esm.rs`): emit `async () => {` when `metas[group.entry].is_tla_or_contains_tla_dependency`. This mirrors the `is_async` flag already threaded into `make_esm_wrapper_stmt` for the non-concatenated (single-module) wrapper path — the concatenated group path just never received it.

### Tests

Added `{ "strictExecutionOrder": true, "onDemandWrapping": true }` config variants to the TLA fixtures so the emitted syntax is pinned by snapshots (and executed):

- `issues/9083`
- `topics/tla/on_demand_await_call`
- `topics/tla/transitive_dep`
- `function/experimental/on_demand_wrapping/top_level_await_syntax`

Each fails on main with the sync closure and passes with this change.
@graphite-app graphite-app Bot force-pushed the fix/on-demand-wrapping-tla-async-wrapper branch from 72350c6 to bc6ce75 Compare July 2, 2026 16:37
@graphite-app graphite-app Bot merged commit bc6ce75 into main Jul 2, 2026
34 checks passed
@graphite-app graphite-app Bot deleted the fix/on-demand-wrapping-tla-async-wrapper branch July 2, 2026 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants