Trouble with .graphqlconfig / bad config validation / questions

[lydell]

Sorry for bundling several things into one issue. I initally had just one problem, but when diagnosing it things snowballed.

TL;DR: This plugin works super well, just need to look over config validation in general .)

Here's the test repo: https://github.com/lydell/eslint-plugin-graphql-bug

graphql/named-operations .graphqlconfig problem

I tried switching from schemaJson: require("./schema.json") to using .graphqlconfig. This worked well for the graphql/template-strings rule, but I got trouble with the graphql/named-operations rule.

Using .graphqlconfig it looks as if no additional config is need for the graphql/named-operations rule in .eslintrc.js. So I tried this:

"graphql/named-operations": "error",

But then nothing is reported. This works, though:

"graphql/named-operations": ["error", {}],

graphql/capitalized-type-name .graphqlconfig problem

Same thing here. It looks as if this should be enough:

"graphql/capitalized-type-name": "error",

This results in a validation error:

"graphql/capitalized-type-name": ["error", {}],

Configuration for rule "graphql/capitalized-type-name" is invalid:
  Value "[object Object]" should have required property '.schemaJson'.
  Value "[object Object]" should have required property '.schemaJsonFilepath'.
  Value "[object Object]" should have required property '.schemaString'.
  Value "[object Object]" should match exactly one schema in oneOf.

I couldn't get this rule to work .graphqlconfig.

Questions

Diagnosing the above problems and looking at the other rules, I stumbled upon some questions:

• Why does graphql/named-operations require a schema?
• Why does graphql/capitalized-type-name require a schema?
• Why does graphql/required-fields let you specify env?
  • It seems to be optional, so what's the default?
• Is env required for graphql/template-strings? It does not seem so when I
  tested. What's the default?

Other minor config validation quirks

They're all described in https://github.com/lydell/eslint-plugin-graphql-bug

[stubailo]

@lydell can you please file these all separately? It's really hard to keep track of issues if they're in one mega-issue.

[lydell]

Not sure, that'll be a lot of work opening so many issues :) I'm also a bit too confused right now – perhaps we could start with answering the four questions I listed above? I'll also try to look into fixing these issues myself.

[lydell]

Today I took the time to study the source of this plugin, and I can now answer my questions myself.

Questions / general stuff

All four rules of this package (graphql/template-strings, graphql/named-operations, graphql/required-fields and graphql/capitalized-type-name) are really the same rule.

graphql/template-strings has an option called validators. It's a list of names of different things it checks. By default, this list consists of all(*) the validators from the graphql package, which does the actual schema validation in this plugin.

((*) With a few exceptions based on the env option.)

The other three rules basically just set the validators option to the name of custom validators defined in src/rules.js, and then runs the same code as graphql/template-strings.

The env option is used for the slight parsing differences in the GraphQL syntax between apollo, relay and lokka. It is also used to choose defaults for the tagName option and the validators option. If no env is provided, the code always takes the same paths as if env: "apollo" was used. (The env option should be required (not "default" to anything) in my opinion, but that's out of scope for this issue.)

A schema (no matter how it is provided), is required by the validate() function from the graphql package, which is (as said) used to do the actual validation in all of the rules. So even if some of the rules need no schema in theory, they need it technically (as an implementation detail).

So, really, all the rules need both env and a schema. Which clears that confusion up :) I will try and adjust the documentation to make this clearer.

graphql/capitalized-type-name .graphqlconfig problem

This is because the graphql/capitalized-type-name rule and the .graphqlconfig support seem to have been developed in parallel (PRs #81 and #80, respectively), so they never got a chance to sync up. I will try and fix this, so that graphql/capitalized-type-name gets .graphqlconfig support as well.

graphql/named-operations .graphqlconfig problem

This is actually not a problem specific to graphql/named-operations. All of the four rules support specifying several objects in the config array for each rule, one object per graphql tag you use. If you give no config object at all, nothing happens. The ESLint config schemas include minLength: 1 so this isn't supposed to happen. I will try and find out why and fix it.

"graphql/capitalized-type-name": ["error", {}] crash

This is because the requiredFields option should be marked as required (and have at least 1 item), but currently isn't. I will try and fix this.

[jnwng]

Yep, that all looks right to me! Thanks for investigating, and looking forward to clarifications that can be made to make this more straightforward for others.

…

On Sat, Aug 26, 2017 at 6:12 AM Simon Lydell ***@***.***> wrote: Today I took the time to study the source of this plugin, and I can now answer my questions myself. Questions / general stuff All four rules of this package (graphql/template-strings, graphql/named-operations, graphql/required-fields and graphql/capitalized-type-name) are really the same rule. graphql/template-strings has an option called validators. It's a list of names of different things it checks. By default, this list consists of all(*) the validators from the graphql package, which does the actual schema validation in this plugin. ((*) With a few exceptions based on the env option.) The other three rules basically just set the validators option to the name of custom validators defined in src/rules.js, and then runs the same code as graphql/template-strings. The env option is used for the slight parsing differences in the GraphQL syntax between apollo, relay and lokka. It is also used to choose defaults for the tagName option and the validators option. If no env is provided, the code always takes the same paths as if env: "apollo" was used. (The env option should be required (not "default" to anything) in my opinion, but that's out of scope for this issue.) A schema (no matter how it is provided), is required by the validate() function from the graphql package, which is (as said) used to do the actual validation in all of the rules. So even if some of the rules need no schema in theory, they need it technically (as an implementation detail). So, really, *all* the rules need both env and a schema. Which clears that confusion up :) I will try and adjust the documentation to make this clearer. graphql/capitalized-type-name .graphqlconfig problem This is because the graphql/capitalized-type-name rule and the .graphqlconfig support seem to have been developed in parallel (PRs #81 <#81> and #80 <#80>, respectively), so they never got a chance to sync up. I will try and fix this, so that graphql/capitalized-type-name gets .graphqlconfig support as well. graphql/named-operations .graphqlconfig problem This is actually not a problem specific to graphql/named-operations. All of the four rules support specifying several objects in the config array for each rule, one object per graphql tag you use. If you give no config object at all, nothing happens. The ESLint config schemas include minLength: 1 so this isn't supposed to happen. I will try and find out why and fix it. "graphql/capitalized-type-name": ["error", {}] crash This is because the requiredFields option should be marked as required (and have at least 1 item), but currently isn't. I will try and fix this. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#85 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABERRvexSaF-pb6w1rdmEV_0B9XEokUuks5scBm9gaJpZM4PChur> .

[lydell]

I've submitted #86 which addresses everything in here.