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

Issues with Decimal128 type #12431

Closed
2 tasks done
HiiiiD opened this issue Sep 14, 2022 · 11 comments
Closed
2 tasks done

Issues with Decimal128 type #12431

HiiiiD opened this issue Sep 14, 2022 · 11 comments
Labels
typescript Types or Types-test related issue / Pull Request

Comments

@HiiiiD
Copy link

HiiiiD commented Sep 14, 2022

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Mongoose version

6.6.1

Node.js version

18.2.0

MongoDB server version

5.x

Description

Adding a field to a Mongoose Schema with the type Schema.Types.Decimal128 breaks all the typings of model, including the ones of the other fields of the Mongoose Schema.
Make sure to use a version of Typescript < 4.8.x due to the fact that otherwise the build breaks.

Steps to Reproduce

For reference this is the tsconfig.json that I used to compile

{
    "compilerOptions": {
        "declaration": true,
        "outDir": "./build",
        "module": "CommonJS"
    },
    "exclude": [
        "node_modules",
        "build"
    ]
}
import { model, Schema } from 'mongoose';

const testSchema = new Schema({
    testDate: { type: Date }
});

const TestModel = model('Test', testSchema);

As expected TestModel has the following type

declare const TestModel: import("mongoose").Model<{
    testDate?: Date;
}, {}, {}, {}, Schema<any, import("mongoose").Model<any, any, any, any, any>, {}, {}, {}, {}, "type", {
    testDate?: Date;
}>>;

If we change our schema just a little bit, introducing Schema.Types.Decimal128 it breaks every type.

import { model, Schema } from 'mongoose';

const testSchema = new Schema({
    testDate: { type: Date },
    testDecimal: { type: Schema.Types.Decimal128 },
});

const TestModel = model('Test', testSchema);

This results in

