feat!(NODE-4862): add BSONType enum and remove internal constants from export #532
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nbbeeken
commented
Dec 2, 2022
| }); | ||
| }); | ||
|
|
||
| describe('BSONType enum', () => { |
There was a problem hiding this comment.
Tests above are old, no change except for the import
Tests here are new for the enum
nbbeeken
commented
Dec 2, 2022
| } as const); | ||
|
|
||
| /** @public */ | ||
| export type BSONType = typeof BSONType[keyof typeof BSONType]; |
There was a problem hiding this comment.
Using the enum pattern here, it makes BSONType the annotation equal to a union of the values.
BSONType the runtime value is an object with properties.
durran
added
the
Primary Review
durran
reviewed
Dec 6, 2022
There was a problem hiding this comment.
Are we planning on removing the export in https://github.com/mongodb/node-mongodb-native/blob/main/src/mongo_types.ts to just re-export BSONType from here as part of NODE-4701?
durran
previously approved these changes
Dec 6, 2022
|
I expect we'll change to a re-export in this ticket: https://jira.mongodb.org/browse/NODE-4867 |
durran
approved these changes
Dec 7, 2022
durran
added
Team Review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
What is changing?
What is the motivation for this change?
The exports are a blocker for #524 passing typescript tests
The enum is a nice to have, it provides the same information that constants did but in a consistent way the driver already does, locating the enum here puts it closer to the code that it relates to, its not used by the BSON library but if someone needs type indicators this is a consistent way to make it available.
Double check the following
npm run lintscript<type>(NODE-xxxx)<!>: <description>