Tighten up types for Entity API

[willeastcott]

At the moment, the Entity API has reasonably weak typings. This PR improves the situation, particularly for addComponent.

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

[Copilot]

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

[hmans]

Hi! As promised, a couple of suggestions. I think this PR is already great in how it establishes a list of supported internal components, mapping them to their data object types, and requiring addComponent to accept one of the official component names:

The big drawback of the PR in its current state is that custom components will always yield a type error (the error around "bad" here was due to me using double quotes, but note how the component name is yielding an error, too):

I think generally with an API like this we want to achieve three things:

1. auto-suggest and -complete known component names (the first argument)
2. auto-suggest etc. and validate the shape of the second argument, if known
3. but also allow custom component names and their data objects

I think one way forward into this direction would be to introduce several type overloads. I've prototyped this on my machine, but instead of opening a PR-into-the-PR or similar, I'm just going to post some code here, if that's alright.

Also, please note that I've never used jsdoc types before (I'm very much a TS person), so chances are this can be cleaned up or otherwise optimized further.

Basically, the idea is to provide two overloads for addEntity:

• one accepts a known internal entity name and its associated data object
• the other accepts just any random string together with a data object typed unknown.

This allows both cases to be supported:

This also successfully gives rich help for all involved properties:

There is one big caveat though -- and I haven't been able to solve it, but maybe someone else has an idea: when you use an internal component but mis-type a property name, the tooling will immediately switch to the "anonymous" overload, instead of marking the unknown property as an error:

I think even with this caveat this is already pretty nice, and if there is no way to solve it, then it might be an acceptable trade-off (compared to just out-right erroring all custom components.)

Implementation

Basically, I just added these two overloads to the codedoc of addComponent:

     *
     * @overload
     * @param {T} type
     * @param {Partial<ComponentMap[T]>} [data]
     *
     * @overload
     * @param {string} type
     * @param {unknown} [data]

Some open ends:

