Cast to ObjectId failed for value \"[object Object]\" (type string) at path \"_id\" for model \"XXX\"

[chladog]

Bug Report

This is bug report for issue discussed in #911.
I managed to replicate the error in minimal project. I believe that the problem is that query/dataloader expands relationship fields when it shouldn't at depth 0 and then it tries to expand already expanded fields at proper level causing the type cast error.

Test query:

query {
  MyPosts {
    docs {
      title
      owner {
        email
      }
    }
  }
}

See comment at
payload.config.ts::75

Expected Behavior

Relationship field objects should get expanded only at their respective depth.

Current Behavior

Relationship field objects get expanded one level sooner.

Steps to Reproduce

1. Create user
2. With that user create a Post
3. While logged in send test query

Detailed Description

[email protected]

HERE IS THE REPO: https://github.com/chladog/payload-chladog-bug-replicator

[jmikrut]

Hey @chladog — unfortunately I still can't replicate. I noticed that you are probably automatically filling the owner field on your MyPosts collection with a hook or similar, so I tried to replicate. Did you mean to provide a sample repo?

I just added a test suite to reflect your exact scenario in the Payload source code, including autofilling the owner field based on the logged in user's ID:

https://github.com/payloadcms/payload/blob/master/test/dataloader/config.ts

The new test passes. And here's a screenshot of the query I am making in the test suite:

I have a sneaking suspicion that maybe however you're trying to autofill the owner field might be wrong. You may be passing the full user object, where you really just need to pass the ID of the user object. See the sample config for how I am doing that with a field hook.

Can you help me figure out how to take this test and replicate your scenario? To run it, you can:

1. Clone down Payload, making sure you are on master branch
2. yarn
3. yarn dev dataloader

And then you'll get a dev environment spun up with this exact scenario, which will allow you to play around in GraphQL Playground.

Any ideas?

[chladog]

Hi @jmikrut, ofc I wanted to share the repo - my bad.
Here it is: https://github.com/chladog/payload-chladog-bug-replicator

The hook that fills the owner field fills an ID, not a full object, as you can see in https://github.com/chladog/payload-chladog-bug-replicator/blob/master/src/collections/Posts.ts
Same goes other way around for activePost field where ID is set as well.

I also checked the Mongo directly to see that really just the "ID" is stored and it is.

[jmikrut]

Ah ok. I see exactly why this is happening now and I fully understand why.

So, when your custom resolver is fetching the results, you need to specify depth: 0 as you suspected, because otherwise you would be doing quite a bit of extra querying.

I don't think that we should change anything in Payload's source code for this for a few reasons:

1. Consider if your posts collection had a lot more relationship fields—in your local API find call, you'd be populating all relationships for no reason if your GraphQL query only wanted the owner to be populated.
2. Being that you're specifying the default depth, in this case, 2, you could have extra "levels" of depth being populated for no reason, where you really only need the first level of owner.
3. You might think that we could safely use the pre-populated document that is provided, rather than having to use the dataloader to manually fetch related documents, but the fetched documents themselves could still be the wrong shape (populated further than they should) if the remaining depth is greater than 0.

Internally, Payload find within GraphQL resolvers all specify 0 as the depth for this exact reason. Because it needs to be up to the shape of the query.

TL;DR: it's going to be more performant for you to specify depth: 0 in your custom resolver in the long run, and the error is actually proper. It will help avoid extra overhead by functioning the way it is now.

Is there a reason why you can't specify depth: 0? If so, I'm up for brainstorming ways to get around this. We could set up GraphQL resolvers to use the data if it's there, but that's kinda putting a bandaid over top of something that should really not be done in the first place IMO.

I'm gonna bump this back to a discussion for now as I don't think there's anything that should be changed at this point internally within Payload.

What do you think?

[chladog]

Yes I see that when I set depth: 0 then everything is working fine, however I'm still confused about the process as I would expect that the depth 0 means no fields except the base document itself get populated.

1. So basically depth: 0 gets exactly one level down populated, shouldn't it then be depth:1?
2. Even if depth:0 that populates one level extra is considered valid, I'll never be able to build a query with any other depth level due the mentioned error , what would be the point of 'depth' option then
3. if depth:0 is somewhat most compatible why is it not a default value for local API - when I explicitly don't set it to "0" I get an error which is quite counterintuitive.

For my use case now depth:0 is fine, the whole point is that it goes against my mental model and I was mainly afraid that sometimes this gets fixed (by the principle depth:0 = not other fields get populated) and it's gonna break my app. If the current behavior is here to stay then it's fine.