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

[BUG] Default GraphQL DataProvider GetListResponse from 'count' property not possible. #5942

Open
jamesdh opened this issue May 11, 2024 · 5 comments · May be fixed by #6357
Open

[BUG] Default GraphQL DataProvider GetListResponse from 'count' property not possible. #5942

jamesdh opened this issue May 11, 2024 · 5 comments · May be fixed by #6357
Assignees
Labels
bug Something isn't working

Comments

@jamesdh
Copy link

jamesdh commented May 11, 2024

Describe the bug

The default GraphQL providers getList function return the following GetListResponse shaped object:

return {
data: response[operation],
total: response[operation].count,
};

As mentioned by another user on Discord (possibly @Karabur?), this does not make sense. In order for getList to function correctly with various integration points within Refine (like the MUI useDataGrid hook), response[operation] must return an array of objects. An array cannot have additional properties on it such as count. I'm not sure I understand how anybody could be using this default data provider out of the box with working pagination.

Steps To Reproduce

N/A. Per the graphQL spec, a list cannot have additional properties.

Expected behavior

I do not see how any GraphQL API could provide a response that would provide working pagination for the default GraphQL provider. Even the docs for useList indicate something considerably different, more akin to a Relay response.

Packages

  • @refinedev/graphql: 6.5.0

Additional Context

No response

@jamesdh jamesdh added the bug Something isn't working label May 11, 2024
@BatuhanW
Copy link
Member

Hey @jamesdh you are right, it's a bit illogical to expect a field in an array. GraphQL data provider, due to nature of GraphQL can't fit into every API so it's more like a blueprint to start building your own data provider. I just checked and it seems we are missing asserting count field in the tests. Nevertheless, this can be fixed to avoid confusion.

@jamesdh
Copy link
Author

jamesdh commented May 21, 2024

GraphQL data provider, due to nature of GraphQL can't fit into every API

True, but there are conventions for this frequently found in the wild that are based on the Relay spec. Specifically, Relay has the concept of Connection types. Connections must contain a PageInfo field. What you commonly see "in the wild" (but that is not part of the official spec) is that either the Connection object itself or the PageInfo object contain a "totalCount" property. I believe it's usually found on the Connection object, since it should not change between paginations (and hence it makes less sense for it to be on the PageInfo object).

In any case, the "path" in the object graph where this count property can be found should be something that is definable in the meta object along with the query. The same is true for where the intended data can be found, and thus #5943 could be fixed.

Copy link

stale bot commented Jul 20, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Jul 20, 2024
@aliemir aliemir removed the wontfix This will not be worked on label Jul 20, 2024
@swethamanur
Copy link

@aliemir , if this issue is still open, could you re-assign this to me please?

@aliemir
Copy link
Member

aliemir commented Jul 31, 2024

I don't have much info about this issue. Maybe @jamesdh or @BatuhanW can help on what to do here or what should we be aware of.

@BatuhanW BatuhanW linked a pull request Sep 19, 2024 that will close this issue
5 tasks
@BatuhanW BatuhanW added this to the October Release milestone Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants