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

Cannot use query complexity limitations on graphql cursor connections, cause it's not a list #848

Closed
jrouaix opened this issue Feb 25, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@jrouaix
Copy link

jrouaix commented Feb 25, 2022

Expected Behavior

Tried to use query complexity limitations on a field returning paginated nodes using cursor connections :

https://async-graphql.github.io/async-graphql/en/depth_and_complexity.html#limiting-query-complexity
https://async-graphql.github.io/async-graphql/en/cursor_connections.html

IMHO, we should be able to limit here based on some first, last parameters.

Actual Behavior

the complexity computation ComplexityType::Fn is ignored in the complexity visitor here :

if MetaTypeName::create(&meta_field.ty).is_list() {

Because the result type of the query is of type async_graphql::Result<Connection<usize, SomeOtherType, EmptyFields, EmptyFields>>

Steps to Reproduce the Problem

use some #[graphql(complexity = "... some_function_call() ... * child_complexity")]
https://async-graphql.github.io/async-graphql/en/depth_and_complexity.html#limiting-query-complexity
https://async-graphql.github.io/async-graphql/en/cursor_connections.html

both at the same time on a field.

the complexity will be ignored.

@jrouaix jrouaix added the bug Something isn't working label Feb 25, 2022
@jrouaix
Copy link
Author

jrouaix commented Feb 25, 2022

If I remove the condition, the expected behavior happens.
Perhaps it's a quick fix ?
Is this condition really an expected limitation ?

@jrouaix
Copy link
Author

jrouaix commented Mar 8, 2022

Hello ? @sunli829 ?
Any idea about this one ?
Would you prefer a PR ?

@jrouaix
Copy link
Author

jrouaix commented Jun 4, 2022

Anyone here with some thoughts about this ?
I strongly thinks it's a bug, and an easy one to kill, but no-one cares ?

@sunli829
Copy link
Collaborator

sunli829 commented Jun 5, 2022

I'm so sorry to see this just now, I'll check tomorrow to see if this condition is really necessary. 🙂

@sunli829
Copy link
Collaborator

sunli829 commented Jun 6, 2022

I was probably confused at the time. 😂

@jrouaix
Copy link
Author

jrouaix commented Jun 13, 2022

Thank you @sunli829 !

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

No branches or pull requests

2 participants