refactor: Deprecate disabled default values for query complexity limits#10207
Conversation
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds six new deprecation entries to the deprecations catalog, introducing dot-notated keys for requestComplexity (includeDepth, includeCount, subqueryDepth, queryDepth, graphQLDepth, graphQLFields) with new default values and guidance text; updates DEPRECATIONS.md and adds tests verifying logging behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Deprecator/Deprecations.js (1)
44-73: Add table-driven tests for the six new deprecation entries.Please cover both paths per key: warning when unset, and no warning when explicitly set (including
-1). This change is config-safety critical and easy to regress without targeted assertions.I can draft a compact parameterized test matrix for this if you want.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Deprecator/Deprecations.js` around lines 44 - 73, Add table-driven unit tests for the six new deprecation entries in Deprecations.js to prevent regressions: create a parameterized matrix over the optionKey values 'requestComplexity.includeDepth', 'requestComplexity.includeCount', 'requestComplexity.subqueryDepth', 'requestComplexity.queryDepth', 'requestComplexity.graphQLDepth', and 'requestComplexity.graphQLFields' and for each assert two behaviors — that the deprecation warning is emitted when the option is unset (or left at default) and that no warning is emitted when the option is explicitly set (test both a positive integer and '-1' disable value). Hook these tests into the existing deprecation-checking helper used by your test suite (the same utility/assertions other deprecation tests use) so they exercise the same warning capture/assert logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Deprecator/Deprecations.js`:
- Around line 44-73: Add table-driven unit tests for the six new deprecation
entries in Deprecations.js to prevent regressions: create a parameterized matrix
over the optionKey values 'requestComplexity.includeDepth',
'requestComplexity.includeCount', 'requestComplexity.subqueryDepth',
'requestComplexity.queryDepth', 'requestComplexity.graphQLDepth', and
'requestComplexity.graphQLFields' and for each assert two behaviors — that the
deprecation warning is emitted when the option is unset (or left at default) and
that no warning is emitted when the option is explicitly set (test both a
positive integer and '-1' disable value). Hook these tests into the existing
deprecation-checking helper used by your test suite (the same utility/assertions
other deprecation tests use) so they exercise the same warning capture/assert
logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 52d53765-4603-413f-9969-88f5943a0c2b
📒 Files selected for processing (1)
src/Deprecator/Deprecations.js
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #10207 +/- ##
=======================================
Coverage 92.56% 92.56%
=======================================
Files 192 192
Lines 16284 16284
Branches 199 199
=======================================
Hits 15073 15073
Misses 1194 1194
Partials 17 17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
🎉 This change has been released in version 9.6.0-alpha.23 |
2 participants
Issue
The
requestComplexityoptions (includeDepth,includeCount,subqueryDepth,queryDepth,graphQLDepth,graphQLFields) currently default to-1(disabled) to avoid a breaking change. In the next major version, these defaults will change to positive values that enforce query complexity limits. This adds deprecation warnings to inform developers to explicitly configure these options.Tasks
Summary by CodeRabbit