declare const TestModel: import("mongoose").Model<{
    testDate?: {
        [Symbol.toPrimitive]?: {};
        toString: {};
        valueOf: {};
        toLocaleString: {};
        toJSON?: {};
        toDateString?: {};
        toTimeString?: {};
        toLocaleDateString?: {};
        toLocaleTimeString?: {};
        getTime?: {};
        getFullYear?: {};
        getUTCFullYear?: {};
        getMonth?: {};
        getUTCMonth?: {};
        getDate?: {};
        getUTCDate?: {};
        getDay?: {};
        getUTCDay?: {};
        getHours?: {};
        getUTCHours?: {};
        getMinutes?: {};
        getUTCMinutes?: {};
        getSeconds?: {};
        getUTCSeconds?: {};
        getMilliseconds?: {};
        getUTCMilliseconds?: {};
        getTimezoneOffset?: {};
        setTime?: {};
        setMilliseconds?: {};
        setUTCMilliseconds?: {};
        setSeconds?: {};
        setUTCSeconds?: {};
        setMinutes?: {};
        setUTCMinutes?: {};
        setHours?: {};
        setUTCHours?: {};
        setDate?: {};
        setUTCDate?: {};
        setMonth?: {};
        setUTCMonth?: {};
        setFullYear?: {};
        setUTCFullYear?: {};
        toUTCString?: {};
        toISOString?: {};
        getVarDate?: {};
    };
    testDecimal?: {
        toString: {};
        toJSON?: {};
        bytes?: {
            [x: number]: unknown;
            [Symbol.toStringTag]?: unknown;
            [Symbol.iterator]?: {};
            length?: unknown;
            toString: {};
            indexOf?: {};
            lastIndexOf?: {};
            slice?: {};
            valueOf: {};
            includes?: {};
            at?: {};
            toLocaleString: {};
            join?: {};
            every?: {};
            some?: {};
            forEach?: {};
            map?: {};
            filter?: {};
            reduce?: {};
            reduceRight?: {};
            find?: {};
            findIndex?: {};
            entries?: {};
            keys?: {};
            values?: {};
            reverse?: {};
            sort?: {};
            fill?: {};
            copyWithin?: {};
            BYTES_PER_ELEMENT?: unknown;
            buffer?: {
                [Symbol.toStringTag]?: unknown;
                slice?: {};
                byteLength?: unknown;
            } | {
                [Symbol.toStringTag]?: unknown;
                slice?: {};
                [Symbol.species]?: any;
                byteLength?: unknown;
            };
            byteLength?: unknown;
            byteOffset?: unknown;
            set?: {};
            subarray?: {};
            copy?: {};
            write?: {};
            toJSON?: {};
            equals?: {};
            compare?: {};
            writeBigInt64BE?: {};
            writeBigInt64LE?: {};
            writeBigUInt64BE?: {};
            writeBigUint64BE?: {};
            writeBigUInt64LE?: {};
            writeBigUint64LE?: {};
            writeUIntLE?: {};
            writeUintLE?: {};
            writeUIntBE?: {};
            writeUintBE?: {};
            writeIntLE?: {};
            writeIntBE?: {};
            readBigUInt64BE?: {};
            readBigUint64BE?: {};
            readBigUInt64LE?: {};
            readBigUint64LE?: {};
            readBigInt64BE?: {};
            readBigInt64LE?: {};
            readUIntLE?: {};
            readUintLE?: {};
            readUIntBE?: {};
            readUintBE?: {};
            readIntLE?: {};
            readIntBE?: {};
            readUInt8?: {};
            readUint8?: {};
            readUInt16LE?: {};
            readUint16LE?: {};
            readUInt16BE?: {};
            readUint16BE?: {};
            readUInt32LE?: {};
            readUint32LE?: {};
            readUInt32BE?: {};
            readUint32BE?: {};
            readInt8?: {};
            readInt16LE?: {};
            readInt16BE?: {};
            readInt32LE?: {};
            readInt32BE?: {};
            readFloatLE?: {};
            readFloatBE?: {};
            readDoubleLE?: {};
            readDoubleBE?: {};
            swap16?: {};
            swap32?: {};
            swap64?: {};
            writeUInt8?: {};
            writeUint8?: {};
            writeUInt16LE?: {};
            writeUint16LE?: {};
            writeUInt16BE?: {};
            writeUint16BE?: {};
            writeUInt32LE?: {};
            writeUint32LE?: {};
            writeUInt32BE?: {};
            writeUint32BE?: {};
            writeInt8?: {};
            writeInt16LE?: {};
            writeInt16BE?: {};
            writeInt32LE?: {};
            writeInt32BE?: {};
            writeFloatLE?: {};
            writeFloatBE?: {};
            writeDoubleLE?: {};
            writeDoubleBE?: {};
        };
        _bsontype?: import("mongoose").Types.Decimal128;
        inspect?: {};
    };
}, {}, {}, {}, Schema<any, import("mongoose").Model<any, any, any, any, any>, {}, {}, {}, {}, "type", {
    testDate?: Date;
    testDecimal?: import("mongoose").Types.Decimal128;
}>>;

It seems that adding the Decimal128 unpacks every type in the Schema.

Expected Behavior

It should simply be something like:

declare const TestModel: import("mongoose").Model<{
    testDate?: Date;
    testDecimal?: import("mongoose").Types.Decimal128;
}, {}, {}, {}, Schema<any, import("mongoose").Model<any, any, any, any, any>, {}, {}, {}, {}, "type", {
    testDate?: Date;
    testDecimal?: import("mongoose").Types.Decimal128;
}>>;
@IslandRhythms
Copy link
Collaborator

IslandRhythms commented Sep 14, 2022

What are you doing to print that result?

@HiiiiD
Copy link
Author

HiiiiD commented Sep 15, 2022

A simple tsc build

@hasezoey
Copy link
Collaborator

maybe related to #12352?

@HiiiiD
Copy link
Author

HiiiiD commented Sep 15, 2022

I think so

@HiiiiD
Copy link
Author

HiiiiD commented Sep 15, 2022

I've also tried digging down TS and it seems that infer breaks types; I know it's weird but if, for example, we hover over, with VSCode, the types that the Schema creates are completely fine.

@hasezoey
Copy link
Collaborator

I know it's weird but if, for example, we hover over, with VSCode, the types that the Schema creates are completely fine.

for context, what version of tsc is used to build and what version of typescript is used in vscode? (tsc --version and CTRL+P(default) -> Select Typescript version -> see which one has a "dot" infront)

VSCode typescript version example

