docs: add guide for operation complexity controls #4402
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
benjie
requested changes
May 20, 2025
There was a problem hiding this comment.
This is really good, and I'm excited to see us addressing this topic! However, ideally we should not promote a particular module for complexity analysis (unless it's one we control), otherwise we open ourselves to adding that to GraphQL.js' security surface area, and we artificially limit competition/innovation within the ecosystem. Instead we should talk about it in more general terms (and perhaps give a few examples of modules that can be used for this). In addition:
- We should strongly encourage the use of trusted documents over complexity limits at runtime
- We should note that complexity limits can be subject to attack themselves (for example, via fragment cycles), so it's generally best to run them after validation but before execution unless we know they're safe to run alongside validation
- The validation phase doesn't have access to variables (validation tends to happen once per document independent of variables) so complexity limits depending on variables will need special plumbing
- Certain field types make operations more complex, in particular lists (and, to a lesser extent, abstract types); we should comment how these "multiply up" the complexity.
- We should note that this should form part of a layered security approach - referencing https://graphql.org/learn/security/
- We should note that with trusted documents, complexity analysis can still be valuable to warn engineers about expensive operations, and can mostly be done at build-time rather than run-time.
- We should be careful with the use of the word "query", because we're talking about mutations and subscriptions too really. Consider "operation" or "document" as appropriate (glossary), or simply elide "query" entirely: "complexity controls".
I don't think you need to change the document very much to accommodate this feedback - e.g. you can keep all the examples, just make it very clear that they're just examples from one of the modules and other approaches are available.
|
@benjie thanks for much for the thorough feedback! My next commit has the following updates:
Please let me know if there's anything else we should update! :) |
benjie
requested changes
May 29, 2025
benjie
approved these changes
May 29, 2025
yaacovCR
pushed a commit
to yaacovCR/graphql-js
that referenced
this pull request
May 30, 2025
Adds guide "Operation Complexity Controls" --------- Co-authored-by: Benjie <benjie@jemjie.com>
yaacovCR
pushed a commit
that referenced
this pull request
May 30, 2025
Adds guide "Operation Complexity Controls" --------- Co-authored-by: Benjie <benjie@jemjie.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds guide "Operation Complexity Controls"