createEntity could return a stronger type

[mikecann]

Because after you createEntity you guarantee that certain components are going to be there you could return the type from createEntity with "required" components.

For now I am hacking this with an "as", but it would be nice is that wasnt needed as it could be inferred from the input to createEntity:

export const createCell = ({
  resources,
  world
}: {
  resources: ResourceCache;
  world: GameWorld;
}) => {
  return world.createEntity({
    position: { x: 0, y: 0, z: 0 },
    mesh: { mesh: resources.cellMesh.clone() }
  }) as EntityWith<"position" | "mesh">;
};

where:

import { World } from "miniplex";
import { Camera, FreeCamera, Mesh } from "@babylonjs/core";

export interface Entity {
  position?: { x: number; y: number; z: number };
  velocity?: { x: number; y: number; z: number };
  mesh?: { mesh: Mesh };
  camera?: { camera: FreeCamera };
}

export type ComponentNames = keyof Entity;

export type EntityWith<TComponentNames extends ComponentNames> = Entity & Required<
  Pick<Entity, TComponentNames>
>;

so then we can do this:

Thoughts? Or is it better to always have the user assume at than entity's components might not be there?

[hmans]

Thanks for the issue! I assume we're talking about the current state of the main branch, which is preparing for a 1.0 release.

I was inferring the type from the entity passed into createEntity until recently, but this had the problem of bypassing the intended type checking (ie. it would allow props to be passed in that were not valid according to the world's entity type.) The upcoming satisfies feature in TypeScript 4.9 will help here, and I'm tempted to migrate to TS 4.9 and use that before pushing the final 1.0 version.

(If you have other ideas, please let me know!)

[mikecann]

Yes I was referring to whatever the latest version is on the main branch.

but this had the problem of bypassing the intended type checking (ie. it would allow props to be passed in that were not valid according to the world's entity type.)

Yes I had a brief go at trying to do a PR for this by taking in a definable type into createEntity but couldnt get it to work:

/**
 * A RegisteredEntityWith is a RegisteredEntity (see above) but is also guaranteed to have certain components
 */
export type RegisteredEntityWith<
  TEntity extends IEntity,
  TComponentNames extends keyof TEntity
> = TEntity  & Required<Pick<TEntity, TComponentNames>> & MiniplexComponent<TEntity>

public createEntity<TInput extends T>(entity: TInput): RegisteredEntityWith<T, keyof TInput> { 
...

But I was getting this error:

As you say, maybe satisfies in 4.9 will fix it.

[hmans]

I've done some experimenting. Not sure if satisfies will help fix it, but I will keep an eye on it (and also for other potential solutions.) I do want to solve this post 1.0, though. We'll figure it out :)

[hmans]

A quick update, this is coming with the upcoming Miniplex 2.0 release (beta soon.)

It is still unclear if TS will still throw a warning if extra properties are added when the type extends the world entity type; some recent experiments with this have shown very weird results that don't make it obvious what the intended behavior of TS is.

Having said that, even if TS allows extra props to live on a type that extends another type, losing his kind of checking against extra props is an acceptable compromise if it means createEntity's return value will be more accurate. It is also in line with Miniplex 2.0's overall theme of being a lot more relaxed in a bunch of places, and easier to use.

So, I'm adding this issue to the "2.0" milestone and closing it. Thanks!

[hmans]

A quick update: it's been confirmed that TS should not display a warning for extra props, and that the intermittent instances of it still doing so is a bug.

Still, I'm going to go ahead and ship this change with 2.0. It will mean that the objects returned by add (the new API for createEntity) has the correct type, but also that add will not complain about extra entities on the object. Which is simultaneously good because it matches how TS typically thinks about types, and it shouldn't be wrong to make an object an entity even if it has additional props not known to the World's entity type; but it's also bad because now you can have a typo in a component name and not get a warning about it. But I think it's an acceptable compromise to make.