Skip to content

Conversation

@thibaudlabat
Copy link
Contributor

@thibaudlabat thibaudlabat commented Jun 13, 2025

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)

query simpleQuery {
      books {
        title
        author
      }
}

query connectionQuery {
      books(first: 1) {
        title
        author
      }
}

The cost complexity of the query below should increase by a factor of the number of books requested

query connectionQuery {
      books(first: 3) {
        title
        author
      }
}

Note the PR does not currently handle VariableDefinitions but will handle the example above

@thibaudlabat thibaudlabat requested a review from LMaxence June 13, 2025 14:00
@changeset-bot
Copy link

changeset-bot bot commented Jun 13, 2025

⚠️ No Changeset found

Latest commit: 5c0513f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codecov-commenter
Copy link

codecov-commenter commented Jun 13, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.13%. Comparing base (13978ed) to head (782b8cb).

❗ 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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@thibaudlabat thibaudlabat merged commit 4a8a50a into main Jun 16, 2025
4 of 12 checks passed
@thibaudlabat thibaudlabat deleted the feat/connectcost branch June 16, 2025 13:06
@dzmitry-miadzvedzeu
Copy link

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 "@escape.tech/graphql-armor-cost-limit": "2.4.3".

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.

@quinkinser
Copy link

quinkinser commented Sep 17, 2025

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 "@escape.tech/graphql-armor-cost-limit": "2.4.3".

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. costMultiplyingArgs: string[] | false; or enableRelaySpecificCostMultiplication: boolean = fasle, etc.), while preserving the existing behavior for anyone who doesn't opt in.

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.

Support Connections in cost complexity calculation

6 participants