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

SchemaTypeOptions.get() is typed incorrectly #11561

Closed
brianlenz opened this issue Mar 23, 2022 · 1 comment
Closed

SchemaTypeOptions.get() is typed incorrectly #11561

brianlenz opened this issue Mar 23, 2022 · 1 comment
Labels
typescript Types or Types-test related issue / Pull Request
Milestone

Comments

@brianlenz
Copy link

brianlenz commented Mar 23, 2022

Do you want to request a feature or report a bug?
Report a bug.

What is the current behavior?
The Typescript type for SchemaTypeOptions.get() may be incorrect. Specifically this:

get?: (value: T, doc?: this) => any;

If the current behavior is a bug, please provide the steps to reproduce.
Consider this schema definition for using Decimal128 represented as a BigNumber (via bignumber.js):

import mongoose from "mongoose";
import BigNumber from "bignumber.js";

const { Schema } = mongoose;

const decimal128Schema: mongoose.SchemaDefinitionProperty<BigNumber> = {
  type: Schema.Types.Decimal128,
  get: (val: any): BigNumber | undefined => {
    return val ? new BigNumber(val.toString()) : undefined;
  },
  set: (val: BigNumber): mongoose.Types.Decimal128 | undefined => {
    if (!val) {
      return undefined;
    }
    return new mongoose.Types.Decimal128(val.toString());
  },
};

This code compiles just fine, but the val arg for get is typed with any, which isn't type-safe. I want it to be type-safe, so I inspected type of val, which is Decimal128 (the type stored in MongoDB). That makes total sense, so I updated the type accordingly:

import mongoose, { Decimal128 } from "mongoose";
import BigNumber from "bignumber.js";

const { Schema } = mongoose;

const decimal128Schema: mongoose.SchemaDefinitionProperty<BigNumber> = {
  type: Schema.Types.Decimal128,
  get: (val: Decimal128): BigNumber | undefined => {
    return val ? new BigNumber(val.toString()) : undefined;
  },
  set: (val: BigNumber): mongoose.Types.Decimal128 | undefined => {
    if (!val) {
      return undefined;
    }
    return new mongoose.Types.Decimal128(val.toString());
  },
};

With that change, it no longer compiles:

error TS2322: Type '{ type: typeof mongoose.Schema.Types.Decimal128; get: (val: Decimal128) => BigNumber | undefined; set: (val: BigNumber) => mongoose.Types.Decimal128 | undefined; }' is not assignable to type 'SchemaDefinitionProperty<BigNumber>'.
  Types of property 'get' are incompatible.
    Type '(val: Decimal128) => BigNumber | undefined' is not assignable to type '((getter: (value: any) => any) => void) | (<K extends keyof SchemaOptions>(key: K) => SchemaOptions[K]) | ((value: BigNumber, doc?: SchemaTypeOptions<BigNumber> | undefined) => any) | undefined'.
      Type '(val: Decimal128) => BigNumber | undefined' is not assignable to type '(getter: (value: any) => any) => void'.
        Types of parameters 'val' and 'getter' are incompatible.
          Type '(value: any) => any' is missing the following properties from type 'SchemaType': OptionsConstructor, cast, default, get, and 16 more.

The only way to get it to compile is to use unsafe any or to change the type to BigNumber, which isn't accurate. The object passed in is a Decimal128.

tsconfig.json

{
  "compilerOptions": {
    "composite": true,
    "target": "es2020",
    "module": "es2022",
    "rootDir": "src",
    "moduleResolution": "node",
    "declaration": true,
    "sourceMap": true,
    "outDir": "dist",
    "allowSyntheticDefaultImports": true,
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
    "strict": true,
    "strictNullChecks": true,
    "strictPropertyInitialization": true,
    "skipLibCheck": true
  },
  "include": [
    "src/**/*.ts"
  ]
}

What is the expected behavior?

I believe the type should be this:

get?: (value: any, doc?: this) => T;

As I understand it, the get function should take the value stored in MongoDB (the value) and return the value per the type defined in the schema, so it seems the type of value and the return type are inverted.

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.
Node: 17.8.0
Mongoose: 6.2.8
MongoDB: 4.3.1

@IslandRhythms IslandRhythms added the typescript Types or Types-test related issue / Pull Request label Mar 28, 2022
@vkarpov15 vkarpov15 added this to the 6.2.13 milestone Apr 7, 2022
@brianlenz
Copy link
Author

@vkarpov15 thanks for fixing this issue! I wonder if my suggestion wasn't 100% correct, though. I wonder if the return type actually needs to be T | undefined so that undefined is a valid return type? As written now, you can only return an object of type T, which makes optional fields unsupported 🤦‍♂️

Sorry for not realizing this in my initial suggestion!

@vkarpov15 vkarpov15 reopened this Jun 5, 2022
@vkarpov15 vkarpov15 modified the milestones: 6.3.2, 6.3.6 Jun 5, 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

3 participants