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

[Modern] Difficulty following paginationContainer example #1625

Closed
AndrewIngram opened this issue Apr 18, 2017 · 9 comments
Closed

[Modern] Difficulty following paginationContainer example #1625

AndrewIngram opened this issue Apr 18, 2017 · 9 comments

Comments

@AndrewIngram
Copy link

I'm trying to follow the paginationContainer example, but have a couple of issues:

Firstly, it won't compile:

Invariant Violation: RelayConnectionTransform: Expected type `EventConnection` to have an edges.cursor field for which the type is an scalar in document `UpcomingEvents_query`.

My EventConnection looks like this

type EventConnection {
  pageInfo: PageInfo!
  edges: [EventEdge]
}

type EventEdge {
  node: Event
  cursor: String!
}

This is unchanged from my old classic-compatible schema.

Secondly, the example specifies the type of the cursor variable as "ID", but the connection spec (as well as graphql-relay-js) defines cursors as "String"

@AndrewIngram
Copy link
Author

There's also no examples of the correct way to expose variables defined on the query in the QueryRenderer to the fragment and pagination containers, I'm assuming they'll just available, but I haven't actually managed to get it to compile yet :)

@AndrewIngram
Copy link
Author

Okay, so it looks like the issue is in RelayConnectionTransform in the validateConnectionType function. When validating cursor and pageInfo, as well as the fields on pageInfo, there are missing calls to RelaySchemaUtils.getNullableType, so it's failing when these fields are marked as required (which is what graphql-relay-js spits out). When I manually edit my schema to loosen the restriction, compilation succeeds.

@kassens
Copy link
Member

kassens commented Apr 19, 2017

This is something we should allow, feel free to send a PR to add the getNullableType if you get to it before we do!

@janicduplessis
Copy link
Contributor

Fixed the compilation issue in #1637

Managed to get everything working based on the example, just a few things I found out that are not clear:

@lxwbr
Copy link

lxwbr commented May 23, 2017

@janicduplessis should the field definition pageInfo in the query be necessary? I thought @connection is there in order to include this field into the fragment/query during compilation time. But trying it out myself, it looks like you still have to define

pageInfo {
  hasNextPage
  endCursor
}

in the fragment manually. Is that a known bug?

@chrisportela
Copy link

Yea I had to include what @Zunder did to get my pagination component working again. Very weird since I swore that once I had added the pageInfo and it broke (but maybe I just hadn't recompiled).

None of the examples seem to show that you need to include the pageInfo.

@akomm
Copy link

akomm commented Oct 10, 2017

I agree on the case, that the documentation is very hard to follow, impossible if not checking the actual implementation.

I started few month ago with graphql. Have one quite complex app implemented with react and react-apollo. I like the approach and idea of relay (I think I understood it quite well). I intend to use relay modern on my current project. I already found many things working much better for me with relay modern, although most of it via checking the implementation and checking issue tracker for the same question, not docs. I understand that it takes time to write full and good docs and it is not an easy task!

When you work through the docs, you have a huge gap from QueryRenderer and fragmentContainer to refetch & paginationContainer. It literally goes from straight-forward to confusing.

  • Suddenly you have directives and naming convention, which might have sort of logic, but are everything else than straight-forward (Feed_feed). Whilte the naming on the fragmentContainer is understandable this way, it is confusing on connections. It feels like a huge "criss-cross" of things.

  • You suddenly have fragment and query all defined for one component. Why do we need fragment in this case? edit Resolves: What's the role of the query that's passed to createPaginationContainer? #1990 (comment)

  • We do not use QueryRenderer. Where does the environment come from? I checked the code: it is expected to be passed via context. But checking via devtools I see no context passed with relay, even not in child components of QueryRenderer. edit Resolves: What's the role of the query that's passed to createPaginationContainer? #1990 (comment)

  • What if you paginate top level connection (products for example). You would have to have a fragment on Query type, or have to create a fragment on the ProductsConnection type, containing only the edge and pageInfo. This does not really work out with the example provided. I would find a way to "adjust" it, if the docs on the paginationContainer would only be more straight-forward in terms of "the concept", not just example code. So currently I try to learn via implementation or check issue posts for the same question, as the docs do not help (this worked for me so far) ;( . edit my assumption about fragment on Query type was correct: [Modern] Paginating a connection in the root query #1705 (comment)

@sibelius
Copy link
Contributor

we going to accept a pull request to improve docs on pagination container

follow this https://github.com/entria/guidelines/blob/master/relay/flatlist-relaymodern.md

if you want to do pagination with refetchContainer instead

@jstejada
Copy link
Contributor

we're hoping to simplify this API a lot soon!

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

No branches or pull requests

8 participants