Skip to content
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

feat: Add AnyAuthClient type #1843

Merged
merged 3 commits into from
Aug 14, 2024
Merged

feat: Add AnyAuthClient type #1843

merged 3 commits into from
Aug 14, 2024

Conversation

danielbankhead
Copy link
Contributor

🦕

@danielbankhead danielbankhead requested review from a team as code owners August 6, 2024 19:34
@product-auto-label product-auto-label bot added the size: s Pull request size is small. label Aug 6, 2024
src/index.ts Outdated
*
* @experimental
*/
export type AuthClients = InstanceType<
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not too sure about the name; AuthClients could imply AuthClient[]. Would something like AnyAuthClient be clearer?

Choose a reason for hiding this comment

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

I see what you mean about the naming. AnyAuthClient works for me, or UnionAuthClients? this is a tough naming situation.

My other feedback is that there is a lot going on in the InstanceType and the Extract - is it possible to split this up into a couple of lines and to comment what's going on so it's easier to read? It's hard to parse just by looking at it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated

src/index.ts Outdated
*
* @experimental
*/
export type AuthClients = InstanceType<

Choose a reason for hiding this comment

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

I see what you mean about the naming. AnyAuthClient works for me, or UnionAuthClients? this is a tough naming situation.

My other feedback is that there is a lot going on in the InstanceType and the Extract - is it possible to split this up into a couple of lines and to comment what's going on so it's easier to read? It's hard to parse just by looking at it

@danielbankhead danielbankhead changed the title feat: Add AuthClients type feat: Add AnyAuthClient type Aug 7, 2024
@leahecole leahecole merged commit 3ae120d into main Aug 14, 2024
18 checks passed
@leahecole leahecole deleted the type-authclients branch August 14, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants