fix(beacon-node): restructure unknownBlock test to fix tsgo overload resolution - #9488
Merged
nflaig merged 1 commit intoJun 9, 2026
Conversation
…resolution The test "downloads the block and retries payload import when EL reports block not in fork choice" used a `let emitter!: ChainEventEmitter` definite-assignment assertion + destructuring-assignment to satisfy a closure dependency where `processBlock`'s mock impl referenced `emitter` before `setupPayloadSyncTest` returned. tsgo's overload resolution then widened the variable's type after the destructuring assignment and failed to pick the StrictEventEmitter overload for the subsequent `emitter.emit(ChainEvent.unknownEnvelopeBlockRoot, ...)` call, falling back to the EventEmitter base overload whose first arg expects `unique symbol`: test/unit/sync/unknownBlock.test.ts(1027,20): error TS2769: No overload matches this call. Argument of type 'ChainEvent.unknownEnvelopeBlockRoot' is not assignable to parameter of type 'unique symbol'. Restructure to: declare `processBlock = vi.fn()` without impl, destructure `emitter` as a `const`, then attach the mock implementation after `setupPayloadSyncTest` returns. This matches the pattern used by the 5 other passing emit sites in the same file. Safe because no event is emitted between `setupPayloadSyncTest` and the subsequent emit, so `processBlock` is not invoked before `mockImplementation` is set. 🤖 Generated with AI assistance
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors a unit test in unknownBlock.test.ts to improve readability and avoid definite assignment assertions. Specifically, it separates the initialization of processBlock and emitter by mocking the implementation of processBlock after emitter has been destructured from setupPayloadSyncTest. There are no review comments provided, and I have no additional feedback to offer.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
nflaig
merged commit Jun 9, 2026
fda72f7
into
ChainSafe:te/fix_block_input_sync_metrics_logs
1 check passed
Contributor
Author
|
Thanks for the review pass — appreciated. (Noted on the consumer-version sunset.) |
This was referenced Jun 9, 2026
nflaig
pushed a commit
that referenced
this pull request
Jun 9, 2026
…verload (#9491) ## Summary Follow-up to #9488 — the restructure removed the `let !` + destructuring pattern but tsgo's overload-resolution miss persists at the same emit site because `emitter` is also captured by a sibling closure (the `processBlock` mock body that emits `routes.events.EventType.block`). Same `TS2769` error reproduces on the current head: ``` test/unit/sync/unknownBlock.test.ts(1027,20): error TS2769: No overload matches this call. Argument of type 'ChainEvent.unknownEnvelopeBlockRoot' is not assignable to parameter of type 'unique symbol'. ``` ## Fix Cast `emitter` at the failing site to `ChainEventEmitter` to re-anchor the `StrictEventEmitter` overload for `ChainEvent.X` keys. Minimal change — single line cast at line 1023, with a comment explaining why. The 5 other `emitter.emit(ChainEvent.unknownEnvelopeBlockRoot, ...)` sites in the file remain untouched because they aren't reached through a sibling closure capture. The previous restructure (now in the base branch) is still worth keeping — it removed an unnecessary indirection — but it wasn't sufficient on its own. ## Test plan - [ ] CI: `Type Checks (24)` passes (no TS2769 on `unknownBlock.test.ts`). - [ ] CI: unit tests for the "downloads the block and retries payload import when EL reports block not in fork choice" test still pass — emit semantics unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: lodekeeper <lodekeeper@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
unknownBlock.test.tsso itsprocessBlockmock no longer needs alet emitter!: ChainEventEmitterdefinite-assignment assertion plus a destructuring-assignment dance.emitter's inferred type and fall back to theEventEmitterbase overload (expectingunique symbolevent names) for the subsequentemitter.emit(ChainEvent.unknownEnvelopeBlockRoot, ...)site — surfaced as theTS2769check-types failure on this PR's CI.Diagnosis
Failing job:
Type Checks (24)→packages/beacon-node check-types.The data shape is fine —
slot: 0is already included at line 1024, and the e2e atunknownBlockSync.test.ts:236also includesslot: headSlot. The 5 otheremitter.emit(ChainEvent.unknownEnvelopeBlockRoot, ...)sites in the same file (lines 762/836/926/1079/1129) all typecheck fine because they use plainconst {emitter} = setupPayloadSyncTest(...)rather thanlet !+ destructuring-assignment.Fix
Declare
processBlock = vi.fn()upfront, destructureemitteras aconst, then attachprocessBlock.mockImplementation(...)aftersetupPayloadSyncTestreturns. This matches the pattern used by every other test in the file and removes the pattern that confused tsgo.Safe because no event fires between
setupPayloadSyncTestand the nextemitter.emit(...)—processBlockis never invoked before its implementation is attached.Test plan
Type Checks (24)passes (no TS2769 onunknownBlock.test.ts).processBlockcalled once,processExecutionPayloadcalled twice).🤖 Generated with Claude Code