-
Notifications
You must be signed in to change notification settings - Fork 0
Add path to input validation error #40
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
44743cf to
d80bd91
Compare
| const valueStack: NestedValue[][] = [[{ value, path: [] }]]; | ||
| const typeStack: (GraphQLNamedType | undefined)[] = []; | ||
| const possibleTypesStack: Set<string>[] = [ | ||
| new Set([schema.getQueryType()!.name]), | ||
| ]; | ||
| const typenameResponseKeyStack: (string | undefined)[] = []; | ||
| const expectedFieldsStack: Map<string, Set<string>>[] = [new Map()]; | ||
|
|
||
| const errors: string[] = []; | ||
| const errors: FixtureInputValidationError[] = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for improving these types <3
| errors.push( | ||
| `Cannot validate ${responseKey}: missing field definition`, | ||
| ); | ||
| const currentPath = pathFromAncestors(ancestors); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was really surprised that we couldn't just use the path from the args here.
I added a bit of logging to understand things more, and saw it doesn't contain any of the field names.
For this query visiting field name:
query {
data {
items {
name
}
}
}
Built-in path:
['definitions', 0, 'selectionSet', 'selections', 0, 'selectionSet', 'selections', 0, 'selectionSet', 'selections', 0]
→ AST structure path (technical)
pathFromAncestors(ancestors):
['data', 'items', 'name']
→ GraphQL field path (semantic, useful for error messages)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I went through the same thing
|
Could you add a |
saga-dasgupta
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the path, LGTM.
Add path to the fixture input validation error so it is easier to identify where the error is occurring.