-
Notifications
You must be signed in to change notification settings - Fork 352
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
chore: fix TypeScript errors in login and logout files #6880
base: main
Are you sure you want to change the base?
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
@@ -3,8 +3,7 @@ import { OptionValues } from 'commander' | |||
import { chalk, exit, getToken, log } from '../../utils/command-helpers.js' | |||
import BaseCommand from '../base-command.js' | |||
|
|||
// @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
@@ -143,7 +151,7 @@ export const getToken = async (tokenFromOptions) => { | |||
if (tokenFromConfig) { | |||
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
tokenTuple
type for thegetToken
function's return typeUpdated
getToken
function 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)