vscode tsc version

@HiiiiD
Copy link
Author

HiiiiD commented Sep 15, 2022

I use the Workspace Version in VSCode, so it's 4.7.4 for both VSCode and tsc

@HiiiiD
Copy link
Author

HiiiiD commented Sep 15, 2022

I've also tried digging down TS and it seems that infer breaks types; I know it's weird but if, for example, we hover over, with VSCode, the types that the Schema creates are completely fine.

This is what I meant
image

@hasezoey
Copy link
Collaborator

hasezoey commented Sep 23, 2022

Note: the following is outdated information, see update at the end of comment

i investigated a bit for #12450 (comment) and found that out of whatever reason, typescript does not like having a T extends Type<infer T1, infer T2> ? { t1: T1, t2: T2 }[alias] : never, it somehow changes the type, but doing T extends Type<any, infer T2> ? T2 : never, it gives the correct type

or in more mongoose-types terms, the following will somehow change the type:

type ObtainSchemaGeneric<TSchema, alias extends 'EnforcedDocType' | 'M' | 'TInstanceMethods' | 'TQueryHelpers' | 'TVirtuals' | 'TStaticMethods' | 'TPathTypeKey' | 'DocType'> =
  TSchema extends Schema<infer EnforcedDocType, infer M, infer TInstanceMethods, infer TQueryHelpers, infer TVirtuals, infer TStaticMethods, infer TPathTypeKey, infer DocType>
    ? {
      EnforcedDocType: EnforcedDocType;
      M: M;
      TInstanceMethods: TInstanceMethods;
      TQueryHelpers: TQueryHelpers;
      TVirtuals: TVirtuals;
      TStaticMethods: TStaticMethods;
      TPathTypeKey: TPathTypeKey;
      DocType: DocType;
    }[alias]
    : unknown;

type InferSchemaType<SchemaType> = ObtainSchemaGeneric<SchemaType, 'DocType'> ;

const Schema2 = new Schema({ // correct 8th generic (DocType)
  createdAt: { type: Date, required: true },
  decimalValue: { type: Schema.Types.Decimal128, required: true }
});

const t = {} as InferSchemaType<typeof Schema2>; // somehow has a modified DocType

while the following returns the type intact:

type ObtainDocType<TSchema> =
  TSchema extends Schema<any, any, any, any, any, any, any, infer DocType>
    ? DocType
    : unknown;

type InferSchemaType<SchemaType> = ObtainDocType<SchemaType>;

const Schema2 = new Schema({ // correct 8th generic (DocType)
  createdAt: { type: Date, required: true },
  decimalValue: { type: Schema.Types.Decimal128, required: true }
});

const t = {} as InferSchemaType<typeof Schema2>; // DocType is returned without modification

@Uzlopak @vkarpov15 @IslandRhythms what do you think about that?


Update: after some more testing after #12450 (comment) it seems like infer TPathTypeKey is the problem, when set to any (instead of infer Something), then the issue is resolved, but creates other issues in other places

@hasezoey hasezoey added the typescript Types or Types-test related issue / Pull Request label Sep 23, 2022
@HiiiiD
Copy link
Author

HiiiiD commented Sep 23, 2022

I've tried to add/remove conditions to ResolvePathType in order to see where it starts to break, and apparently removing PathValueType extends Record<string, any> ? ObtainDocumentType<PathValueType, any, TypeKey> fixes my issue, but it has clearly broken tests that depend on it

I don't understand why without this condition everything works, and adding it breaks my use case

@vkarpov15 vkarpov15 added this to the 6.6.5 milestone Oct 3, 2022
@vkarpov15 vkarpov15 modified the milestones: 6.6.5, 6.6.6 Oct 5, 2022
@vkarpov15 vkarpov15 modified the milestones: 6.6.6, 6.6.7, 6.6.8 Oct 19, 2022
@vkarpov15
Copy link
Collaborator

I took a look and confirmed that this issue was fixed in 6.7.0 with #12352. I added a test case that covers the issue described in OP and it is passing.

@vkarpov15 vkarpov15 removed this from the 6.7.1 milestone Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript Types or Types-test related issue / Pull Request
Projects
None yet
Development

No branches or pull requests

4 participants