-
Notifications
You must be signed in to change notification settings - Fork 222
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
changed tests to expected behavior of object pageInfo.has*Page #824
changed tests to expected behavior of object pageInfo.has*Page #824
Conversation
Hello, is there any chance you will have time to validate this issue? Any information about it would be great! 🙏 |
Hello @s3tezsky, the implementation of the bundle first follow the Relay Specs. We are not against changes but to stay compatible with Relay client, we need to reference the specs section leading to changes in this PR. |
Thanks @mcg-web for the reply. Actually my proposal shoul be OK with relay specs - in further reading I am getting to this paragraph (https://relay.dev/assets/files/connections-61fc54c286f0afc0b4f230f7c4b150bf.htm#sec-undefined.PageInfo.Fields) which may bahave the same as my expectation. That is the main reason why I am a bit confused it is not implemented that way. |
9f1bc12
to
095e7dc
Compare
Hi @mcg-web! I have pushed a commit which fixes the behavior of
According to provided relay specification do you think this proposal could be accepted? Or is there anything suboptimal from your point of view? As this change should be considered as BC break I am looking forward to see the first major version 🙏 ! Cheers! |
Any news about this ? I'm having this issue too. Would be great if it could be merged soon :) |
Up |
1 similar comment
Up |
Hello, I am really confused about object pageInfo when using pagination.
IMO the current implementation is wrong because e.g.
hasPreviousPage
is always returningfalse
(when usingfisrt
andafter
combination) even there are some items which could be listed and provided data set to pagination contains the one more previous result for it.I have walked through GraphQL specification and your documentation but unfortunatelly found no well described information about this.
The best informations are here which lead me to creating this PR.
Besides there was a PR (#31) probably with the same trouble but it was closed with no more informations about it.
I wanted to create an issue but breaking your tests to demonstrate my expectations seems much better to me.
Could you please explain to me what am I missing if it is wrong? Otherwise I would appreciate admitting it is wrong on your side and I could probably help with fixing it.
Cheers!