mickhansen / graphql-sequelize

GraphQL & Relay for MySQL & Postgres via Sequelize
MIT License
1.9k stars 172 forks source link

Changing relay edges to be nonNull causes a node error #717

Closed intellix closed 1 year ago

intellix commented 1 year ago

Was just trying to debug why I couldn't set my edges to be notNull (so I can make a strictNulls project less defensive from generated types).

After breakpointing through the code, I noticed it was this function: https://github.com/mickhansen/graphql-sequelize/blob/578478e9d75e1a65b439b491a9eba8c0acf16b23/src/relay.js#L119-L121

I believe when you go from:

edges: [UserEdge]

To:

edges: [UserEdge!]!

It's essentially wrapping the types in GraphQLNonNull so you have to go deeper:

connectionType._fields.edges.type.ofType.ofType.ofType._fields.node.type

So I guess it needs to search down the types better in case of non-nulls

intellix commented 1 year ago

Would you hate me for changing that to this? Can't think of anything better/easier right now:

function nodeType(connectionType) {
  return connectionType._fields.edges.type.ofType._fields?.node.type ??
    connectionType._fields.edges.type.ofType.ofType._fields?.node.type ??
    connectionType._fields.edges.type.ofType.ofType.ofType._fields?.node.type;
}

Then it supports all three levels:

edges: [UserEdge]
edges: [UserEdge!]
edges: [UserEdge!]!
mickhansen commented 1 year ago

@intellix Might want to verify that the ofType parent is a list or nonNull, but otherwise sounds good to me.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.