-
Notifications
You must be signed in to change notification settings - Fork 132
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
Clean up & standardise TypeScript conventions #2234
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I briefly looked through and looks good to me. Curiously most of the changes in the form of{ [key: string]: string };
to Record<string, string>;
will make the type work better or? What's the advantage of Record
|
What is the purpose of this pull request?
Overview of changes:
As we've been doing quite a large-scale file-by-file migration to TypeScript (#1913), there is some inconsistency and redundancy in how we handle some types, exports, etc. With #2201 and #2221, we now have a more organised typing system and a rough "convention" that can be followed by TS files.
This PR:
Record
Record<K,V>
is interchangeable with{ [key: K]: V }
syntax, but is more extensible, generalisable, and is an officially supported utility type in TS.js
files are nowts
filesas unknown as
andany
typecasts, now that we have a more mature typing systemAnything you'd like to highlight/discuss:
N/A
Testing instructions:
N/A
Proposed commit message: (wrap lines at 72 characters)
Checklist: ☑️