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(schema): add BigInt support, upgrade mongodb -> 5.3.0 #13318

Merged
merged 10 commits into from
Apr 25, 2023

Conversation

vkarpov15
Copy link
Collaborator

@vkarpov15 vkarpov15 commented Apr 23, 2023

Fix #13081
Fix #6936

Summary

Add support for BigInt as a schema type, now that MongoDB driver supports it and all Node.js versions (and Deno) that Mongoose supports have support for BigInt.

Still WIP:

  1. Test query casting ($gt, etc.)
  2. populate()

Examples

Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

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

i think useBigInt64 still needs to be added to the documentation (likely at Query.prototype.setOptions?)

also SchemaBigInt would need to be added to the list in Schema.Types (which seemingly is also missing UUID, subdocumentpath and documentarray)

lib/schema/bigint.js Outdated Show resolved Hide resolved
test/bigint.test.js Show resolved Hide resolved
Co-authored-by: hasezoey <hasezoey@gmail.com>
@vkarpov15
Copy link
Collaborator Author

@hasezoey this PR adds mongoose.Schema.Types.BigInt, see below node shell output

$ node
Welcome to Node.js v16.17.0.
Type ".help" for more information.
> const mongoose = require('./')
undefined
> mongoose.Schema.Types.BigInt
[Function: SchemaBigInt] {
  schemaName: 'BigInt',
  defaultOptions: {},
  _cast: [Function: castBigInt],
  set: [Function: set],
  cast: [Function: cast],
  _checkRequired: [Function (anonymous)],
  checkRequired: [Function (anonymous)],
  '$conditionalHandlers': {
    '$gt': [Function: handleSingle],
    '$gte': [Function: handleSingle],
    '$lt': [Function: handleSingle],
    '$lte': [Function: handleSingle],
    '$type': [Function (anonymous)],
    '$exists': [Function (anonymous)],
    '$nin': [Function: handle$in],
    '$ne': [Function: handleSingle],
    '$in': [Function: handle$in],
    '$eq': [Function: handleSingle],
    '$all': [Function: handleArray]
  }
}
> 

@vkarpov15
Copy link
Collaborator Author

I'll add useBigInt64 to the docs, but for the most part you don't have to use that. I added it to the API docs and docs on lean() though just in case.

@hasezoey
Copy link
Collaborator

also SchemaBigInt would need to be added to the list in Schema.Types (which seemingly is also missing UUID, subdocumentpath and documentarray)

@hasezoey this PR adds mongoose.Schema.Types.BigInt, see below node shell output

sorry i wasnt clear, i meant at:

mongoose/lib/schema.js

Lines 2658 to 2667 in f50a194

* #### Types:
*
* - [String](/docs/schematypes.html#strings)
* - [Number](/docs/schematypes.html#numbers)
* - [Boolean](/docs/schematypes.html#booleans) | Bool
* - [Array](/docs/schematypes.html#arrays)
* - [Buffer](/docs/schematypes.html#buffers)
* - [Date](/docs/schematypes.html#dates)
* - [ObjectId](/docs/schematypes.html#objectids) | Oid
* - [Mixed](/docs/schematypes.html#mixed)

@vkarpov15
Copy link
Collaborator Author

@hasezoey good point, fixed 👍

Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

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

LGTM

@vkarpov15 vkarpov15 merged commit 624ee82 into 7.1 Apr 25, 2023
@hasezoey
Copy link
Collaborator

seems like esprima does not like big numbers (the n at the end), we probably need to look into updating that in the future (used by acquit and nyc?)

@hasezoey hasezoey deleted the vkarpov15/gh-13081 branch April 25, 2023 18:46
@hasezoey hasezoey added this to the 7.1.0 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants