-
Notifications
You must be signed in to change notification settings - Fork 274
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
Clarify connections with naming, count field, and convenience field #3
Conversation
var array = await Promise.all(promisedArray); | ||
return { | ||
...connectionFromArray(array, args), | ||
totalCount: array.length |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is array.length
here the result of a first
limit? Or is this the info derived from the swapi API request that tells you total count?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for the case where we're converting a list of URLs into a connection, so we're just counting the URLs here.
facebook/relay#185 allows Relay to detect usages of fields like this, and suggest using |
👍 |
@@ -90,17 +117,17 @@ function rootConnection(name, swapiType) { | |||
var rootType = new GraphQLObjectType({ | |||
name: 'Root', | |||
fields: () => ({ | |||
allFilms: rootConnection('Films', 'films'), | |||
filmConnection: rootConnection('Films', 'films'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious what @schrockn thinks about this case. To me, the "xConnection" naming scheme seems less clear here, as a connection implies a source node, and these are just paginateable lists from the query's root. I think allX
is actually clearer in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I could go either way on this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
although allFilmsConnection is strictly more consistent I think I prefer allFilms
This PR is excellent. Thanks. |
445825e
to
1365809
Compare
The connections in this repo are designed to allow efficient pagination by associating cursors with each item in a connection: ```graphql { allPeople { edges { cursor node { name } } } } ``` When quickly exploring the API, though, it can be useful to have a shortcut to just get the list of objects: ```graphql { allPeople { people { name } } } ``` While this shortcut would not allow for the efficient cursor-based pagination that Relay and other clients might implement, it's quite convinient for browsing the API. To clarify naming once that shortcut is added, each field that returns a connection is renamed: ```graphql { personConnection { people { name } } } ``` Finally, it's useful to have a total count of the number of items in each connections, so that field is added: ```graphql { personConnection(first: 5) { totalCount people { name } } } ```
1365809
to
2e11018
Compare
Clarify connections with naming, count field, and convenience field
[FP-1796 & FP-1709] finished filter / extension work on graphql swapi
The connections in this repo are designed to allow efficient pagination
by associating cursors with each item in a connection:
When quickly exploring the API, though, it can be useful to have a shortcut
to just get the list of objects:
While this shortcut would not allow for the efficient cursor-based pagination
that Relay and other clients might implement, it's quite convenient for browsing
the API.
To clarify naming once that shortcut is added, each field that returns a
connection is renamed:
Finally, it's useful to have a total count of the number of items in each
connections, so that field is added: