fix(NODE-3275): Fix enum type export naming and serverApi validation #2809
Conversation
src/mongo_client.ts
Outdated
|
|
||
| /** @public */ | ||
| export interface ServerApi { | ||
| version: string | ServerApiVersionId; | ||
| version: string | ServerApiVersion; |
There was a problem hiding this comment.
@emadum Do you happen to know why this is typed as string | ServerApiVersion? The api version is also a string, so this typing is equivalent to just string - do we actually want to allow arbitrary strings here?
There was a problem hiding this comment.
I don't recall a reason, we don't want to allow arbitrary strings so I think string can be removed.
There was a problem hiding this comment.
Actually, it looks like it's currently assigned when we parse the connection options on the condition that it is a string... do we want to restrict the option validation so that only the api versions known to the driver are supported? or do we want users to be able to specify any version regardless of whether it's known to the driver?
There was a problem hiding this comment.
Cross referencing the spec:
it looks like the string | ServerApiVersion given in the spec for the class is intended to accommodate languages that can't provide an enum; since we can provide an enum via typescript, it's fine for us to just have the enum, however in light of:
the driver MUST validate (e.g. when the application provides a version string to the ServerApi class) that the version string is valid and trigger a client-side error if an unknown API version was used
I'm going to add the check and error in the connection parser since typescript doesn't provide run time validation
dariakp
force-pushed
the
NODE-3275/enum-type-export-naming
branch
from
b6f2fb8
to
f420e36
Compare
dariakp
requested review from
a team,
durran,
emadum and
nbbeeken
and removed request for
a team
dariakp
changed the title
Description
NODE-3275
Make type exports for enums use the same name as the enums themselves