Remove FlattenMaps due to issues with private and protected fields

[thaoula]

Prerequisites

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

Mongoose version

7.3.0

Node.js version

16

MongoDB server version

v18.16.0

Typescript version (if applicable)

5.1.3

Description

We have recently upgrade Mongoose from v7.0.3 -> v7.3.0 and noticed that calls to lean are now returning the Model type wrapped with FlattenMaps.

For example we we previously had a model typed in the following way -

// Model
const document: Model<Quote>;

// Example Repository Usage
public async getById(id: string): Promise<Quote> {
     const quote = await this.document .findById(id).lean().exec();

     // This used to be of type Quote but now FlattenMaps<Quote> and causes a Typescript error
     return quote;
}

Since v7.3.0 we have over 100 typescript errors complaining -

// Quote her is just an example it can be replaced with T
Type '(FlattenMaps<Quote> & Required<{ _id: string; }>)[]' is not assignable to type 'Quote[]' 

To enable us to build we to do the following -

// Model
const document: Model<Quote>;

// Example Repository Usage
public async getById(id: string): Promise<Quote> {
     const quote = await this.document .findById(id).lean<Quote>().exec();

     // This would be of type Quote now
     return quote;
}

Steps to Reproduce

Please see description above

Expected Behavior

The expected behaviour is that calls to to lean with be type to the T used by model unless overridden like I have done in my example above.

[vkarpov15]

What does your Quote type look like - do you use maps? This change was intentional because lean() means Mongoose won't convert maps. Your map types will still be POJOs.

[thaoula]

Hi @vkarpov15 ,

As always, really appreciate your prompt replies.

What do you mean by maps in this context?

Our Quote is a complex object a gist of the Typescript interface definition is below -

export interface Quote extends Entity, QuotePricingOptions, AuditDetails {

    status?: QuoteSimpleStatus;
    quoteNumber: string;

    sections?: ItemSection[];
    theme?: Entity;
    terms?: string;
    links?: EntityLink[];
    date: Date;
    expiryDate: Date;

    note?: string;
    description: string;

    account?: Contact;
    company?: Entity;
    owner?: Entity;
    address?: Address;
    contact?: ContactSummary;
    customerQO?: string;

    totalBuy?: number;
    totalSellEx?: number;
    totalAdjustment?: number;
    totalTax?: number;
    totalSellInc?: number;

    prepaymentEx?: number;
    prepaymentTax?: number;
    prepaymentInc?: number;

    overridePrepaymentEx?: number;
    overridePrepaymentInc?: number;
    overridePrepaymentTax?: number;

    source?: Source;
    imported?: boolean;

    integrationId?: string;
    budget?: JobBudget;

    customerRef?: string;

    approvalDate?: Date;
    acceptedDate?: Date;
    sentDate?: Date;

    signature?: string;
    poAttachments?: Attachment[];
    poNumber?: string;

    approver?: Entity;
    acceptor?: Entity;

    policy?: QuotePolicy;
    version?: number;
}

We have a schema typed to the above interface and we having been using calls to .lean() or aggregations to get our data as POJO in that shape.

[vkarpov15]

JavaScript's Map class. In theory, FlattenMaps<T> should be exactly equal to T unless there's a Map somewhere in your interface. Can you please double check whether you have any Maps in your document interface?

Also, does TypeScript give you any more hints about why exactly Type '(FlattenMaps<Quote> & Required<{ _id: string; }>)[]' is not assignable to type 'Quote[]' ? Does TS indicate what properties are incompatible?

[thaoula]

Hi @vkarpov15,

We do not use Javascript Maps in interfaces.

It seems that the issue may be related to a specific type call Lex. This is a LexoRank object (Jira style ordering) or string. export type Lex = LexoRank | string;

The Mongoose schema is set to String. The LexoRank object will serialize to string and were not having any issues using it with Mongoose before.

Here is a full typescript error for Quote.

Type '(FlattenMaps<Quote> & Required<{ _id: string; }>)[]' is not assignable to type 'Quote[]'.
  Type 'FlattenMaps<Quote> & Required<{ _id: string; }>' is not assignable to type 'Quote'.
    The types of 'policy.prepayments.rules' are incompatible between these types.
      Type 'FlattenMaps<QuotePolicyPrepaymentRule>[]' is not assignable to type 'QuotePolicyPrepaymentRule[]'.
        Type 'FlattenMaps<QuotePolicyPrepaymentRule>' is not assignable to type 'QuotePolicyPrepaymentRule'.
          Types of property 'sortOrder' are incompatible.
            Type 'FlattenMaps<LexoRank>' is not assignable to type 'LexoRank'.ts

Here is another model called OpportunityView -

Type 'FlattenMaps<OpportunityView> & Required<{ _id: string; }>' is not assignable to type 'OpportunityView'.
  Types of property 'list' are incompatible.
    Type 'FlattenMaps<OpportunityViewListField>[]' is not assignable to type 'OpportunityViewListField[]'.
      Type 'FlattenMaps<OpportunityViewListField>' is not assignable to type 'OpportunityViewListField'.
        Types of property 'sortOrder' are incompatible.
          Type 'string | FlattenMaps<LexoRank>' is not assignable to type 'Lex'

