[Tech] Let's be even more strict when writing TS code #1761
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These changes mostly boil down to: - Telling TS we're sure an array member will always exist since we're accessing it with an index gotten by `findIndex` before - Using already defined `window`/`mainWindow` variables instead of obtaining them again - Checking `unknown | undefined` values for being `undefined` first, before assigning them to something The syntax is still a little messy in places, I don't really like having to repeat `someArray[index]!` all the time. But that's something that can be improved some other time :^)
CommandMC
commented
Aug 31, 2022
Nocccer
requested changes
Sep 1, 2022
|
Arg, got caught by polymorphism here... |
These things are quite complicated, not sure why we didn't use the library since the start
Someone in the internet said checking for null also catches undefined. |
|
I ended up just using the |
CommandMC
added
pr:ready-for-review
Nocccer
reviewed
Sep 16, 2022
flavioislima
reviewed
Oct 8, 2022
flavioislima
requested review from
a team,
arielj,
imLinguin,
redromnon and
BrettCleary
and removed request for
a team
This makes them a little easier to read I've decided against putting them in a separate file, since they're used exclusively here. If that changes, having a `constants.ts` file might make sense Note that since the Epic Store URL is dynamic, depending on the language, I've added `$lang` to the string and replace any occurrences of it with the actual language once the URL is pulled from the object
|
@CommandMC will still work on this one? |
|
I doubt I'm going to work on it anytime soon, might as well open up a new PR then |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've changed some of our TS settings so we're forced to account for more situations that were easy to miss before. I think this will ultimately lead to a more stable experience for our users, at the cost of having to do a bit more work when writing new code
Changes in tsconfig.json:
allowUnreachableCode:falseallowUnusedLabels:falsenoUncheckedIndexedAccess:truesuppressImplicitAnyIndexErrors(Just to clarify, these changes only cleaned up our config file, no functional changes were made)
noImplicitThis: Alreadytruesincestrictis enablednoImplicitAny: Same as abovestrictNullChecks: Same as aboveUse the following Checklist if you have changed something on the Backend or Frontend: