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

You supplied the `edges` field on a connection named `...`, but you did not supply an argument necessary to do so. Use either the `find`, `first`, or `last` argument. #1201

Closed
Globegitter opened this Issue Jun 7, 2016 · 7 comments

Comments

Projects
None yet
4 participants
@Globegitter
Contributor

Globegitter commented Jun 7, 2016

What is the reason for ConnectionFields requiring this?

We have a use-case where we always want to query and display all nodes the backend returns. In this case first/list/find just feels a bit awkward as the frontend should not be aware of how many items the backend returns and otherwise we would have to write something like first: 10000.

Is there any chance this part of the spec will be relaxed?

@kassens

This comment has been minimized.

Member

kassens commented Jun 7, 2016

The reason this restriction was added is that pretty much every connection can grow large for some user. What if there are actually 10 000 objects? It would probably make the client slow and put a lot of stress on the server. If there's business logic keeping the number pretty low, you can also consider using a plural field instead of a connection.

Can you share more about your use case?

@wincent

This comment has been minimized.

Contributor

wincent commented Jun 8, 2016

I tried to find a previous issue where we discussed this previously but I couldn't (looked here and in the GraphQL repos + Stack Overflow), but I am pretty sure there is some discussion of it somewhere.

Connections pretty much only exist to enable pagination, and you generally want pagination in order to work with datasets that are too expensive to fetch in their entirety, or infeasible to process or display on the client due to the volume of data. As such, the "fetch all pages" scenario isn't really core, which is why it is possible but not necessarily beautiful (because you have to do something like first: $maximumSafeInt).

In that other issue that I can't find right now, we've recommended to do exactly that (see the TODO example, for example), or use a plural like @kassens says.

@Globegitter

This comment has been minimized.

Contributor

Globegitter commented Jun 8, 2016

@kassens In our use case the end-user does not have direct control over the objects that will be displayed for them (that is handled by certain enterprise users), so we know that the number will be relatively stable (it could grow or shrink by a few numbers though). In fact we are not showing the list of objects directly, but each object belongs to a category and we are showing it grouped by that category. So again, while the number of categories can change, they are not controlled by the end-user and we know they will stay relatively stable (at least following our current business requirements). The main reasons for me choosing ConnectionField was because the list does have some metadata on it and also what I am writing below.

@wincent We where using a list originally but after reading more about ConnectionFields (graphql/graphql-relay-js#27 (comment), #444 (comment), #444 (comment)) I wanted to try and embrace them to the fullest: e.g. not just use them for pagination but for lists that just have some metadata, like total_count or extra information about each node and also use them for lists where we do not need pagination but proper mutation support. Having started to go down that road maybe it is better to just use ConnectionFields for pagination and Lists for everything else.

I suppose it would just be great to have one clear message, e.g. Only use ConnectionFields when you need the pagination future otherwise use Lists and Relay can handle both fully. Or 'always' use ConnectionFields but they are flexible enough to support a variety of use-cases.

Slightly OT: I actually paused my work on fixing #444 because right now I tried to go down the second path (i.e. 'always' use ConnectionFields) but maybe it would just be better to have Relay properly support Lists (happy to finish up my work on that and make a PR) and people can then pick and choose what to use themselves.

That are just my thoughts on that overall topic based on my current understanding of the situation.

In any case thanks for getting back so quickly it is appreciated.

@wincent

This comment has been minimized.

Contributor

wincent commented Jun 8, 2016

@Globegitter: So, if I am understanding you correctly, you are saying that lists are like an iPad, connections are like a Mac Pro, and there is room for an iMac model somewhere in the middle. It is hard to strip away complexity from connections, but more straightforward to add power to lists, I think (the #444 approach).

@yuzhi

This comment has been minimized.

Contributor

yuzhi commented Jun 8, 2016

Quoting @dschafer

Without a limiting filter like first or last, the behavior of the connection is undefined. The server could return every item in the list (which could mean we're massively overfetching if the list grows in size), it could just return a default value; the client has no idea. Clearly, the client has some idea of the behavior that it wants: it should specify that behavior with a first: or last: filter. If you want up to N items, do (first: N). If you want all items, ask yourself: if there were 1,000,000 items in the future, would you want to fetch all of them? The answer is clearly no; you would want to cap it at some reasonable limit X. In that case, do (first: X).

@wincent

This comment has been minimized.

Contributor

wincent commented Jun 8, 2016

@yuzhi: That's the comment I was looking for!

@Globegitter

This comment has been minimized.

Contributor

Globegitter commented Jun 8, 2016

@wincent @yuzhi Thank you for the comments and yes I think the iPad/Mac Pro/iMac analogy is quite spot on.

I can see the point that @dschafer is making and that clarifies the ConnectionField use-case fully and this can be closed. I would agree then that it makes most sense to add more power to lists, which then gives users full flexibility. Will try and finish the work on that soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment