Skip to content

fix(runEffectInsideEffect): report Effect.runSyncExit and Effect.runPromiseExit - #876

Merged
mattiamanzati merged 2 commits into
mainfrom
fix/run-effect-inside-effect-exit
Oct 19, 2026
Merged

mattiamanzati merged 2 commits into
mainfrom
fix/run-effect-inside-effect-exit

Conversation

@mattiamanzati

Copy link
Copy Markdown
Contributor

Why

Effect.runSyncExit and Effect.runPromiseExit start a separate fiber, just like Effect.runSync and Effect.runPromise. runEffectInsideEffect matched only four runner names, so the Exit forms were never reported. This ports the matching fix from Effect-TS/language-service#781.

export const nested = Effect.gen(function*() {
  yield* Effect.void
  setTimeout(() => {
    Effect.runPromiseExit(child)
    // ^- `Effect.runPromiseExit` is called inside an Effect with a separate services invocation.
    //    In this context, child Effects run with the surrounding services, which can be accessed
    //    through `Effect.context` and `Effect.runPromiseExitWith`.
  }, 0)
})

The quick fix rewrites the call to Effect.runPromiseExitWith(effectContext)(child) in v4 and to Runtime.runPromiseExit(effectRuntime, child) in v3.

What changed

  • runEffectApis now includes runSyncExit and runPromiseExit.
  • New fixtures, taken from the upstream PR, cover the Exit runners in v3 and v4, and reuse of an existing context binding in v4.

Scope

  • The other half of the upstream PR, which made the v4 quick fix use Effect.context instead of Effect.services, was already in tsgo. The new existing-context fixture confirms it.
  • Left out: the quick-fix test runner ignores its Effect version and mounts v3 only when a file contains // @effect-v3. 59 of 223 v3 fixtures lack that line, so their quick-fix baselines probably run against v4. The new v3 fixture has the line.

Verification

  • The first commit adds the fixtures with baselines that show the Exit calls unreported. The second commit adds the fix, and the six calls per Effect version now match the upstream snapshots.
  • pnpm lint, pnpm check, and pnpm test pass.

Fixes #873

🤖 Generated with Claude Code

mattiamanzati and others added 2 commits October 19, 2026 11:10
…mples

Ports runEffectInsideEffect_exit.ts (v3, v4) and
runEffectInsideEffect_existingContext.ts (v4) from
Effect-TS/language-service#781. The baselines record current behavior:
Effect.runSyncExit and Effect.runPromiseExit are not reported.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…romiseExit

The Exit runners start a separate fiber just like runSync and
runPromise, but the rule only matched four runner names. Port of
Effect-TS/language-service#781.

Fixes #873

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Effect perf vs stock tsgo: no significant change

Details
MetricbaseprDeltaBestWorstp-value
latest · 7.0.2 · effect (all rules)
Errors11~11counter, n=5
Warnings00~00counter, n=5
Symbols over stock+712,112 (±2)+712,113 (±3)~+712,107+712,113counter, n=5
Types over stock+340,886 (±2)+340,886 (±4)~+340,878+340,886counter, n=5
Instantiations over stock+1,208,648+1,208,648 (±11)~+1,208,626+1,208,648counter, n=5
Memory allocs over stock+17.9% (17.8 to 18.0)+17.8% (17.8 to 17.9)~+17.8%+17.9%p=0.421 n=5
Check time over stock+33.5% (29.1 to 34.6)+31.3% (30.6 to 36.0)~+30.6%+36.0%p=0.548 n=5
Total time over stock+22.6% (18.2 to 25.7)+22.6% (19.6 to 24.6)~+19.6%+24.6%p=1.000 n=5
next · 7.1.0-dev.20261018.1 · effect (all rules)
Errors1818~1818counter, n=5
Warnings00~00counter, n=5
Symbols over stock+564,076 (±3)+564,076~+564,076+564,076counter, n=5
Types over stock+299,169 (±3)+299,169 (±1)~+299,169+299,171counter, n=5
Instantiations over stock+1,001,071 (±10)+1,001,071 (±2)~+1,001,071+1,001,074counter, n=5
Memory allocs over stock+15.7% (15.1 to 16.6)+15.4% (15.1 to 15.5)~+15.1%+15.5%p=0.421 n=5
Check time over stock+37.5% (27.8 to 41.4)+33.9% (32.1 to 41.7)~+32.1%+41.7%p=0.690 n=5
Total time over stock+28.8% (23.5 to 29.2)+27.2% (26.5 to 29.5)~+26.5%+29.5%p=0.841 n=5
  • Stock counters, latest: 7.0.2 symbols 18,255,289 (±1) · types 6,184,373 · instantiations 26,135,429
  • Stock counters, next: 7.1.0-dev.20261018.1 symbols 17,711,128 · types 5,960,725 · instantiations 25,181,429
  • Workload: https://github.com/Effect-TS/effect@7f524e25cfb49a71ac39b0f44278d08d40dda5de with every Effect rule executing (skipDisabledOptimization: true)
  • Pairs per side: 5, alternating stock-first and patched-first
  • Commits: base de42db08b3266c0c61cf78cdf501840760714a1e · head 69ecd8d1cf5ade5a916456e0225c95625c060fd9
  • Counters are patched − stock per pair (median ±half-range); allocs and times are patched / stock − 1 per pair, median (min to max). Time and alloc deltas are flagged only when the exact two-sided Mann-Whitney U p < 0.01.

@mattiamanzati
mattiamanzati merged commit c83ee4d into main Oct 19, 2026
14 checks passed
@mattiamanzati
mattiamanzati deleted the fix/run-effect-inside-effect-exit branch October 19, 2026 10:35
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.

runEffectInsideEffect: Effect.runPromiseExit and Effect.runSyncExit are still not reported (Effect-TS/language-service#781 is not in tsgo)

1 participant