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
test(NODE-4498): ts tests for OptionalId wrapping schemas with _id #3486
test(NODE-4498): ts tests for OptionalId wrapping schemas with _id #3486
Conversation
@@ -44,20 +55,110 @@ class MyId {} | |||
expectNotType<OptionalId<{ _id: MyId; a: number }>>({ a: 3 }); | |||
expectNotType<OptionalId<{ _id: MyId; a: number }>>({ _id: new ObjectId(), a: 3 }); | |||
|
|||
// OptionalId assignability when wrapping a schema with _id: ObjectId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional, non-blocking
I've had two thoughts for better "grouping" tests in our typescript tests
- we could wrap them in functions (
async function testOptionalIdAssignibility() { ... }
) - we could just wrap them in arbitrary scopes
two benefits of either approach would be better understanding of where each set of tests start and end, as well as no need to worry about naming collisions
thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's an interesting idea but I don't want to set the precedent in this PR, we can discuss this further when we revisit cleaning up our TS tests at some point
test/types/schema_helpers.test-d.ts
Outdated
const typeTestCollection = new Collection<OptionalId<SchemaWithIdType>>({} as Db, 'test'); | ||
const interfaceTestCollection = new Collection<OptionalId<SchemaWithIdInterface>>({} as Db, 'test'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const typeTestCollection = new Collection<OptionalId<SchemaWithIdType>>({} as Db, 'test'); | |
const interfaceTestCollection = new Collection<OptionalId<SchemaWithIdInterface>>({} as Db, 'test'); | |
declare const typeTestCollection: Collection<OptionalId<SchemaWithIdType>>; | |
declare const interfaceTestCollection: Collection<OptionalId<SchemaWithIdInterface>>; |
up to you - I find using declare less cluttered than actually instantiating our classes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
Description
NODE-4498
What is changing?
Adding tests to make sure document types returned from methods on a collection using an
OptionalId
-wrapped schema with a defined_id
are still assignable to the original schema.Is there new documentation needed for these changes?
No
What is the motivation for this change?
Ensure user types continue to work as expected when using the
OptionalId
helper wrapperDouble check the following
npm run check:lint
scripttype(NODE-xxxx)[!]: description
feat(NODE-1234)!: rewriting everything in coffeescript