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

Made it possible for totalCount to be a promise #778

Merged
merged 2 commits into from
Nov 10, 2020
Merged

Made it possible for totalCount to be a promise #778

merged 2 commits into from
Nov 10, 2020

Conversation

rsolyanik
Copy link
Contributor

An error occurs when trying to return a promise to totalCount

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Documented? no
Fixed tickets #286
License MIT

@murtukov
Copy link
Member

murtukov commented Nov 9, 2020

ping @mcg-web

Could you review this PR? I don't work with Relay, so I can't say if it's a good thing to allow to return a promise from the getter.

@murtukov
Copy link
Member

murtukov commented Nov 9, 2020

@rsolyanik before pushing any changes you have to execute composer fix-cs to bring the code to predefined coding styles and composer static-analysis to check if there are any problems. If there are any problems, Travis-CI check won't pass. Also your branch is out-of-date with the current master, you either have to merge the master into your branch or rebase your branch onto current master and force-push it.

@rsolyanik
Copy link
Contributor Author

rsolyanik commented Nov 10, 2020

ping @mcg-web

Could you review this PR? I don't work with Relay, so I can't say if it's a good thing to allow to return a promise from the getter.

This is necessary to load total counts via data loader in nested paged lists
Something like
{ customers(first: 1000) { edges { node { id, loyaltyAccounts { totalCount edges { node { .... } } } } } } }

@murtukov murtukov merged commit 5dc5b7c into overblog:master Nov 10, 2020
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

Successfully merging this pull request may close these issues.

2 participants