Skip to content

Feat: foundation for TypeScript migration#7308

Draft
niklas-e wants to merge 17 commits into
phaserjs:masterfrom
niklas-e:feat/typescript-migration-foundation
Draft

Feat: foundation for TypeScript migration#7308
niklas-e wants to merge 17 commits into
phaserjs:masterfrom
niklas-e:feat/typescript-migration-foundation

Conversation

@niklas-e
Copy link
Copy Markdown

Disclaimer: this contains the eslint config fix commit from #7304, to enable proper lint configuration for TS

Introduction

This PR starts an incremental JavaScript to TypeScript migration path for Phaser. I acknowledge this will be an epic journey if we go on this path.

The motivation is related to #7298: the current declaration pipeline still depends heavily on jsdoc, which has known parser limitations and an uncertain maintenance future. Rather than proposing an all-at-once rewrite, this branch shows that Phaser can migrate individual modules to native TypeScript while keeping the existing source tree, build output, and generated types/phaser.d.ts workflow intact.

The migrated modules are intentionally small but varied, covering math helpers, a class, mixins, a struct, geometry, and a game object helper. They are now discovered automatically from migrated TypeScript source via JSDoc namespace tags, and the type generation step overlays authoritative TypeScript declarations for those migrated symbols into the existing Phaser declaration file.

Pros

  • Allows gradual migration from JS to TS without requiring the whole codebase to move at once
  • Keeps existing JavaScript modules working alongside migrated TypeScript modules
  • Uses tsc as the source of truth for migrated declarations, avoiding some jsdoc parser limitations
  • Gives migrated code real compiler checking instead of relying only on JSDoc annotations
  • Preserves the existing public types/phaser.d.ts output shape for consumers
  • Adds validation around migrated symbols so missing or incorrect generated declarations are caught
  • Provides a practical foundation for continuing module-by-module migration
  • When whole migration is completed, .d.ts maps can be published which enables navigation to source definition

Cons / Trade-offs

  • The type generation pipeline becomes more complex while Phaser is in a hybrid JS/TS state
  • Contributors need to understand both existing JSDoc-based type generation and the new TypeScript overlay path

Technicalities

How distribution artifacts are stitched together

The published phaser.d.ts remains a single output file, but during the migration period it is assembled from tsgen output and an overlay of declarations emitted by tsc for migrated modules.

flowchart TD
    jsFiles["JSDoc (.js files)"] --> jsdoc["jsdoc parser"] --> dtsDom["dts-dom tree"]
    tsFiles["TypeScript (.ts files)"] --> discovery["auto-discovery (JSDoc tags)"] --> tsc["tsc --emitDeclarationOnly"]

    dtsDom --> overlay["MigratedOverlay<br/>(synthetic stubs + overlay)"]
    tsc --> overlay

    overlay --> publish["publish.ts<br/>(orchestrator)"]
    publish --> phaser["phaser.d.ts"]
Loading

Phases of the migration

  1. Start with the tsgen + overlay strategy shown above
  2. Continue using npm run ts to update and validate the type declarations during the migration period. Once migrated modules reach 50% of source modules, it will also start warning that it may be time to consider a TS-first declaration pipeline. No concrete plan yet, but at that point most of phaser.d.ts will be generated by tsc and we should patch JSDoc definitions for the remaining JavaScript modules on top of that
  3. Once the codebase is fully migrated to TypeScript, tsgen can be removed as declarations will be emitted by tsc

Agent skill for migrating modules

I wrote and tested an agent skill to help with the migration. This should be a great aid for contributors. You can find it here: https://github.com/niklas-e/phaser-typescript

TODO

  • Remove dist artifacts before merging (included for easier testing within actual projects)
  • Setup an umbrella issue and/or GH project to track the progress
  • Ensure docs.phaser.io works as expected - did not find the sources for it so couldn't see how it is set up
  • Write a contribution guide

Comment thread src/structs/Map.ts
Comment on lines +25 to +29
* var map = new Map([
* [ 1, 'one' ],
* [ 2, 'two' ],
* [ 3, 'three' ]
* ]);
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSDoc version allowed numbers for keys, but it is confusing while using an object for storage internally. In JavaScript this will be true obj[1] === obj["1"], thus causing potential runtime bugs. Therefore I set the signature to Map<K extends string = string, V = unknown>.

Comment thread src/structs/Map.ts
*
* @returns The callback result.
*/
type EachMapCallback<K extends string, V> = (key: K, entry: V) => boolean | void;
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSDoc declaration said this callback returns null when in reality it returns entries[key], not null. The type was misleading.

Comment thread tsconfig.json
{
"compilerOptions": {
"strict": true,
"target": "ES2018",
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be a comfortable target without dropping too much compatibility? I understand there's probably varying views, but my point of view is that as a framework Phaser could ship builds with quite modern versions and people who want to support really old stuff can use existing tools in the ecosystem to convert to their preferred ES version. --- Also good to note we should definitely set the target to at least ES2015 to not have the distributables include bunch of utility code for e.g. classes

Comment thread src/utils/Mixin.ts
Comment on lines +107 to +119
export function composeMixins<const TMixins extends readonly Mixin<object>[]> (
...mixins: TMixins
): <TBase extends Constructor>(
Base: TBase
) => TBase & Constructor<InstanceType<TBase> & AddedByAll<TMixins>>
{
// The reduce chain is type-safe at call sites via the overload signature;
// the implementation needs a single cast to bridge the generic gap.
return ((Base: Constructor) =>
mixins.reduce((Current, mixin) => mixin(Current), Base)
// eslint-disable-next-line @typescript-eslint/no-explicit-any
) as any;
}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's multiple ways to implement mixins, but this composable implementation keeps the footprint same size as before.

For comparison the worst case scenario with TS handbook mixins in current codebase would be:

// Note: creates many anonymous classes
const SpriteBase =
  Alpha(
  BlendMode(
  Depth(
  Flip(
  GetBounds(
  Lighting(
  Mask(
  Origin(
  RenderNodes(
  ScrollFactor(
  Size(
  TextureCrop(
  Tint(
  Transform(
  Visible(
    GameObject
  )))))))))))))));

export class Sprite extends SpriteBase { ... }

But with the composable approach it would be:

// Note: creates only single class
const SpriteBase = composeMixins(
  Alpha,
  BlendMode,
  Depth,
  Flip,
  GetBounds,
  Lighting,
  Mask,
  Origin,
  RenderNodes,
  ScrollFactor,
  Size,
  TextureCrop,
  Tint,
  Transform,
  Visible)(GameObject);

export class Sprite extends SpriteBase { ... }

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.

1 participant