Best type for Game object Index Signatures?

[tprk77]

I've been using typed-screeps for a few months now, and have been getting more familiar with TypeScript. This library is extremely useful and it's been great. There's still one thing that seems odd to me, and I'm wondering if it's an opportunity for a PR.

When you do a index into one of the Game objects, the result is always non-nullable, even though the result actually may be undefined. For example:

let nonExistentFlagName = "nonExistentFlagName";

// "Game.flags" has type "{ [flagName: string]: Flag }"

let flag1 = Game.flags[nonExistentFlagName];

// "flag1" has type "Flag" but value "undefined"

The type of Game.flags certainly looks reasonable, and seems like idiomatic TypeScript. But flag1 is always a Flag even when the lookup has failed and the value is undefined. So it seems that flag1 should really have type Flag | undefined. And following that logic, Game.flags should really have type { [flagName: string]: Flag | undefined }.

My understanding is that, by default, all types in TypeScript are nullable. So this really only matters when strict mode is turned on. But I still think it would be nice to have the option for the extra type safety, and it seems more correct. Plus, screeps-typescript-starter comes with strict mode turned on.

The other thing I might mention, is that ES6 Map.get returns a nullable type. For example:

// Imagining that "Game.flags" had type "Map<string, Flag>"...

let flag1 = Game.flags.get(nonExistentFlagName);

// "flag1" has type "Flag | undefined"

So there is some precedence I think. Anyway, like I mentioned before, the change could be accomplished by adding | undefined to the various Game objects. I don't think the change would require too much work for users. In some cases it might uncover bugs. In other cases, users would just have to add ! non-null assertion operators. For example:

let existentFlagName = "existentFlagName";

let flag1 = Game.flags[existentFlagName]!;

// "flag1" forced to type "Flag"

At the end of the day, it would still require some user intervention, and it would break existing code. So it is a big change.

So did all of that make sense? Does a change like what I'm proposing make sense? (Probably with a version bump.) If so, I would be happy to write a PR. Please let me know.

[tprk77]

I've noticed that changing the hash type does negatively affect loops. When iterating over, e.g., Game.rooms, we know that all of the elements will not be undefined. But this is not expressed by the type { [roomName: string]: Room | undefined }, which describes elements which can be a Room or undefined in all contexts, in lookups, iteration, etc.

Fixing it requires temporarily overriding the type, which is pretty ugly:

// Find all controlled and claimed rooms
const claimedRooms = _.filter(Game.rooms as {[roomName: string]: Room}, (room) => {
  return room.controller && room.controller.my;
});

(The type of claimedRooms is then {[roomName: string]: Room} which then kind of goes back to the original problem if I want to do lookups on this new hash.)

I also found this discussion about pretty much the same thing in the TpyeScript repo:

Microsoft/TypeScript#9235 https://github.com/Microsoft/TypeScript/pull/7140#issuecomment-192606629

So I don't think my approach in #108 is really workable anymore. I don't really want to model a hash with undefined elements. I think what I'm really looking for is to just change the type of the hash lookup function. But I don't think it's really possible in TypeScript right now.

[Jomik]

You can use https://www.typescriptlang.org/tsconfig#noUncheckedIndexedAccess

[Jomik]

Can we close this?