fix: keep the GlobalBus emit override assignable to EventEmitter

Draft
SA
2mo ago
+72-3

Issue for this PR

Closes #1165

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

GlobalBusEmitter overrode emit with a single narrow signature:

override emit(eventName: "event", event: GlobalEvent): boolean

EventEmitter<T> declares emit across several overloads, one of them (eventName: string | symbol, ...args: any[]). An override must be assignable to all of them, and that narrow one is not. Newer @types/node rejects it with Type 'any[]' is not assignable to type '[event: GlobalEvent]', which fails bun typecheck — and therefore blocks git push through the pre-push hook, on a file the contributor never touched.

The failure is indistinguishable from a real type error the contributor introduced, so the workaround people reach for is git push --no-verify, which disables the only pre-push gate the repo has.

This keeps the public overload so call sites stay typed exactly as before (26 of them across the repo), and widens only the implementation signature, which is the part that has to satisfy the base. Runtime behaviour is unchanged: an "event" payload without an id still gets one, still preferring syncEvent.id.

How did you verify your code works?

The regression is a compile error, not a runtime one, so the test pins it with an assignment that only compiles while the wide signature stays:

const wide: (eventName: string | symbol, ...args: any[]) => boolean = GlobalBus.emit.bind(GlobalBus)

Reverting global.ts to the previous override and running tsgo --noEmit fails with the same assignability error the reports show:

test/bus/global-emit.test.ts(15,11): error TS2322:
  Type '(eventName: "event", event: GlobalEvent) => boolean' is not assignable to
  type '(eventName: string | symbol, ...args: any[]) => boolean'.

With the fix: 0 errors. So tsgo fails if anyone narrows it again.

Also:

  • bun typecheck — 13 tasks successful.
  • bun test test/bus test/server — 289 pass, 0 fail.
  • Three runtime tests covering id stamping, an id already present, and the syncEvent.id preference.
  • Marker Guard passes.

One caveat, stated plainly: I could not reproduce the original failure locally. On a clean worktree at origin/main with the catalog-pinned @types/node@24.12.2, typescript@7.29.7 and @typescript/native-preview@7.0.0-dev.20251207.1, global.ts typechecks clean — which also matches main's CI, where the TypeScript job passes. What is demonstrated above is that the old override genuinely is not assignable to the base signature and the new one is, which is the root cause the reported error names. Whether a toolchain version skew is also in play is tracked separately in #1165 — this PR does not address that half.

Screenshots / recordings

Not a UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

🤖 Generated with Claude Code

https://claude.ai/code/session_018fJ3X7pcGT4R9yzjsJnqsV

Summary by cubic

Fixes GlobalBusEmitter.emit so it stays assignable to the base EventEmitter overload set, which unblocks bun typecheck and git push on untouched code.

Previously, the override had a single narrow signature that newer @types/node rejects. Now the implementation signature is widened while the public overload is preserved, so call sites keep their exact types. Runtime behavior is unchanged. Adds a compile-time test that fails if the wide signature is narrowed again.

Closes #1165.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed compatibility with standard event emitter usage and type checking.
    • Preserved existing event behavior, including automatic identifier assignment and synchronization identifier preference.

  • Tests

    • Added coverage for event emitter compatibility, identifier assignment, existing identifier preservation, and synchronized event identifiers.