graphql-compose-connection icon indicating copy to clipboard operation
graphql-compose-connection copied to clipboard

Projection of the `edges` field on wrapped type

Open yoadsn opened this issue 4 years ago • 6 comments

@nodkz Funny - we talked about this already back in 2016 :) So now that the mongoose composter flattens the projection to improve performance - it also flattens the non required fields from the connection structure.

This means that for schema Post { title } and User { posts } where posts is a connection you would get for a query: { user { posts { edges { node { title } } } } }

This projection on mongodb "posts" collection find: { 'edges.node.title': true, title: true }

Also, on the "users" collection you can expect this find projection: {'posts.edges.nodes.title': true }

This used to be a small problem since there was no "flattening" so "edges" would be the only wrong field used. now it's much more. The more fields are used from the connected type the more duplication exists. I am not sure if mongodb has any performance implications but it might.

Since i know for sure my schema is not using "edges" and "node" fields on internal types - I wanted to remove this. I thought of working around this in multiple ways:

  • Wrapping the resolver for the "find user" and clearing the "posts" field from the projection sent to the resolver. (of course working on a copy of RP). This works, and I can think of a way to build a generic wrapper.
  • Wrapping findMany of the connection is not possible since there is only way to provide customization opts to connection resolver factory.
  • No customization to connection resolver factory allows the ability to "let the connection" know those fields are "OK" to be removed before using them on the findMany resolver. For example I would make up a parameter like allowEdgeNodeFieldsInProjection which by default is off.

What do you think - I revisited all the code involved but could not find a very clean way to let the projection ignore those fields - it must be the responsibility of the "connection" plugin I feel.

yoadsn avatar May 17 '21 11:05 yoadsn

I completely agree about the responsibility of the "connection" plugin. And I think that we can remove edges key from projection. If somebody has edges fields in Mongoose models then they will be provided in node projection.

@yoadsn can you try 8.1.1-alpha.1 version? If it works well for you then I merge it with master and publish a stable package.

nodkz avatar May 18 '21 06:05 nodkz

You may copy-paste https://unpkg.com/browse/[email protected]/lib/connection.js to your node_modules for fast testing.

nodkz avatar May 18 '21 06:05 nodkz

Thanks @nodkz The above fix seems to solve it for the internal connection findMany. I think it's a great improvement.

Of course, the extra fields on the top level access (in my example "users" collection) still had all the "posts.edgest.*" flattened projections.

As I see it a type (User) was added a relation (to [Post] on the fields "posts") using a "connection" resolver which took care of the projection cleanup all the levels below it. Now, I think that the addRelation which better knows that "posts" is a connection compared to all other resolvers on the "User" level - would need to make sure the projection of parent excludes "edges.*" when the "User" mongodb resolver is executed. This is most likely NOT the responsibility of this plugin since it only provides the resolver and down. not up.

Where should such cleanup be if we built it? I don't know. Maybe the same way addRelation specifies "projection" to force parent to include some fields it could specify "excludeProjection" to prevent parent from loading some field. 😵

yoadsn avatar May 18 '21 14:05 yoadsn

@nodkz Should we at least close the fix on this plugin since it's already providing great benefit as-is?

yoadsn avatar May 20 '21 14:05 yoadsn

@yoadsn can you provide a repro case of your problem? You may use this sandbox https://codesandbox.io/s/github/yurtaev/graphql-compose-mongoose-codesandbox/tree/master/?file=/index.js

PS. A new version will be published in minutes.

nodkz avatar May 25 '21 13:05 nodkz

@nodkz Thank you so much! (This sandbox is so convenient).

Here is a very small repro using the User and Post schemas I mentioned in my original example. https://codesandbox.io/s/compassionate-christian-21qge?file=/index.js When running this query:

query example {
        users(limit: 1) {
          name
          posts {
            edges {
              node {
                title
              }
            }
          }
        }
      }

The queries on the DB are:

Mongoose: users.find({}, { limit: 1, projection: { name: true, 'posts.edges.node.title': true, _id: true }})
Mongoose: posts.find({ author: ObjectId("a00000000000000000000000") }, { limit: 21, sort: { _id: -1 }, projection:{ 'edges.node.title': true, title: true, _id: true }})

And I was referring to the 'posts.edges.node.title': true projection on the users collection.

yoadsn avatar May 26 '21 12:05 yoadsn