Skip to content

fix: actually populate auth user field #7181

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 5 commits into from
Apr 15, 2025
Merged

Conversation

serhalp
Copy link
Collaborator

@serhalp serhalp commented Apr 9, 2025

Summary

The types fixes in #7130 identified a number of bugs. This fixes one of those. We were reading from the wrong property on the GitHub API response.

Copy link

github-actions bot commented Apr 9, 2025

📊 Benchmark results

Comparing with 1c27810

  • Dependency count: 1,237 (no change)
  • Package size: 311 MB (no change)
  • Number of ts-expect-error directives: 418 ⬇️ 0.24% decrease vs. 1c27810

@serhalp serhalp marked this pull request as ready for review April 9, 2025 16:15
@serhalp serhalp requested a review from a team as a code owner April 9, 2025 16:15
@serhalp serhalp requested a review from ndhoule April 14, 2025 19:49

const { authMethod } = await inquirer.prompt([
const { authMethod } = (await inquirer.prompt([
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't notice this before, but prompt accepts a generic:

Suggested change
const { authMethod } = (await inquirer.prompt([
const { authMethod } = (await inquirer.prompt<{ authMethod: typeof authChoices[number] }>([

I don't think this is any more safe, but at least lets you avoid the assertion? 🤷

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird, I believe I'd tried that and couldn't get it working, but it's working fine now. I've just refactored all the inquirer.prompt type assertions. 👍🏼

@@ -32,7 +32,7 @@ const promptForAuthMethod = async () => {
'What would you like to do?',
choices: authChoices,
},
])
])) as { authMethod: typeof authChoices[number] }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
])) as { authMethod: typeof authChoices[number] }
]))

@serhalp serhalp enabled auto-merge (squash) April 15, 2025 15:55
@serhalp serhalp merged commit fe1de94 into main Apr 15, 2025
52 checks passed
@serhalp serhalp deleted the fix/populate-auth-user-correctly branch April 15, 2025 16:14
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants