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

Don't set startCursor in relayStylePagination when only one cursor is present #10982

Merged

Conversation

sdeleur-sc
Copy link
Contributor

@sdeleur-sc sdeleur-sc commented Jun 15, 2023

In relayStylePagination, the startCursor and endCursor are set based on cursors under the edges if the information is not present under the pageInfo. This works well when both of the cursors are defined under the nodes. However, if only a single cursor is present, both of the fields get populated with the same cursor, causing startCursor and endCursor to have the same value.

This causes an issue on subsequent merge operation because the startCursor gets written to the first edge here, causing duplicate edges to have the same cursor

The expected behavior here is for only the endCursor to be populated and for startCursor to remain empty when only a single cursor is present under the edges.

Checklist:

  • If this PR contains changes to the library itself (not necessary for e.g. docs updates), please include a changeset (see CONTRIBUTING.md)
  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

@apollo-cla
Copy link

@sdeleur-sc: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@netlify
Copy link

netlify bot commented Jun 15, 2023

👷 Deploy request for apollo-client-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 9090b82

@changeset-bot
Copy link

changeset-bot bot commented Jun 15, 2023

🦋 Changeset detected

Latest commit: 9090b82

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@apollo/client Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Member

@jerelmiller jerelmiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 🔥 This looks great to me and the logic is sound. Great to see you were able to make this change in a way that preserved all the existing logic.

Thanks so much for the contribution!

.changeset/shiny-books-roll.md Outdated Show resolved Hide resolved
@jerelmiller jerelmiller merged commit b9be7a8 into apollographql:main Jun 15, 2023
@github-actions github-actions bot mentioned this pull request Jun 15, 2023
@jerelmiller
Copy link
Member

/release:pr

@github-actions
Copy link
Contributor

Please add a changeset via npx changeset before attempting a snapshot release.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants