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

Convert CONST properties to Branded types where appropriate #2992

Open
esheyw opened this issue Dec 21, 2024 · 2 comments
Open

Convert CONST properties to Branded types where appropriate #2992

esheyw opened this issue Dec 21, 2024 · 2 comments

Comments

@esheyw
Copy link
Collaborator

esheyw commented Dec 21, 2024

Lots of properties in CONST are currently in the form

export declare const ENUM_NAME: Readonly<{
  prop1: T;
  prop2: T;
}>
export type ENUM_NAME = ValueOf<typeof ENUM_NAME>

The repo has been moving towards using the new Brand type like so:

export type ENUM_NAME: Brand<T, "Namespace.ENUM_NAME">
export declare const ENUM_NAME: Readonly<Record<"prop1" | "prop2", ENUM_NAME>>;

This works for properties that are already effectively Record<string, T>, but I am not sure what we want to do with arrays. There's maybe value in preserving the literal values of things like CONST.WORLD_DOCUMENT_TYPES, etc.

@JPMeehan
Copy link
Collaborator

The various arrays have a lot of value in not collapsing to string

@LukeAbby
Copy link
Collaborator

Yeah I probably would recommend not converting arrays. They can act like pseudo-enums but you're supposed to refer to them by value. Going around and doing WORLD_DOCUMENT_TYPES[3] would be silly, just write "Combat".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants