foundry-vtt-types icon indicating copy to clipboard operation
foundry-vtt-types copied to clipboard

Audit all usages of common footgun types like `object`

Open LukeAbby opened this issue 1 year ago • 1 comments
trafficstars

object is the wrong type in 99% of cases because it includes all functions and all arrays. It has some limited usage in complex scenarios, mostly to prevent an index signature but that only matters when the base constraint is used or in other similarly niche scenarios.

Even when any object, any array, or any function is truly meant it's more self documenting to say Record<string, unknown> | Record<number, unknown> | (...args: any[]) => any. If this is a common situation then helper types could shorten it.

LukeAbby avatar Jul 06 '24 23:07 LukeAbby

Similar footgun types include, but are not limited to:

  • {}; this is not an empty object, it instead includes numbers, strings, arrays, literally everything but null and undefined.
  • Record<string, any>; this includes plain objects, arrays, and functions. This probably is meant to be Record<string, unknown> which is only plain objects.
  • Record<string | number, T> this doesn't include arrays. Use Record<string, T> | Record<number, T> for that.
  • Record<any, T> this only includes objects.
  • Record<never, T> - this probably was supposed to be Record<string, never> which is the closest type to an object with no properties. Record<never, T> allows any object.
  • Record<string, unknown> - despite only being for plain objects, this has the footgun that interfaces can't be assigned to it. This is somewhat illogical if they already have properties. This can be rectified on the user's end by merging in the index signature. This also has the second footgun of not allowing readonly properties.
  • In certain contexts Promise is a arguably a footgun because it disallows synchronous functions to be assigned to it. This is mostly only a problem within callbacks or object properties.
  • any is also a footgun in many contexts. In some cases it makes sense but any in many contexts can be replaced with unknown or never or so on.

Therefore the best constraint to use for most objects would be { readonly [K: string]: T } unless mutability is actively desired.

LukeAbby avatar Jul 06 '24 23:07 LukeAbby