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

Remove dependency on Pagy #362

Closed
adrianna-chang-shopify opened this issue Mar 3, 2021 · 2 comments · Fixed by #371
Closed

Remove dependency on Pagy #362

adrianna-chang-shopify opened this issue Mar 3, 2021 · 2 comments · Fixed by #371
Assignees

Comments

@adrianna-chang-shopify
Copy link
Contributor

Pagy is a nice, simple pagination gem that we turned to for paginating previous runs. However, it uses offset, which is prohibited in Shopify Core:
Screen Shot 2021-03-03 at 2 48 46 PM

and just generally won't scale well. Plus, most apps don't really need paginated runs since they'll just run a Task once. It might be worthwhile to investigate removing Pagy and implementing some sort of value-based pagination, or even dropping pagination and allowing users to load more data on their page with a button click at the bottom of their page.

@simonlevasseur
Copy link

I saw your linked PR and it seems you have a pretty good handle on things but wanted to link a similar issue we had in Partners. We analyzed the performance implications of value-based pagination vs offset-based pagination. Not only is the performance better but it's also more reliable as documents change.

See https://github.com/Shopify/partners/pull/23097

@adrianna-chang-shopify
Copy link
Contributor Author

That's an awesome writeup, thanks for sharing @simonlevasseur ! ❤️ The more I read up on the two, the more apparent it became that we should make the switch, but it's really neat to see the performance implications in an actual production application.

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