Skip to content

Conversation

addaleax
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@gribnoysup gribnoysup left a comment

Choose a reason for hiding this comment

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

Looks good!

// support for a newly introduced driver option when it is being added
// to the driver API.
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const __typecheck1: (typeof allTransactionOptions)[number] = '' as Exclude<keyof TransactionOptions, keyof CommandOperationOptions>;
Copy link
Collaborator

@gribnoysup gribnoysup Mar 17, 2022

Choose a reason for hiding this comment

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

Don't have a strong preference really, but it took me awhile to grasp what's exactly is going on there, at the same time I used something like this before (which provides the same result and also doesn't require eslint-disable):

 function assertAllOptionsUsed(options: Exclude<keyof TransactionOptions, keyof CommandOperationOptions>) { // These typechecks might look weird, but will tell us if we are missing // support for a newly introduced driver option when it is being added // to the driver API. } assertAllOptionsUsed(allTransactionOptions)

EDIT: yikes, I think this particular snippet doesn't work, sorry, but the idea would be to use a noop function to do the assertion instead of using as operator. But now I'm thinking that maybe I'm just misunderstanding how this works and so feel free to ignore this 😅

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, your suggestion is basically to use a function call instead of an assignment, right? That’s easy enough to do, updated the code here :) Still needs eslint comments, though, because the argument is unused.

@addaleax addaleax merged commit 95ad859 into main Mar 17, 2022
@addaleax addaleax deleted the 1151-dev branch March 17, 2022 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants