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

[ApolloPagination] Additional conveniences for offset pagination #267

Closed
wants to merge 1 commit into from

Conversation

Iron-Ham
Copy link
Contributor

@Iron-Ham Iron-Ham commented Feb 14, 2024

While writing docs in #262, I recognized that support for offset pagination could be better. Specifically:

  • We could support bidirectional and reverse pagination
  • We could have additional convenience methods for that pagination type

As such, this pull request modifies the OffsetPagination class to be a namespace, just like CursorBasedPagination. We introduce three new structs to facilitate pagination:

  • OffsetPagination.Forward: Introduced OffsetPagination.Forward structure to represent a forward pagination state. This is identical to the previous contents of OffsetPagination.
  • OffsetPagination.Reverse: Introduced OffsetPagination.Reverse structure to represent a reverse pagination state.
  • OffsetPagination.Bidirectional: Introduced OffsetPagination.Bidirectional structure to represent a bidirectional pagination state.

With the addition of these new structures, we could now add and update convenience make functions for offset pagination, which add support for Bidirectional pagination – as well as add additional transform options, supporting both single-model output and Collection output.


cc: @AnthonyMDev I'm glad that you pointed out the need for offset pagination docs. I've focused almost entirely on cursors, as that's what we use at GitHub. Once this is merged, I'll look into:

  • Updating the docs to accommodate for these changes
  • Introducing a test suite around offsets, as I realize that our existing test suite is similarly focused on cursors.
  • I could additionally implement convenience functions for multi-query pagination using offsets, but I'm a bit hesitant. The number of convenience make functions is quite large, and I'm concerned that it would be confusing for users to parse through them. Yes, they could filter by typing: make followed by their direction Forward, and their pagination type Offset or Cursor, which will then present them with a further six possibilities: (single-query, single-query collection transform, single-query single-model transform, multi-query, multi-query collection transform, multi-query single-model transform). Perhaps for now, it's best to add it and then re-think the way that convenience initializers are exposed. Perhaps we can do something silly and make use of namespaces? Imagine a case-less CursorPager and OffsetPager, with further namespaces for direction (Forward, Reverse, and Bidirectional, that then had the specific convenience functions available? This can be an improvement for later.

@Iron-Ham Iron-Ham requested a review from a team as a code owner February 14, 2024 15:21
Copy link

netlify bot commented Feb 14, 2024

👷 Deploy request for eclectic-pie-88a2ba pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit fe2de1a

Copy link
Contributor

@AnthonyMDev AnthonyMDev left a comment

Choose a reason for hiding this comment

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

Approved with the caveat that we are going to remove a bunch of the convenience make functions as we discussed privately.

@Iron-Ham
Copy link
Contributor Author

Closing in favor of #268

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