Support for BigInt field types #21

jonbarrow · 2023-08-23T19:17:01Z

Mongoose supports BigInts as a schema field type, and this library already works with them as-is. The only thing preventing this was a check for only Number of the field type

hasezoey

please add a test actually verifying BigInt works~~, also, you may wanna check utils.isNumber too~~

Edit: nevermind, i thought this was a PR for typegoose (not auto-increment)

jonbarrow · 2023-08-24T16:02:27Z

I tested this in an actual project I'm working on and it worked just fine, but I'll add a proper test here later today

jonbarrow · 2023-08-24T18:14:18Z

After looking into this further, it looks like Mongoose doesn't expose BigInt in their type defs (see here for reference), which is why these tests are failing. However Mongoose does support this, as seen here as well as the documentation

The way I had tested this originally in my personal project was, admittedly, quick and hacky. I just modified the transpiled JavaScript to add these checks, and it works just fine

In fact you can verify that this type does exist and is accessible by simply logging it

The issue is only that TypeScript thinks the property BigInt doesn't exist, when it does

Not entirely sure how to proceed from here. This is a type defs issue now, not a functionality issue. I suppose I'll file an issue on the Mongoose side and then check back later

… support BigInts

hasezoey

looks better now, but the snapshots would need to be updated (yarn run test -u, and verify it is what it should be)

also it looks like there are only tests for BigInt and AutoIncrementID and not AutoIncrementSimple?

jonbarrow · 2023-08-25T14:06:47Z

Ah my apologies. I have never used yarn or jest before, and the test which failed was named with should Error, so I assumed it failing was expected. Thank you

… BigInt and cast

jonbarrow · 2023-08-25T14:33:26Z

There were some errors which cropped up while adding the AutoIncrementSimple when count would become negative and newCount was a BigInt. To fix this I had to change count to be a BigInt internally and cast from Number if it wasn't already

All tests pass now, though

codecov · 2023-08-25T18:18:46Z

Codecov Report

Patch coverage is 90.90% of modified lines.

Files Changed	Coverage
src/autoIncrement.ts	`90.90%`

📢 Thoughts on this report? Let us know!.

hasezoey

Sorry for the delay, code looks relatively good to me, though there are still some things i noticed:

type AutoIncrementIDOptions options startAt and incrementBy do not list bigint as supported
type AutoIncrementOptionsSimple options incrementBy do not list bigint as supported
i would rather not change the IDSchema's type, but i guess it should not be a problem (old stuff should get automatically get converted to bigint from number)

src/autoIncrement.ts

hasezoey · 2023-08-27T09:00:31Z

There were some errors which cropped up while adding the AutoIncrementSimple when count would become negative and newCount was a BigInt.

also could you explain more on what the problem with negative was?

jonbarrow · 2023-08-29T18:45:06Z

My apologies for the late reply as well

type AutoIncrementIDOptions options startAt and incrementBy do not list bigint as supported
type AutoIncrementOptionsSimple options incrementBy do not list bigint as supported

I'm not sure what you mean here? Where should this be listed at? The types themselves are already updated to number | bigint for each field you listed here

i would rather not change the IDSchema's type, but i guess it should not be a problem (old stuff should get automatically get converted to bigint from number)

In my testing yes, everything does get converted correctly

also could you explain more on what the problem with negative was?

I do apologize but I've forgotten the specifics now, I've been prepping for the hurricane about to hit my state so that was the last thing on my mind 😅

hasezoey · 2023-08-30T12:30:22Z

I'm not sure what you mean here? Where should this be listed at? The types themselves are already updated to number | bigint for each field you listed here

i am sorry, it seems i look at some other versions of the code and didnt notice, all is good here, aside from AutoIncrementIDTrackerSpec missing the switch to bigint

… callback

hasezoey · 2023-08-31T12:42:04Z

src/autoIncrement.ts

@@ -39,13 +39,18 @@ export function AutoIncrementSimple(
      throw new Error(`Field "${field.field}" does not exists on the Schema!`);
    }
    // check if the field is an number
