Skip to content

fix(transform): #5925 — keep comma-sequence operands ahead of hoisted awaits#5926

Merged
proggeramlug merged 1 commit into
mainfrom
fix/async-seq-await-order
Jul 4, 2026
Merged

fix(transform): #5925 — keep comma-sequence operands ahead of hoisted awaits#5926
proggeramlug merged 1 commit into
mainfrom
fix/async-seq-await-order

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Problem — #5925

The plain-async pre-pass hoists every nested Expr::Await into a let __await_N = await <operand>; above the containing statement. For a comma sequence, that moves the awaited operand's evaluation above the sequence's earlier operands:

this.authCodeListener = new Listener, this.port = await this.authCodeListener.start();

hoists to (pre-fix):

let __await_N = await this.authCodeListener.start();   // reads the field's PRE-assignment value: null
this.authCodeListener = new Listener, this.port = __await_N;
  • node: GOT 42
  • perry-compiled: ERR TypeError: Cannot read properties of null (reading 'start')

Minifiers comma-fold consecutive statements, so any const x = new X(); const y = await x.m(); pair in bundled async code hits this — observed in a real-world 13 MB bundle where it broke an auth flow's listener startup. Conditional branches (#342) and logical RHS (#5434) already have dedicated lifts for this class of unsound hoist; Expr::Sequence had none. Reproduces on current origin/main (v0.5.1219).

Fix

lift_sequence_with_await: when a sequence contains an await, lift the non-final operands to statements pushed onto hoisted (each recursively hoisted first, so awaits inside earlier operands suspend in evaluation order), leaving the final operand as the expression value. The __await_N let the caller hoists for the final operand then lands AFTER the lifted operands. Wired into:

  • hoist_awaits_in_expr_full (nested sequences), and
  • hoist_awaits_avoiding_top_level (statement-position sequences — re-processed after the lift so a final-operand await stays in the linearizer's directly-handled position).

The generator yield hoist (generator/hoist_yields.rs) has the same latent gap for yield-in-sequence; noted in #5925 as a follow-up rather than piggybacking untested changes here.

Validation

Fixes #5925.

https://claude.ai/code/session_01K1w6WLaFNKhf4aNxeSWHy6

Summary by CodeRabbit

  • Bug Fixes

    • Fixed await handling in comma-separated expressions so evaluation order is preserved correctly in more cases.
    • Resolved an issue where earlier parts of a sequence could be skipped or read in the wrong order around await.
  • Tests

    • Added regression coverage for sequence-and-await ordering in assignment, let initialization, and return scenarios.
    • Verified expected output order for async evaluation paths.

… awaits

The plain-async pre-pass hoists every nested await into a
`let __await_N = await <operand>;` above the containing statement. For a
comma sequence that moved the awaited operand's evaluation above the
sequence's earlier operands: `this.l = new L, this.p = await this.l.start()`
evaluated `this.l.start()` while `this.l` still held its pre-assignment
value (null) — a TypeError under perry where node prints the result.
Minifiers comma-fold consecutive statements, so any
`const x = new X(); const y = await x.m();` pair in bundled async code hits
this. Conditional branches (#342) and logical RHS (#5434) already have
dedicated lifts for exactly this class of unsound hoist; Sequence had none.

Fix: `lift_sequence_with_await` — when a sequence contains an await, lift
the non-final operands to statements (each recursively hoisted, preserving
evaluation order) and continue with the final operand as the expression
value. Wired into both `hoist_awaits_in_expr_full` (nested sequences) and
`hoist_awaits_avoiding_top_level` (statement-position sequences, re-processed
so a final-operand await stays directly handled). The generator yield hoist
(generator/hoist_yields.rs) has the same latent gap for yield-in-sequence;
left as a follow-up per #5925.

Fixes #5925.

Claude-Session: https://claude.ai/code/session_01K1w6WLaFNKhf4aNxeSWHy6
@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 27995425-4653-4ec9-8ee1-c09fa5faddd7

📥 Commits

Reviewing files that changed from the base of the PR and between 42e2401 and 796ffbb.

📒 Files selected for processing (2)
  • crates/perry-transform/src/async_to_generator.rs
  • crates/perry/tests/issue_5925_seq_await_order.rs

📝 Walkthrough

Walkthrough

The async-to-generator pre-pass now special-cases Expr::Sequence expressions containing await, lifting non-final comma operands into separate statements before hoisting the awaited final operand, preserving JavaScript comma evaluation order. New regression tests validate this fix for issue #5925.

Changes

Sequence-aware await hoisting fix

Layer / File(s) Summary
Sequence-aware hoisting logic
crates/perry-transform/src/async_to_generator.rs
hoist_awaits_in_expr_full and hoist_awaits_avoiding_top_level detect await-containing comma sequences and lift non-final operands into statements first; a new lift_sequence_with_await helper performs the lift, hoisting awaits inside each lifted operand before leaving the final operand in place.
Regression tests for issue #5925
crates/perry/tests/issue_5925_seq_await_order.rs
New tests compile and run TypeScript snippets covering assignment-then-await via comma sequences, and awaits in earlier operands within let-init and return positions, asserting correct evaluation order and values.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Source as "Source Sequence Expr"
  participant Hoister as "hoist_awaits_avoiding_top_level"
  participant Lifter as "lift_sequence_with_await"
  participant Stmts as "Lifted Statements"

  Source->>Hoister: sequence expr containing await
  Hoister->>Lifter: detect await in sequence, delegate lifting
  Lifter->>Stmts: push non-final operands as statements (hoisted in order)
  Lifter->>Hoister: return final operand in place
  Hoister->>Hoister: hoist await from final operand as __await_N
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main fix to async transform sequence ordering.
Description check ✅ Passed The description covers the problem, fix, linked issue, and validation, though it doesn't follow the template headings exactly.
Linked Issues check ✅ Passed The code and tests address #5925 by hoisting non-final sequence operands before awaited final operands in both handled paths.
Out of Scope Changes check ✅ Passed The changes stay focused on the async sequence-hoisting bug and its regression tests, with no unrelated scope added.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/async-seq-await-order

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug proggeramlug merged commit 6590841 into main Jul 4, 2026
16 checks passed
@proggeramlug proggeramlug deleted the fix/async-seq-await-order branch July 4, 2026 00:50
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.

async transform: await hoisted above earlier comma-sequence operands — awaited receiver reads its pre-assignment value

1 participant