- 
                Notifications
    You must be signed in to change notification settings 
- Fork 41
feat: add support for connections in complexity calculation (@freshjsw) #810
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
Conversation
| 
 | 
| 
 Codecov ReportAll modified and coverable lines are covered by tests ✅ 
 ❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@            Coverage Diff             @@
##             main     #810      +/-   ##
==========================================
+ Coverage   91.96%   92.13%   +0.16%     
==========================================
  Files          17       17              
  Lines         386      394       +8     
  Branches      121      134      +13     
==========================================
+ Hits          355      363       +8     
  Misses         31       31              ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
 | 
| Hello @LMaxence and @thibaudlabat. Thanks for your work on this package. I just noticed that this update changes a lot cost-limits validation and it was released as patch version  It seems for me this should be minor version release and better to mention it in changelog as potentially a lot of people might update the dependency and then it appears that a lot of their queries are not valid anymore because cost estimations algorithm changed. | 
| 
 Yes, this should have been a major version bump. Any time we update the complexity/depth/etc. calculation in a way that can increase the measured value, it's a breaking change that could start rejecting previously OK requests (that'd be a fun incident to figure out on the fly if it was your website's home page query 😬 ). In the future, if we want to release changes like this one in a minor version bump, we'd need to introduce the change via an optional config setting (e.g.  | 
See #762
Original contribution from @freshjsw
I just made changes according to @LMaxence comments.
--- ORIGINAL PULL REQUEST DESCRIPTION ---
The cost complexity cost-limit plugin does not take into account how many entities are being returned when either the first or last argument are specified at the field level. This PR supports calculating complexity based on the number of records requested in the query.
This commit fixes #761
These two examples which have the same complexity cost (I have omitted edges and pageInfo to keep these examples simple)
The cost complexity of the query below should increase by a factor of the number of books requested
Note the PR does not currently handle VariableDefinitions but will handle the example above