phaserjs / phaserjs/phaser

`Phaser.Structs.Map` has unsatisfactory and incorrect typing

Open
#7,323 0 comments 0 reactions 0 assignees View on GitHub

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: 4.21.0
  • Operating system: N/A
  • Browser: N/A

Description

This is a conglomerate of typing issues related to the Map struct:

  1. The type of key should be constrained to string | number - attempting to index the map with anything else will result in potentially unsafe string coercion. (I have yet to see an example where such coercion is even remotely useful.)
  2. setAll should not have extra generic type parameters - the separate K, V types used shadow the main Map's types and allow assigning completely incompatible and unexpected values. (It doesn't even enforce that the values being passed are themselves arrays or array-like objects, despite that very function crashing if said invariant is violated.)
    • Even though jsdoc doesn't support emitting tuples in declarations (rendering the "correct" type of [K, V][] unusable for now), a nested array type like (K|V)[][] would still be better than one which effectively disables type safety altogether.
  3. get doesn't include undefined in its type signature, despite mentioning as such in its JSDoc comment. This is ripe territory for game crashes and undefined property accesses.
  4. The JSDoc claims that the class constructor's elements parameter is optional, but the constructor states it as required to be passed.

Example Test Code

Indexing maps with an object leads to undesireable behaviour, despite being A-OK as far as types are concerned
Image

import Phaser from "phaser";

// Error - constructor needs empty array despite being "optional"
const myMap = new Phaser.Structs.Map<string, number>();

// should produce type error but doesn't, despite these not even being tuples
myMap.setAll([() => 1, "apples"]);

const elem = myMap.get("11");
// Oops! Elem could be undefined and you might get a game crash!
console.log(elem.toFixed(2));

Additional Information

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Search the Phaser.Structs.Map implementation and its JSDoc or generated type declarations. Compare the constructor, key, setAll, and get signatures with the examples in the issue, then run the relevant type-checking or test commands available in the repository. Done means the declarations reject invalid keys and values, include undefined for missing get results, and match the constructor documentation.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, typescript
Domain
developer-experience, game-dev
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.