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
Enable noUncheckedIndexedAccess
#3867
base: main
Are you sure you want to change the base?
Enable noUncheckedIndexedAccess
#3867
Conversation
✅ Deploy Preview for compassionate-pike-271cb3 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Hi @louisscruz, I'm @github-actions bot happy to help you with this PR 👋 Supported commandsPlease post this commands in separate comments and only one per comment:
|
You have successfully added a new CodeQL configuration |
@louisscruz Thanks for PR 👍 |
Motivation: Fix edge cases like in graphql#3869 Also I notice similar issue in graphql#3867 so I decided to fix it for the entire codebase.
8d5f1ee
to
7cc264d
Compare
@IvanGoncharov Just giving a ping on this. I just rebased to address a conflict. I noticed the use of delegating multiple checks to single |
I'm interested in possibly contributing to
graphql
in the future. I noticed thatnoUncheckedIndexedAccess
is not yet enabled and that there was an associatedFIXME
comment in the configuration. Because it would make contributions safer in the future, and because I wanted a kind of tour of the code in this package, I turned onnoUncheckedIndexedAccess
and fixed all of the violations.The vast majority of violations were fixed with either
assert
orinvariant
. In some cases, fixes were simple in that they only required different iterating mechanisms to be done in a more type safe way.General comments/questions:
Object.entries
, but that seems to be compatible back to Node 7. Any reviews with this in mind would be good.