Phaser `EventEmitter` typings are unsatisfactory and unsafe
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 40.3k
- Forks
- 7.2k
- PR merge metrics
- No merged PRs in 30d
Description
Version
- Phaser Version: 3.90.0
- Operating system: Ubuntu 24.04 LTS (platform agnostic issue)
Description
At the current moment, the types of Phaser.Events.EventEmitter and company look like this:
This (especially the use of Function and any-typed callback scope) presents several pitfalls when working with the project in TypeScript:
- No autocomplete or type safety for what event names are accepted by which emitter
- This is only exacerbated by the properties of
Phaser.Eventsall being typed asstring, despite having canonical string names in the example code!
- This is only exacerbated by the properties of
- No inherent type safety or argument inference for events on callback types, requiring developers to flip back and forth between the API pages for trivial tasks
Example Test Code
import Phaser from "phaser";
const game = new Phaser.Game({type: Phaser.HEADLESS});
game.events.on("b00t", (arg1) => initializeGame(arg1)) // Oops! meant to put "boot"; this never runs
game.events.on("ready", (foo, bar) => {}) // no inference of the callback type
Additional Information
While this would be a fairly large change, it would be fairly routine to convert this code into being type safe.
- Add any necessary
@constannotations to the types of allPhaser.Events.XYZvariables to ensure TypeScript emits them as their actual values instead ofstringorsymbol. - Convert
EventEmitterinto a genericEventEmitter<Events extends PhaserEventMap = PhaserEventMapclass via JSDoc's@templatetag. This ensures backwards compatibility for existing code that extended the generic class before it got a type parameter. (Moreover, code that wants to expand the allowed types of a given event map can extend said event map with their own constants.) - Create unique "event map" interfaces for each instance of
EventEmitterthat is relied upon by currently functioning code. It is very important that these are interfaces, not types for the interface merging reasons highlighted above. - Provide said event maps at callsites to specify what events each emitter produces.
Example snippet of what the .d.ts file could look like:
import type Phaser from "phaser";
type PhaserEventMap = {
[key: string | symbol]: Function;
}
declare class EventEmitter<T extends PhaserEventMap = PhaserEventMap> {
emit<E extends keyof T>(evt: E, callback: T[E], ...scope: ThisParameterType<T[E]> extends unknown ? [] : [ThisParameterType<T[E]>]): void
}
interface MyEventMap {
'complete': (s: Phaser.Scene) => void;
'completeWithScope': (this: 3, s: Phaser.Scene) => void;
}
// NB: We will have to use a type helper to convert from interfaces to types, as TS considers interfaces "closed"
// and requires explicit index signature declarations on anythign that implements EventMap when we really just want to check assignability
type Simplify<T> = { [KeyType in keyof T]: T[KeyType]; }
declare const e: EventEmitter<Simplify<MyEventMap>>;
e.emit("complete", (s) => s.add.arc(1, 2)); // callback has specified type of (Phaser.Scene) => void
Notably, the types accepted are already inside the code - we'd just have to convert the @param descriptions into some sort of map.
[!IMPORTANT]
I understand this would be a large change for consumers of existing packages, but this would only actually break those users using TypeScript who were previously passing incorrect callbacks. As such, I see no harm in tightening the compile-time checks to match the runtime ones.
I am willing to work on this myself, provided I have the time and get a go-ahead from someone about the approach.
Blocked by #7248
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing the Phaser.Events.EventEmitter typings and the event-emitter call sites described in the issue, then inspect blocked issue #7248. Map existing event names and callback parameters before deciding the event-map boundaries. Done means the relevant emitters expose checked event names and inferred callback arguments without losing existing compatibility.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, typescript
- Domain
- developer-experience
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100