Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Audit all usages of common footgun types like object #2595

Open
LukeAbby opened this issue Jul 6, 2024 · 1 comment
Open

Audit all usages of common footgun types like object #2595

LukeAbby opened this issue Jul 6, 2024 · 1 comment
Labels
foundry v11 This belongs to Foundry VTT version 11
Milestone

Comments

@LukeAbby
Copy link
Collaborator

LukeAbby commented Jul 6, 2024

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
Copy link
Collaborator Author

LukeAbby commented Jul 6, 2024

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.

@JPMeehan JPMeehan added the foundry v11 This belongs to Foundry VTT version 11 label Jul 6, 2024
@JPMeehan JPMeehan added this to the v11 Release milestone Jul 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
foundry v11 This belongs to Foundry VTT version 11
Projects
None yet
Development

No branches or pull requests

2 participants