-    if (!(schemaField instanceof mongoose.Schema.Types.Number)) {
-      throw new Error(`Field "${field.field}" is not an SchemaNumber!`);
+    if (schemaField.instance !== 'Number' && schemaField.instance !== 'BigInt') {


should this still be used once upgraded to mongoose 7.5, which has typescript types for BigInt?

This reverts commit 2941446.

* Support for BigInt field types * refactor: switch from instanceof to field.instance for type checks to support BigInts * test: added BigInt tests * refactor: allow incrementBy and startAt to be BigInt, change count to BigInt and cast * test: added Simple tests and update snapshots * refactor: move incrementBy and startAt BigInt conversions to pre save callback * fix: type AutoIncrementIDTrackerSpec count is now bigint

hasezoey · 2023-09-01T13:44:34Z

i have merged this PR for now, but did a mistake and had to force-push it (bad commit message).

i would still like a answer to #21 (comment)

hasezoey · 2023-09-01T14:05:06Z

Released in 3.5.0

Support for BigInt field types

fea2f66

hasezoey requested changes Aug 24, 2023

View reviewed changes

jonbarrow mentioned this pull request Aug 24, 2023

Property 'BigInt' does not exist on type 'typeof Types' Automattic/mongoose#13780

Closed

2 tasks

jonbarrow added 2 commits August 24, 2023 14:39

refactor: switch from instanceof to field.instance for type checks to…

3d5580f

… support BigInts

test: added BigInt tests

5b70397

hasezoey requested changes Aug 25, 2023

View reviewed changes

jonbarrow added 2 commits August 25, 2023 10:30

refactor: allow incrementBy and startAt to be BigInt, change count to…

1805e3f

… BigInt and cast

test: added Simple tests and update snapshots

cb2d1e2

hasezoey requested changes Aug 27, 2023

View reviewed changes

src/autoIncrement.ts Outdated Show resolved Hide resolved

jonbarrow added 2 commits August 30, 2023 14:41

refactor: move incrementBy and startAt BigInt conversions to pre save…

ce5e871

… callback

fix: type AutoIncrementIDTrackerSpec count is now bigint

e697e13

hasezoey reviewed Aug 31, 2023

View reviewed changes

hasezoey merged commit 2941446 into typegoose:master Sep 1, 2023

hasezoey added a commit that referenced this pull request Sep 1, 2023

Revert "Support for BigInt field types (#21)"

d726305

This reverts commit 2941446.

hasezoey added the released label Sep 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for BigInt field types #21

Support for BigInt field types #21

jonbarrow commented Aug 23, 2023

hasezoey left a comment •

edited

Loading

jonbarrow commented Aug 24, 2023

jonbarrow commented Aug 24, 2023 •

edited

Loading

hasezoey left a comment

jonbarrow commented Aug 25, 2023

jonbarrow commented Aug 25, 2023

codecov bot commented Aug 25, 2023 •

edited

Loading

hasezoey left a comment

hasezoey commented Aug 27, 2023

jonbarrow commented Aug 29, 2023

hasezoey commented Aug 30, 2023

hasezoey Aug 31, 2023

hasezoey commented Sep 1, 2023

hasezoey commented Sep 1, 2023

Support for BigInt field types #21

Support for BigInt field types #21

Conversation

jonbarrow commented Aug 23, 2023

hasezoey left a comment • edited Loading

Choose a reason for hiding this comment

jonbarrow commented Aug 24, 2023

jonbarrow commented Aug 24, 2023 • edited Loading

hasezoey left a comment

Choose a reason for hiding this comment

jonbarrow commented Aug 25, 2023

jonbarrow commented Aug 25, 2023

codecov bot commented Aug 25, 2023 • edited Loading

Codecov Report

hasezoey left a comment

Choose a reason for hiding this comment

hasezoey commented Aug 27, 2023

jonbarrow commented Aug 29, 2023

hasezoey commented Aug 30, 2023

hasezoey Aug 31, 2023

Choose a reason for hiding this comment

hasezoey commented Sep 1, 2023

hasezoey commented Sep 1, 2023

hasezoey left a comment •

edited

Loading

jonbarrow commented Aug 24, 2023 •

edited

Loading

codecov bot commented Aug 25, 2023 •

edited

Loading