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

Add nodes field #432

Merged
merged 1 commit into from
Apr 8, 2022
Merged

Add nodes field #432

merged 1 commit into from
Apr 8, 2022

Conversation

kervin5
Copy link
Contributor

@kervin5 kervin5 commented Apr 8, 2022

There may be instances in which a nodes field is expected in a connection field of a schema. This could also be useful in scenarios in which having access to the nodes field can be a little more practical on the frontend to make the code less verbose.

With this change, I'm proposing the addition of a nodes attribute as part of the object returned by findManyCursorConnection. Making the integration with plugins such as Nexus relay connection even easier.

Thank you for all the great work put into this package!

Copy link
Member

@queicherius queicherius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate the PR! Looks good to me, going to be in the next minor release.

@queicherius queicherius merged commit bea03f3 into devoxa:master Apr 8, 2022
@queicherius
Copy link
Member

Released as 2.2.0

@jonasschultheiss
Copy link

For my usecase it's unnecessary to have both nodes and edges. can I opt out of having these nodes?

@queicherius
Copy link
Member

For my usecase it's unnecessary to have both nodes and edges. can I opt out of having these nodes?

What is your use-case, do you just pass the response back as-is? With GraphQL usually the response only consists of the defined fields, so if you don't add those to your schema the user cant request them.

@jonasschultheiss
Copy link

I use this library in various rest services. I know, that it is mainly made for graphql but this lib was recommended in an official resource.
I followed these resource when I started to implement our services:

@queicherius
Copy link
Member

I use this library in various rest services. I know, that it is mainly made for graphql but this lib was recommended in an official resource. I followed these resource when I started to implement our services:

Interesting, thank you for the context!

Would a hasNodes: false option that turns nodes into an empty array work for you? This would be the easiest for maintenance, because completely removing the nodes requires a different type based on a deep option.

Alternatively you could run delete result.nodes before returning the result, but this would require an extra wrapper function on your end.

@jonasschultheiss
Copy link

Well, yes. But it's still a bit weird to have an empty unused prop on the return type.
How about you make this a low priority issue and I'll use the delete keyword in the meantime?

This isn't a big issue and I don't want to annoy you with a rest usecase for a graphql library. I'd be happy if the deep option would be implemented in the long run.

@queicherius
Copy link
Member

Well, yes. But it's still a bit weird to have an empty unused prop on the return type.
How about you make this a low priority issue and I'll use the delete keyword in the meantime?

Sounds good, moved into an issue: #447

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants