fix mutation name #15
Conversation
There was a problem hiding this comment.
Hey @msayagh, thanks for the PR! Looks like a good improvement. I have some comments that would help us land this in the next release. Let me know if something is unclear :)
Kind regards,
Michiel Westerbeek
| @@ -2,7 +2,12 @@ | |||
| "name": "gqlint", | |||
| "version": "1.8.0", | |||
| "description": "GraphQL Linter", | |||
| "keywords": ["graphql", "gql", "lint", "linter"], | |||
| "keywords": [ | |||
There was a problem hiding this comment.
Can you remove the changes from this file? They're not related to your pr.
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "gqlint", | |||
| "version": "1.6.1", | |||
| "version": "1.8.0", | |||
There was a problem hiding this comment.
Please rebase on master (git pull --rebase origin master) as I don't think you have the latest code. I'll do the bumping :)
| @@ -58,12 +58,43 @@ const checkWrongName = (text, node, messages, name) => { | |||
| } | |||
| }; | |||
|
|
|||
| function getMutationNames(ast) { | |||
There was a problem hiding this comment.
Could you place this function in a new file in the "lib" folder, maybe helpers.js or mutations.js, whatever you think is best.
It's now duplicated in two places and if we are gonna have more rules for mutations we should probably use this there as well.
| visit(ast, { | ||
| ObjectTypeDefinition(node) { | ||
| if (node.name.value !== 'GraphQLMutation') { | ||
| if (mutationNames.indexOf(node.name.value) < 0) { |
There was a problem hiding this comment.
And lastly, would you be able to add a test for this new "extend type Mutation" in https://github.com/happylinks/gqlint/blob/master/tests/rules/removedelete.mutations.test.js and maybe also in https://github.com/happylinks/gqlint/blob/master/tests/rules/singular.mutations.test.js just to be sure.
|
Sure, I will address your comments ASAP. |
Mutations were previously found using the keyword "GraphQLMutation", while users do not always follow this rule. They can, however, follow this pattern:
extend type Mutation {
users: UserMutation
}
type UserMutation {
createUsers(email: String): User
}
This pull request fixes that problem. I first find what are the mutations that we have, then run the previously implemented analysis