Adds Validation Rules #135
Merged
Conversation
This file contains 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
Comment on lines
-177
to
+183
| public init( | ||
| name: String, | ||
| description: String? = nil, | ||
| serialize: @escaping (Any) throws -> Map | ||
| ) throws { | ||
| try assertValid(name: name) | ||
| self.name = name | ||
| self.description = description | ||
| self.serialize = serialize | ||
| parseValue = nil | ||
| parseLiteral = nil | ||
| } | ||
| let parseValue: (Map) throws -> Map | ||
| let parseLiteral: (Value) throws -> Map | ||
|
|
||
| public init( | ||
| name: String, | ||
| description: String? = nil, | ||
| serialize: @escaping (Any) throws -> Map, | ||
| parseValue: @escaping (Map) throws -> Map, | ||
| parseLiteral: @escaping (Value) throws -> Map | ||
| parseValue: ((Map) throws -> Map)? = nil, | ||
| parseLiteral: ((Value) throws -> Map)? = nil |
There was a problem hiding this comment.
Note that the combination of these two initializer changes is that the Scalar public API is preserved.
NeedleInAJayStack
force-pushed
the
feature/validation
branch
from
dc14896
to
55d41b8
Compare
These issues are caught by separate validation rules
NeedleInAJayStack
force-pushed
the
feature/validation
branch
from
55d41b8
to
33fa691
Compare
|
@paulofaria @d-exclaimation Hey guys, let me know if there's something I can do to make this easier to review. I realize that it's very big and I'd be willing to split it up into smaller PRs if that helps. |
paulofaria
previously approved these changes
Nov 27, 2023
TypeInfo also matched to graphql-js
NeedleInAJayStack
force-pushed
the
feature/validation
branch
from
33fa691
to
a4063a1
Compare
|
@paulofaria Thanks! In applying it to my complex GraphQL schema, I found and fixed one small bug. Please re-approve if that looks okay! https://github.com/GraphQLSwift/GraphQL/compare/33fa691e106911893265995f1be99efd965d6e1b..a4063a10941592c45c3dd92f3f0c5f9d70043aca |
paulofaria
approved these changes
Nov 28, 2023
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.
This adds 19 new validation rules from graphql-js as well as all associated tests. As bugs were exposed through this testing, relevant fixes were made. This should improve usability of GraphQL APIs built on this package, since APIs will now automatically return better error messages to clients.
Since this is such a large merge request, I've organized the commits so that each new rule is specified in a
featurecommit. Any required fixes or adjustments were made infixcommits before thefeaturecommit in which the rule was added. The MR may be easier to review commit-by-commit.