I hope this helps Regards, Tarek

[vkarpov15]

What does the LexoRank type look like?

[thaoula]

LexoRank is a class with methods and has on two class level fields. Below is a gist of the class and I have removed the body of most methods. We are expecting the toString value to persist to the database.

export class LexoRank {

    constructor(
        readonly value: string,
        readonly bucket = '0'
    ) {

        if (!LexoRank.isValidLexValue(value)) {
            throw `Invalid lex value "${value}"`;
        }

        if (!LexoRank.isValidLexBucket(bucket)) {
            throw `Invalid lex bucket "${bucket}"`;
        }

        this.value = value;
        this.bucket = bucket;

    }

    public toString() {
        return `${this.bucket}|${this.value}`;
    }

    public static first(): Lex;
    public static from(lex: Lex);
    public static nextBucket(bucket: string);
    public static prevBucket(bucket: string);
    public static between(lexBefore: Lex | null | undefined, lexAfter: Lex): LexoRank;
    public static between(lexBefore: Lex, lexAfter: Lex | null | undefined): LexoRank;
    public static between(lexBefore: Lex, lexAfter: Lex): LexoRank;
    public lessThan(lex: Lex) ;
    public increment();
    public decrement();

    private hasNonZeroLeadingChars() ;
    private append(str: string);
    private static parse(lex: string);
    private static isValidLexValue(value: string);
    private static isValidLexBucket(bucket: string);
    private static cleanTrailingZeros(str: string);
    private static incrementChar(char: String) ;
    private static decrementChar(char: String);
}

[vkarpov15]

I suspect the issue is the private fields. It doesn't look like TypeScript can iterate over private fields when creating a copy of a class. For example, the following script fails to compile with a Property 'append' is missing in type 'FlattenMaps<LexoRank>' but required in type 'LexoRank'. error:

import { FlattenMaps } from 'mongoose';

export class LexoRank {
    
    constructor(
        readonly value: string,
        readonly bucket = '0'
    ) { 

        this.value = value;
        this.bucket = bucket;

    }
    
    public toString() {
        return `${this.bucket}|${this.value}`;
    }

    private append(str: string) {}
}

type T1 = (FlattenMaps<LexoRank>) | string;

const X: T1 = new LexoRank('foo', '0');
const X2: LexoRank = X;

We're investigating this and looking for a workaround.

[vkarpov15]

Realistically, there's no good way for Mongoose to support classes with private members with FlattenMaps. private and protected fields don't show up in keyof, so there's no way for Mongoose to omit fields from types that have private fields without losing the private fields.

We should consider removing FlattenMaps in our next backwards breaking release, and any other places where we use a recursive keyof pattern, in favor of encouraging people to not define document interfaces with maps. The idea that FlattenMaps just breaks when private or protected fields are involved in a hard-to-debug way is concerning.

@hasezoey @AbdelrahmanHafez what do you think? Keep FlattenMaps or remove?

[hasezoey]

what do you think? Keep FlattenMaps or remove?

from my understanding FlattenMaps is about mapping the ts type Map<key, val> to a Record<key, val>(plain object), which is representative of what gets returned, is this understanding correct?

also from what i can tell, typescript is correct, you cannot assign a object that looks like LexoRank to a field expecting a instance of a class and .lean does not return a instance of anything, it just returns a plain object (POJO)

so there is either we let FlattenMaps stay and have a more correct type1 and the user has to create a POJO interface, or we remove FlattenMaps and return a more inaccurate type2, but the user does not have to deal with it

• *1: the current FlattenMaps as used in .lean does not remove functions / methods, which is inaccurate (see example code
• *2: when FlattenMaps is removed the type would say that at that path it is actually a instance of the class, which it is not and would say that a somePath: Map is actually a Map, which it is not (it is a Record)

some example code

// NodeJS: 20.2.0
// MongoDB: 5.0 (Docker)
// Typescript 4.9.5
import * as mongoose from 'mongoose'; // [email protected]

class SomeClass {
  public test!: string;

  public method() {}

  // private testprop!: string;
  // private pmethod() {}
}

interface Test {
  class: SomeClass;
}

interface TestPOJO {
  class: Pick<SomeClass, keyof SomeClass>;
}

// function is expected to return a "Test" interface with a instance of "SomeClass" at "class"
function test(): Test /* TestPOJO */ {
  // using undefined just as a placeholder for types
  const t = undefined as any as mongoose.FlattenMaps<Test>;

  t.class.method(); // type does not error, but is incorrect and does not work at runtime in .lean

  return t; // return works without protected or private fields or methods
}

uncommenting either private field will result in Property 'nameOfProperty' is missing in type 'FlattenMaps<SomeClass>' but required in type 'SomeClass'., though replacing Test return type with TestPOJO fixes it somewhat (but still methods are present, which is still incorrect)