graphql-schema-linter fails when called upon lint-staged hook

[gagoar]

graphql-schema-linter requires the entire schema to properly lint.

in the other hand lint-staged passes the file/s that has changed as an argument making the graphql-schema-linter cli use that input as the schema and ignoring the configuration pulled from cosmic.

This is also a source of issues when trying to chain behaivour, for instance, yarn graphql-schema-linter && yarn test. In this case the cli will understand that && yarn test is part of the arguments passed to graphlq-schema-linter and not other command making it a bit hard to chain unless creating auxiliary scrpits.

For now we have opted for creating a script wrapping the cli that passes again the schema glob to avoid the duplicated configuration (I find this solution awkward given that duplicates configuration that we will have to remember and maintain going forward).

there are a couple of options I can think of to mitigate this issue:

1) Ignore extra arguments wihtout flags if the configuration from cosmic is present. (this solution will be backwards compatible).

2) Only allow schemaPath passed in the cli with a flag (ex: -s --schemaPaths) matching the configuration we have in place already. (this will break backwards compatibility but it will match the configuration from cosmic in the cli).

3) add a flag that forces the use of the configuration in the command line otherwise keep doing what it does ( ex: --ignore-extra-config) to be used specifically for this type of case where the user has undesired arguments passed to the command.

Happy to implement any of these. I would love to know which one you see better. Open to other approaches as well.

[cjoudrey]

Hey @gagoar! Thanks for opening this issue.

in the other hand lint-staged passes the file/s that has changed as an argument making the graphql-schema-linter cli use that input as the schema and ignoring the configuration pulled from cosmic.

The initial version of graphql-schema-linter was built on the assumption that people had a single .graphql file. Later on, I added support for schemas that were defined amongst many files and probably forgot to update the lint-staged example in the README.md.

I don't personally use lint-staged, but I took a look at their README.md and found this: https://github.com/okonet/lint-staged#example-run-eslint-on-entire-repo-if-more-than-10-staged-files

This would effectively allow us to do:

'**/*.graphql': filenames => 'graphql-schema-linter'

It's not great, but I think that might work.

This is also a source of issues when trying to chain behaivour, for instance, yarn graphql-schema-linter && yarn test. In this case the cli will understand that && yarn test is part of the arguments passed to graphlq-schema-linter and not other command making it a bit hard to chain unless creating auxiliary scrpits.

That's strange. I can't reproduce that in bash.

$ yarn graphql-schema-linter && echo "test"
yarn run v1.15.2
$ /private/tmp/test2/node_modules/.bin/graphql-schema-linter

✔ 0 errors detected

✨  Done in 0.25s.
test

What shell are you using? Or did I misunderstand what you were saying?

[gagoar]

Hey @gagoar! Thanks for opening this issue.

in the other hand lint-staged passes the file/s that has changed as an argument making the graphql-schema-linter cli use that input as the schema and ignoring the configuration pulled from cosmic.

The initial version of graphql-schema-linter was built on the assumption that people had a single .graphql file. Later on, I added support for schemas that were defined amongst many files and probably forgot to update the lint-staged example in the README.md.

I don't personally use lint-staged, but I took a look at their README.md and found this: https://github.com/okonet/lint-staged#example-run-eslint-on-entire-repo-if-more-than-10-staged-files

This would effectively allow us to do:

'**/*.graphql': filenames => 'graphql-schema-linter'

It's not great, but I think that might work.

yea the issue here is that we try to avoid logic on configuration when possible. and this option it will move all the configuration to JS, just to bypass the filename.

This is also a source of issues when trying to chain behaivour, for instance, yarn graphql-schema-linter && yarn test. In this case the cli will understand that && yarn test is part of the arguments passed to graphlq-schema-linter and not other command making it a bit hard to chain unless creating auxiliary scrpits.

That's strange. I can't reproduce that in bash.

$ yarn graphql-schema-linter && echo "test"
yarn run v1.15.2
$ /private/tmp/test2/node_modules/.bin/graphql-schema-linter

this error happens when lint-staged calls the script directly. I can share with you a mini repo with the issue so you can see it. 
✔ 0 errors detected

✨  Done in 0.25s.
test

What shell are you using? Or did I misunderstand what you were saying?

it's not the shell, is how yarn/npm takes the extra arguments coming. I will try today to share the repo and expose the issue more clearly!

[mshwery]

How feasible would it be to load a single file's AST in a multi-file schema to lint individual files passed into the linter?

[cjoudrey]

How feasible would it be to load a single file's AST in a multi-file schema to lint individual files passed into the linter?

Hey @mshwery! 😄

It really depends on what inputs you have.

If you're asking, is there a way to pass a partial GraphQL schema to the linter, that might be tricky because of this:

https://github.com/cjoudrey/graphql-schema-linter/blob/4b9566fc5435c2d6967fb2740e87c24714d0c115/src/validator.js#L33-L44

It's also made other things tricky in the past (like supporting custom directives provided by various libraries / services) because it wants a fully valid GraphQL schema. I've definitely questioned whether or not that behaviour was desirable or not.

If you're asking, I have a multi-file schema but only care about the errors in a.graphql, then I suppose you could do some filtering with --format json / jq or --format compact / grep, but it would be a bit hacky. You'd probably want to preserve the original process' exit code if you're relying on that in CI, etc..

[mshwery]

If you're asking, I have a multi-file schema but only care about the errors in a.graphql, then I suppose you could do some filtering with --format json / jq or --format compact / grep, but it would be a bit hacky. You'd probably want to preserve the original process' exit code if you're relying on that in CI, etc..

There are two main uses cases I'm thinking of (1 relevant to this issue):

1. During local dev, using lint-staged to only validate the graphql files being modified
2. Being able to ignore some legacy schema files that are too difficult to support all of the linting rules (or even disable rules on fields/types within a file).

[cjoudrey]

Thanks for the feedback @mshwery.

During local dev, using lint-staged to only validate the graphql files being modified

I think this is do-able with the tool today by filtering through the output, but it's definitely not a great solution.

I'm open to suggestions / PRs for a more appropriate solution.

Just thinking out loud here.

The tool today assumes it is given a complete GraphQL schema. If it didn't need the complete GraphQL schema, we could just pass it a partial file and let it lint that. I haven't looked at the implications of this.

Alternatively, if we realize we can't break that assumption, then I guess another option would be to add a way to tell the linter to lint file.graphql in the context of full-schema.graphql. This could effectively run the linter and only consider errors originating from file.graphql.

Being able to ignore some legacy schema files that are too difficult to support all of the linting rules (or even disable rules on fields/types within a file).

Yeah, I think such a feature is important to have. This really ties into #18. I'll reply to that issue directly so we don't conflate both issues.