feat: allow yield in fat-arrow functions (#2140)#2143
Conversation
Transforms `=> yield X` into `(function*() { return yield X }).bind(this)`,
preserving the lexical `this` semantics of the arrow.
When the body contains no `this` references, the `.bind(this)` wrapper is
omitted. Works with `async`, `yield*`, parameters, and multi-statement
arrow bodies.
This implements the long-requested generator-arrow feature referenced in
TC39 — CoffeeScript 1 supported the same shape via `=> yield @`.
Co-authored-by: Daniel X Moore <STRd6@users.noreply.github.com>
|
@claude make code comments much more succinct |
|
Claude finished @STRd6's task in 5m 30s —— View job Making code comments more succinct
SummaryTrimmed the verbose comments around
Net: −30 lines comment, +3 lines comment. |
Co-authored-by: Daniel X Moore <STRd6@users.noreply.github.com>
Greptile SummaryThis PR implements yield support inside Civet
Confidence Score: 3/5Safe for direct this references but produces silently wrong runtime this binding when a nested arrow inside the generator body is the only place @x appears. The transformation correctly handles the common case where the outer arrow body directly references @x or this. However, because gatherRecursiveWithinFunction halts at ArrowFunction boundaries, any @x that lives only inside a nested arrow within the outer generator body is invisible to the this-detection logic. In that pattern the outer generator is emitted without .bind(this), so the nested arrow inherits the wrong this at runtime. source/parser/function.civet — specifically the gatherRecursiveWithinFunction call in transformArrowFunctionWithYield; test/yield.civet needs a corresponding test for the nested-arrow this case. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[processSignature called on function node] --> B{f.generator?# AND hasYield?}
B -- No --> Z[Done]
B -- Yes --> C{f.generator? slot exists?}
C -- Yes --> D[addGenerator = true]
C -- No --> E{f.type is ArrowFunction?}
E -- Yes --> F[transformArrowFunctionWithYield]
E -- No --> G[Inject Error node into yield]
F --> H{block.implicitlyReturned?}
H -- Yes --> I[insertReturn block]
H -- No --> J{block.bare?}
I --> J
J -- Yes --> K[Add braces to block]
J -- No --> L[Build function* children]
K --> L
L --> M[f.type = FunctionExpression, mark generator]
M --> N[makeLeftHandSideExpression f]
N --> O{this token in block via gatherRecursiveWithinFunction?}
O -- Yes --> P[Wrap: lhs.bind this]
O -- No --> Q[lhs only]
P --> R{newNode !== f AND originalParent?}
Q --> R
R -- Yes --> S[replaceNode + updateParentPointers]
R -- No --> Z
S --> Z
|
| describe "yield in arrow function (#2140)", -> | ||
| testCase """ | ||
| simple thick arrow with yield | ||
| --- | ||
| => yield 5 | ||
| --- | ||
| (function*() {return yield 5}) | ||
| """ | ||
|
|
||
| testCase """ | ||
| thick arrow with yield and this | ||
| --- | ||
| => yield @ | ||
| --- | ||
| (function*() {return yield this}).bind(this) | ||
| """ | ||
|
|
||
| testCase """ | ||
| thick arrow with yield and @-property | ||
| --- | ||
| => yield @x | ||
| --- | ||
| (function*() {return yield this.x}).bind(this) | ||
| """ | ||
|
|
||
| testCase """ | ||
| thick arrow with parameter and yield | ||
| --- | ||
| (x) => yield x | ||
| --- | ||
| (function*(x) {return yield x}) | ||
| """ | ||
|
|
||
| testCase """ | ||
| thick arrow with multiple yields | ||
| --- | ||
| => | ||
| yield 1 | ||
| yield 2 | ||
| --- | ||
| (function*() { | ||
| yield 1 | ||
| yield 2 | ||
| }) | ||
| """ | ||
|
|
||
| testCase """ | ||
| assigned thick arrow generator | ||
| --- | ||
| gen := => yield @x | ||
| --- | ||
| const gen =(function*() {return yield this.x}).bind(this) | ||
| """ | ||
|
|
||
| testCase """ | ||
| async thick arrow with yield becomes async generator | ||
| --- | ||
| async => yield 42 | ||
| --- | ||
| (async function*() {return yield 42}) | ||
| """ | ||
|
|
||
| testCase """ | ||
| yield* in thick arrow | ||
| --- | ||
| => yield* arr | ||
| --- | ||
| (function*() {return yield* arr}) | ||
| """ | ||
|
|
||
| testCase """ | ||
| yield in arrow inside class method preserves this | ||
| --- | ||
| class Foo | ||
| method() | ||
| gen := => yield @x | ||
| --- | ||
| class Foo { | ||
| method() { | ||
| const gen =(function*() {return yield this.x}).bind(this);return gen | ||
| } | ||
| } | ||
| """ |
There was a problem hiding this comment.
Missing test: indirect
this via nested arrow inside generator
The suite covers direct @x in the outer body, but not the case where @x lives exclusively inside a nested arrow within the outer => generator (e.g. => (inner := => @x; yield 5)). That pattern is the one affected by the gatherRecursiveWithinFunction boundary issue noted in function.civet, and adding a failing test here would lock in the expected .bind(this) behavior once the fix lands.
| --- | ||
| => yield 5 | ||
| --- | ||
| (function*() {return yield 5}) |
There was a problem hiding this comment.
Double space in
return yield output
The expected output (function*() {return yield 5}) contains two spaces between return and yield. This appears in every single-expression arrow-generator test. It comes from insertReturn appending "return " (with trailing space) when the existing yield token already carries a leading space. The generated JS is still valid, but the redundant whitespace is unexpected and would look odd in source maps or formatted output. It is worth tracking down where the extra space originates and trimming it.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2143 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 57 57
Lines 27649 27691 +42
Branches 4590 4599 +9
=========================================
+ Hits 27649 27691 +42
🚀 New features to boost your workflow:
|
|
@claude ensure typecheck passes |
|
Claude finished @STRd6's task in 10m 56s —— View job Ensuring typecheck passes
SummaryThe PR introduced 2 typecheck regressions in
- originalParent := f.parent as ASTNodeObject?
+ originalParent := f.parent as ASTNodeParent?A parent always has children, so Verification
|
The originalParent cast was ASTNodeObject?, but replaceNode and updateParentPointers expect ASTNodeParent? (which adds the children constraint). A parent node always has children, so ASTNodeParent is the right type. Co-authored-by: Daniel X Moore <STRd6@users.noreply.github.com>
|
Awesome, thanks! |
Closes #2140.
Summary
Allows
yieldinside=>arrow functions by compiling to a generator function expression bound to the enclosingthis:When the arrow body doesn't reference
this, the.bind(this)wrapper is omitted.Implements the feature @edemaine sketched in #2140, matching CoffeeScript 1's
=> yield @behavior.Test plan
test/yield.civetcover simple, @-property, parameters, multi-yield, assignment, async, yield*, and class-method cases🤖 Generated with Claude Code