• I'm not familiar with jsdoc types at all, so maybe something can be expressed in a leaner fashion
• Probably need to check if documentation still renders correctly in tooling (what I've seen so far looked OK)
• removeComponent and maybe others probably need a similar treatment

I hope this helps a little. Either way, looking forward to the improved typings!

[willeastcott]

Wow @hmans - we make a great team. 😄 Thanks for your help! Check out the updated PR. Seems to work great.

entity-typings.mp4

Here are the docs:

What do you think?

[LeXXik]

This is pretty close to my land. I do have custom physics components that get introduced once you import our Jolt physics library:

github.com/gamebop/physics

It will create multiple new components, like 'body'. I just had to suck up the errors, since, I concured I am in minority of users who introduce new components to the system. However, I am aware of the strides towards supporting custom addons/components and was holding off till closed beta is out.

My library will definitely benefit from this PR. Good motion!

Also worth mentioning that a factory component could add Debug asserts for supplied properties. We do it to spot any unrecognized properties, mostly due to typos. A component may throw a warning if a property is not recognized, or values are out of acceptable range.

[willeastcott]

@LeXXik Wow, that physics library looks really cool! Nice job!

@hmans I do have one concern about something. Before adding support for custom components, if you tried to add an invalid property to the options object, IntelliSense would show a red line under that property and log an error in the PROBLEMS panel. Now, if you do that, IntelliSense thinks it's just a custom component. Hmmm. I'm wondering if we can say a custom component is a string but NOT any of the keyof ComponentMap values.

[hmans]

Before adding support for custom components, if you tried to add an invalid property to the options object, IntelliSense would show a red line under that property and log an error in the PROBLEMS panel. Now, if you do that, IntelliSense thinks it's just a custom component.

Yeah, this is the one caveat I was writing about in my post. I have some more ideas for stuff I want to try to improve this. I won't be able to get to it until later tonight, but maybe we can figure something out.

My main issue that's slowing me down right now is that I'm having a hard time figuring out a speedy development/testing flow for this. I'm diving into this codebase for the first time, and it's also the first time I am working with jsdoc types (as opposed to plain old TS, which I'm very familiar with.)

So far, the only way I've found to get my changes to entity.js to register was to npm run build the entire engine and then use the build bundle in a new example within the examples package. Is there a more direct way to approach this?

Right now I'm trying the changes from within the test scripts in the engine project, but there these changes don't seem to register at all. - this works now. A restart of VS Code seems to have fixed it. Technology! So... stay tuned. =)

[kungfooman]

Technically we should have everything that's needed from this point:

createOptions.componentSystems = [
    pc.RenderComponentSystem,
    pc.CameraComponentSystem,
    pc.LightComponentSystem,
    pc.ScriptComponentSystem,
    pc.AnimComponentSystem
];

Then just do some type voodoo. Minimal example with a new component:

class OceanComponent {}
class OceanComponentData {}
class OceanComponentSystem extends ComponentSystem {
    /** @type {'ocean'} */
    id = 'ocean';

    ComponentType = OceanComponent;

    DataType = OceanComponentData;
}

const systems = [OceanComponentSystem];
console.log("systems", systems);

/**
 * @typedef {{
 *   [S in (typeof systems)[number] as InstanceType<S>['id']]: InstanceType<InstanceType<S>['ComponentType']>
 * }} ComponentMap
 */

So we get the actual types we are using:

Instead of simply registering every PC system. Why would I even want types for e.g. ModelComponentSystem when I specifically only added RenderComponentSystem? Funnily enough the @typedef for this ComponentMap is way shorter than the first one based on the pc.ComponentSystemRegistry class.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (3)

src/framework/entity.js:452

• removeComponent is now typed as keyof ComponentMap, but the runtime implementation supports any registered system id (including custom component systems) and the same method already has a custom-component overload for addComponent. This change makes it impossible for TypeScript users to remove custom components (or built-in but not listed in ComponentMap) without casting. Consider adding overloads similar to addComponent (built-in: keyof ComponentMap; custom: string) or widening this param back to keyof ComponentMap | string while keeping the narrowed overload for known built-ins.

     * Remove a component from the Entity.
     *
     * @param {keyof ComponentMap} type - The name of the Component type.
     * @example
     * const entity = new pc.Entity();
     * entity.addComponent("light"); // add new light component
     *
     * entity.removeComponent("light"); // remove light component
     */
    removeComponent(type) {

src/framework/entity.js:476

• findComponent is now restricted to keyof ComponentMap, but it works at runtime for any component id stored in entity.c[...] (including custom component systems). Tightening this to built-ins will force casts for common patterns (e.g. looking up by a dynamic string). Consider adding a second overload for string that returns Component | undefined, while keeping the keyed overload for strong typing on built-ins.

     * Search the entity and all of its descendants for the first component of specified type.
     *
     * @template {keyof ComponentMap} T
     * @param {T} type - The name of the component type to retrieve.
     * @returns {ComponentMap[T] | undefined} A component of specified type, if the entity or any
     * of its descendants has one. Returns undefined otherwise.
     * @example
     * // Get the first found light component in the hierarchy tree that starts with this entity
     * const light = entity.findComponent("light");
     */
    findComponent(type) {

src/framework/entity.js:492

• Same issue as findComponent: findComponents is now limited to keyof ComponentMap, but the underlying storage (entity.c) supports arbitrary system ids (including custom components). Consider providing an additional overload accepting string and returning Component[] so dynamic/custom lookups remain type-safe without casts.

     * Search the entity and all of its descendants for all components of specified type.
     *
     * @template {keyof ComponentMap} T
     * @param {T} type - The name of the component type to retrieve.
     * @returns {ComponentMap[T][]} All components of specified type in the entity or any of its
     * descendants. Returns empty array if none found.
     * @example
     * // Get all light components in the hierarchy tree that starts with this entity
     * const lights = entity.findComponents("light");
     */
    findComponents(type) {

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

In the implementation signature, Partial<ComponentMap[T]> | any collapses to any, which can negate the benefit of having a typed data parameter in some tooling/emit paths. Consider using unknown (or object) instead of any here, and rely on the overloads to provide the correct data typing for built-in vs custom components.

Suggested change

	* @param {Partial<ComponentMap[T]> | any} [data] - Optional initialization data for the component.
	* @param {unknown} [data] - Optional initialization data for the component.