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

Typescript: Can't represent newly-constructed document as "populated" #12233

Closed
2 tasks done
ajwootto opened this issue Aug 8, 2022 · 5 comments · Fixed by #12341
Closed
2 tasks done

Typescript: Can't represent newly-constructed document as "populated" #12233

ajwootto opened this issue Aug 8, 2022 · 5 comments · Fixed by #12341
Labels
typescript Types or Types-test related issue / Pull Request
Milestone

Comments

@ajwootto
Copy link

ajwootto commented Aug 8, 2022

Prerequisites

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

🚀 Feature Proposal

related to #11758
When constructing a new document, there currently isn't a way (that I can find) to tell Mongoose that I want the document to be represented as "populated". This presents a problem when constructing a new one using related documents:

import { Schema, model, connect, Types } from 'mongoose'

interface NestedChild {
    name: string
    _id: Types.ObjectId
}
const nestedChildSchema: Schema = new Schema({ name: String })

interface Parent {
    nestedChild: Types.ObjectId
    name?: string
}

const ParentModel = model<Parent>('Parent', new Schema({
    nestedChild: { type: Schema.Types.ObjectId, ref: 'NestedChild' },
    name: String
}))

const NestedChildModel = model<NestedChild>('NestedChild', nestedChildSchema)

async function run() {
    await connect('mongodb://localhost:27017/')
    const newParent = new ParentModel({
        name: 'Parent'
    })

    // doesnt work, model instance requires assignment to object id
    newParent.nestedChild = new NestedChildModel({ name: 'NestedChild' })

    // can't use assertPopulated because the internal $populated field isn't set
    // this will throw
    const asserted = newParent.$assertPopulated<{nestedChild: NestedChild}>('nestedChild')

    // instead, sketchy no-op populate call to make type system happy
    const populated = await newParent.populate<{nestedChild: NestedChild}>('nestedChild')

    // now I can assign the instance
    populated.nestedChild = new NestedChildModel({ name: 'NestedChild' })
}

run()

Basically the $assertPopulated method that was recently added doesn't help in this case, because the document isn't actually "populated" technically so the internal $populated field is false.

A workaround I found is to basically perform a no-op "populate" call to receive the new type that contains the populated field.

Should there be some kind of built-in way to represent this case without workarounds? Could $assertPopulated just proceed if the document is "new" and the field being asserted on hasn't been modified yet? (since it then has the "potential" to be populated)

Motivation

I'm writing an API that creates new documents and returns them in populated form. Currently I don't have a way of constructing a populated document from scratch and having it be typed correctly.

Example

The easiest way I see would just be to make $assertPopulated not throw when the document is new:

import { Schema, model, connect, Types, MergeType } from 'mongoose'

interface NestedChild {
    name: string
    _id: Types.ObjectId
}
const nestedChildSchema: Schema = new Schema({ name: String })

interface Parent {
    nestedChild: Types.ObjectId
    name?: string
}

const ParentModel = model<Parent>('Parent', new Schema({
    nestedChild: { type: Schema.Types.ObjectId, ref: 'NestedChild' },
    name: String
}))

const NestedChildModel = model<NestedChild>('NestedChild', nestedChildSchema)

async function run() {
    await connect('mongodb://localhost:27017/')
    const newParent = new ParentModel({
        name: 'Parent'
    })

    // doesnt work, model instance requires assignment to object id
    newParent.nestedChild = new NestedChildModel({ name: 'NestedChild' })

    // this call succeeds because nothing has been set on the populated field yet, it's still possible for it to be "populated" by assignment later on
    const asserted = newParent.$assertPopulated<{nestedChild: NestedChild}>('nestedChild')

    // now I can assign the instance
    asserted.nestedChild = new NestedChildModel({ name: 'NestedChild' })
}

run()
@ajwootto ajwootto added enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature new feature This change adds new functionality, like a new method or class labels Aug 8, 2022
@vkarpov15 vkarpov15 removed new feature This change adds new functionality, like a new method or class enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature labels Aug 11, 2022
@vkarpov15 vkarpov15 added this to the 6.5.4 milestone Aug 11, 2022
@IslandRhythms IslandRhythms added the typescript Types or Types-test related issue / Pull Request label Aug 24, 2022
vkarpov15 added a commit that referenced this issue Aug 27, 2022
@vkarpov15
Copy link
Collaborator

@ajwootto please take a look at #12341, with that you'll be able to do the following.

newParent.$assertPopulated<{nestedChild: NestedChild}>('nestedChild', {
  nestedChild: new NestedChildModel({ name: 'NestedChild' })
})

As an alternative workaround that works today, you can do the following.

newParent.set(nestedChild, new NestedChildModel({ name: 'NestedChild' }));

newParent.$assertPopulated<{ nestedChild: NestedChildModel }>('nestedChild');

@ajwootto
Copy link
Author

@vkarpov15 I think I can make that work. In our code we tend to initialize the document first and then assign all the various fields to it later on before finally saving it. In this case, I imagine I'd be able to do something like this:

const newChild = new NestedChild()
const newParent = (new Parent()).$assertPopulated<{nestedChild: NestedChild}>('nestedChild', {
  nestedChild: newChild
})
nestedChild.name = 'some name'
newParent.save()

So the associated child document would be modifyable later on in the code.
Something that just occurred to me though is could I just do this:

const newChild = new NestedChild()
const newParent = (new Parent({nestedChild: newChild})).$assertPopulated<{nestedChild: NestedChild}>('nestedChild')
nestedChild.name = 'some name'
newParent.save()

And would $assertPopulated not throw there since the document has a child document assigned to that field? Or does that still not set the internal $populated flag?

@vkarpov15
Copy link
Collaborator

Yeah $assertPopulated() would work fine there too. The problem is just with newParent.nestedChild = new NestedChildModel({ name: 'NestedChild' }). new Parent({ nestedChild: new NestedChildModel({ ... }) }) and newParent.set('nestedChild', new NestedChildModel({ ... })) both work fine.

@ajwootto
Copy link
Author

Cool.

I generally want to avoid using .set because it bypasses the type safety of assigning that field, though I suppose that's also true with passing it via the constructor.

vkarpov15 added a commit that referenced this issue Aug 30, 2022
fix(document): allow calling `$assertPopulated()` with values to better support manual population
@vkarpov15
Copy link
Collaborator

Mongoose does type casting, so the "type safety" of = is often unnecessary and sometimes incorrect, so that's why set() allows any type. Sometimes it's convenient to bypass the type checks.

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
3 participants