-
Notifications
You must be signed in to change notification settings - Fork 430
chore: fix TypeScript errors in login and logout files #6880
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
Conversation
Fixed TS errors in login and logout. Created `tokenTuple` type for the `getToken` function's return type Updated `getToken` function calls to remove TypeScript ignore comments Co-authored-by: Ben Hancock<benhancock859@gmail.com>
Co-authored-by: Ben Hancock <benhancock859@gmail.com>
…ed tokens Co-authored-by: Ben Hancock <benhancock859@gmail.com>
src/commands/login/login.ts
Outdated
|
|
||
| // @ts-expect-error TS(7006) FIXME: Parameter 'location' implicitly has an 'any' type. | ||
| const msg = function (location) { | ||
| const msg = function (location: 'env' | 'flag' | 'config' | 'not found') { |
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.
you can extract this type and move it into the types file as it's used later as well
src/commands/integration/deploy.ts
Outdated
| let [token] = await getToken() | ||
| if (!token) { | ||
| token = '' | ||
| } |
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.
why did this need to change?
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.
Prior to our changes, getToken returned an any type. In reality, this value would be a string or undefined. The fetch request below it (see screenshot) relies on token being a string type. Because getToken can return undefined, and we don't want to pass undefined to the fetch request, we reassigned it to an empty string.
It does seem that our change may have unintended consequences as a result of changing token from undefined to an empty string in cases where there is no token. We are pushing a change that goes about fixing this in a way that does not involve reassigning token.
src/utils/command-helpers.ts
Outdated
| return [tokenFromConfig, 'config'] | ||
| } | ||
| return [null, 'not found'] | ||
| return [undefined, 'not found'] |
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.
why did this change as well?
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.
This was originally intended to satisfy the type requirements of a pair of functions that use getToken's return value. Our updated commit restores the original return value and creates a new variable whose value is set based on getToken's return value to preserve type safety for these function calls.
Co-authored-by: Ben Hancock <benhancock859@gmail.com>
Co-authored-by: Ben Hancock <benhancock859@gmail.com>
In cases where `getToken` can't find a token, changed it back to previous behavior of returning `[null, 'not found']`. Changed `tokenTuple` type to reflect changes. This preserves previous behavior while ensuring type safety for function calls that use `getToken`'s return value. This required a new variable `blobsToken` to satisfy `runCoreSteps` function argument type requirements. Co-authored-by: Ben Hancock <benhancock859@gmail.com>
Co-authored-by: Daniel Lew <51924260+DanielSLew@users.noreply.github.com>
Co-authored-by: Ben Hancock <benhancock859@gmail.com>


🎉 Thanks for submitting a pull request! 🎉
Summary
Fixes conversion to TypeScript
Fixed TS errors in login and logout.
Created
tokenTupletype for thegetTokenfunction's return typeUpdated
getTokenfunction calls to remove TypeScript ignore commentsFor us to review and ship your PR efficiently, please perform the following steps:
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)