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

Warning for last + after #247

Closed
amccloud opened this issue Sep 6, 2015 · 8 comments
Closed

Warning for last + after #247

amccloud opened this issue Sep 6, 2015 · 8 comments
Assignees

Comments

@amccloud
Copy link

amccloud commented Sep 6, 2015

I'm getting this warning:

GraphQLRange does not currently handle retrieval for before(<cursor>).first(<count>) and after(<cursor>).last(<count>)

For this query fragment:

fragment on User {
  photos(last: $count) {
    edges {
      node {
        id,
        imageUri,
        createdAt
      }
    }
  }
}

Which appears to convert to:

query DiscoverController{node(id:"VXNlcjox"){..._0u97mo2}} fragment _0u97mo2 on User{_photosyhsn3z:photos(before:"YXJyYXljb25uZWN0aW9uOjI0",after:"YXJyYXljb25uZWN0aW9uOjI=",last:8){edges{node{id,imageUri,createdAt},cursor},pageInfo{hasNextPage,hasPreviousPage}},id}
@amccloud
Copy link
Author

amccloud commented Sep 6, 2015

I should note that first: $count works fine.

@josephsavona
Copy link
Contributor

cc @yuzhi

@yuzhi
Copy link
Contributor

yuzhi commented Sep 7, 2015

Thanks for reporting and providing such complete information.
TLDR: You can probably ignore the warning.

Looking at the query it is trying to send to the server, it has both a before and after call. That can only happen when we have a gap of unknown edges between two segment of known edges.

edgeA
edgeB
edgeC
.
.
.
(unfetched edges )
.
.
.
edgeX
edgeY
edgeZ

My best guess is you have some components fetching the first:n of this photos connection and some components fetching the last:m of this same photos connection, which generates the gap and that query. That totally within what Relay's connection manager should support.

The warning is mainly to prevent people from directly doing before/first and after/last inside components. Those two combinations are used for polling additional things off of a connection and should not be part of any component's queries. The warning is showing up because we recently updated the write path to also read out some edges for some additional logic.

@josephsavona do you think we can move that warning about doing before/first and after/last to the transform step so we don't get a lot of false positive warnings?

@josephsavona
Copy link
Contributor

do you think we can move that warning about doing before/first and after/last to the transform step so we don't get a lot of false positive warnings?

@yuzhi Awesome idea, I'm on it.

@josephsavona josephsavona self-assigned this Sep 7, 2015
@amccloud
Copy link
Author

amccloud commented Sep 7, 2015

My best guess is you have some components fetching the first:n of this photos connection and some components fetching the last:m of this same photos connection, which generates the gap and that query. That totally within what Relay's connection manager should support.

That is correct :)

@yuzhi
Copy link
Contributor

yuzhi commented Sep 7, 2015

@josephsavona I can remove the warning from GraphQLRange once you update the transform.

@josephsavona
Copy link
Contributor

@yuzhi sounds perfect

josephsavona added a commit that referenced this issue Sep 9, 2015
…rrors

Summary: Makes the use of `field(before: .., first: ..)` or after/last an error. This combination is only supported when generated by Relay during the diffing process.

Also, the previous version replaced invalid GraphQL templates with a function that throws - while I was at it, I changed this to include the actual validation error message.

Addresses #247
Closes #252

Reviewed By: @yuzhi

Differential Revision: D2419715
@yuzhi
Copy link
Contributor

yuzhi commented Sep 10, 2015

Removed it from GraphQLRange a04a514

@yuzhi yuzhi closed this as completed Sep 10, 2015
josephsavona added a commit that referenced this issue Sep 18, 2015
…rrors

Summary: Makes the use of `field(before: .., first: ..)` or after/last an error. This combination is only supported when generated by Relay during the diffing process.

Also, the previous version replaced invalid GraphQL templates with a function that throws - while I was at it, I changed this to include the actual validation error message.

Addresses #247
Closes #252

Reviewed By: @yuzhi

Differential Revision: D2419715
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

No branches or pull requests

3 participants