Proposal: Use type declarations over @typedef

[kpal81xd]

Problem

The engine uses JSDoc @typedef {object} Type notation to declare types. This works for using them and will show in typedoc but they are not exported in playcanvas.d.ts unlike if we used export type Type ... like in typescript

Proposal

JS supports import external types files e.g. If you have a file called types.d.ts with a type like this:

export type Type {
   name: string;
}

you can import it into JS using the @import keyword just like you can with @typedefs from JS files

/** @import { Type } from './type' */

The idea would be to have a types.d.ts file in subfolders - similar to the way we have constants.js to keep the type files organised and scopes to particular areas.

Pros

• Can explicitly exported in the NPM package (and can control visibility in API reference since typedoc respects export keyword)
• Typescript types declarations will behave the same way for development (including examples since type files will be ignored since examples uses the bundled playcanvas.d.ts)
• Typescript types are bundled in with no additional configuration
• Type fixing StandardMaterial and ScriptType can be removed as extra method types can be declared directly and bundled in without any custom plugins
• Callback declarations can be converted into type declarations
• Constants for enums can now be converted directly into types reducing code size

Cons

• Mixture of .d.ts and .js files in the codebase
• Will take some time to convert @typedefs to type declarations (can be mostly automated however)

[Maksims]

Major question: what user problem (real commercial problem) does it solve and what new capabilities for users it enables?

There are only a very few typedef instances in the codebase (/src/). Would love to avoid TS issues, where simple code like so:

return data.bodyTypes[bodyType]?.[slot]?.list?.length > 1;

Becomes an ugly monstrosity like so:

return data.bodyTypes[bodyType as keyof typeof data.bodyTypes]?.[slot as CataloguePartsKeys]?.list?.length > 1;

[kpal81xd]

Major question: what user problem (real commercial problem) does it solve and what new capabilities for users it enables?

There are only a very few typedef instances in the codebase (/src/). Would love to avoid TS issues, where simple code like so:

return data.bodyTypes[bodyType]?.[slot]?.list?.length > 1; Becomes an ugly monstrosity like so:

return data.bodyTypes[bodyType as keyof typeof data.bodyTypes]?.[slot as CataloguePartsKeys]?.list?.length > 1;

This has no affect on JS notation - just type declarations. That notation you are using is an example of poor typing regardless. Good typing minimises casting and explicit type definitions as the typescript compiler will correctly infer values as you use them.

Exporting correct types gives you better type information for usage and manipulation:

import type { ShaderDesc } from 'playcanvas';

const shaders: ShaderDesc[] = [..., ...];
const materials = shaders.map((desc) => new ShaderMaterial(desc));

Additionally if constants are migrated to types you wouldnt need to use values such as pc.FILLMODE_FILL_WINDOW as the type of setCanvasFillMode will tell you which modes are supported by the union of string literals:

Better type information allows the engine API to be explored directly through its use for faster development and safer more reliable code

[Maksims]

Better type information allows the engine API to be explored directly through its use for faster development and safer more reliable code

That line is not a great example, as it clearly does not communicate simply what arguments there are. It is way way too verbose. Instead of simply:

AppBase.setCanvasFillMode(mode);

Minimize the reading and provide simple discoverability when user needs it - that is much better, than overloading with information with the line that can't even fit the width of the screen:

AppBase.setCanvasFillMode(mode: "FILL_WINDOW" | "FILLMODE_FILL_WINDOW" | "FILLMODE_KEEP_ASPECT", width?: number | undefined, height?: number | undefined): void

This example you've provided, is one of the first constants that been written for the engine, a better API design would not use something like that. And use simple clear strings, easy to discover and remember, and surely not taking 3 widths of the screen.

[kpal81xd]

Screen width is not an issue.

That is a tooltip generated by VSCode and will be wrapped and formatted accordingly. The literals can easily be put into a type for better notation but Typescript will fold the literals in directly:

type FillMode = "FILL_WINDOW" | "FILLMODE_FILL_WINDOW" | "FILLMODE_KEEP_ASPECT"
...
AppBase.setCanvasFillMode(mode: FillMode)

[Maksims]

AppBase.setCanvasFillMode(mode: FillMode)

But now it is not clear, is it an object argument? It is not a string. But users use strings.