Skip to content
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

loadGraphQLConfig searches for graphqlrc files relative to CWD, not the linted files #899

Closed
lennyburdette opened this issue Jan 13, 2022 · 4 comments · Fixed by #900
Closed
Labels
kind/bug Bug :-( stage/5-alpha-release-testing The pull request is merged, an alpha release is available, to be tested

Comments

@lennyburdette
Copy link
Contributor

lennyburdette commented Jan 13, 2022

Describe the bug
When running rules that need access to the schema, and using a .graphqlrc file to locate the schema, the search for the .graphqlrc file always starts from process.cwd() (see graphql-config docs for rootDir).

To Reproduce
Runnable reproduction: https://stackblitz.com/edit/node-6fhvcm?file=README.md

Original steps to reproduce the behavior:

  1. Open a project in VSCode with subfolders, something like:
    project/
      - packages/
        - app/
          - schema.graphql
          - .eslintrc
          - .graphqlrc
          - documents/
            - operation.graphql
    
  2. Observe that the ESLint VSCode plugin shows an error similar to:
    Error while loading rule '@graphql-eslint/no-scalar-result-type-on-mutation': 
    Rule 'no-scalar-result-type-on-mutation' requires 'parserOptions.schema' to 
    be set and schema to be loaded.
    
  3. Open the packages/app folder directly in VSCode. Observe that the ESLint plugin works correctly.

Expected behavior
Graphql-eslint should search for .graphqlrc files relative to the file being linted. This is a relatively simple change:

// graphql-config.ts line 17
      loadConfigSync({
        // NEW CODE HERE
        rootDir: options.filePath ? dirname(options.filePath) : undefined,
        throwOnEmpty: false,
        throwOnMissing: false,
        extensions: [addCodeFileLoaderExtension],
      });

Environment:

Additional context
There's also an issue with declaring the schema in .eslintrc with the "parserOptions.schema" option, where it looks for the file relative to process.cwd instead of relative to the .eslintrc, but that would be a different fix.

@dotansimha dotansimha added the kind/bug Bug :-( label Jan 16, 2022
@dotansimha dotansimha reopened this Jan 20, 2022
@dotansimha dotansimha added the stage/5-alpha-release-testing The pull request is merged, an alpha release is available, to be tested label Jan 20, 2022
@arusanov
Copy link
Contributor

hey @arusanov, thanks again for the wonderful contribution! Your PR is merged and released as @graphql-eslint/[email protected] .

Hey, sorry, I don't remember any of my PRs in this repo. Only this one, but it's from 2020 #176

@dotansimha
Copy link
Collaborator

hey @arusanov, thanks again for the wonderful contribution! Your PR is merged and released as @graphql-eslint/[email protected] .

Oops, wrong mention :)

@dotansimha
Copy link
Collaborator

hey @lennyburdette , thanks again for the wonderful contribution! Your PR is merged and released as @graphql-eslint/[email protected] .

If it's ok, I wanted to ask you about https://github.com/apollographql/eslint-plugin-graphql - what is the future of that library from Apollo's perspective?

It has been unmaintained for a while but is still very popular.

We saw that some new projects at Apollo use graphql-eslint (https://github.com/apollographql/rover/blob/4475f01f52f2808bd3b8c1ea0f993ea57aae2815/crates/rover-client/.eslintrc.json#L13). any chance we could join forces here to make it easier for GraphQL developers to land on the best solution from Apollo's perspective?

Also, if there are more changes or things you would want us to solve on graphql-eslint we would love to work on it!

Thank you so much and we are looking forward to more ways we could collaborate together!

@lennyburdette
Copy link
Contributor Author

Hi @dotansimha ! Thanks for the review and for shepherding this through!

I can't speak directly for the open source maintainers within Apollo, but personally I think that we should deprecate our library and direct future linting efforts towards graphql-eslint. I'll start some conversations internally and get back to you.

I've spent some time over the last few weeks trying to put together some examples of using graphql-eslint for our customers. The most common ask is for enforcing standards and best practices for schema authors. It's proven a bit more challenging than I expected because of the rules that require an instance of the full schema, like no-unknown-types. Issue #593 seems related — generating and refreshing the schema for real-time feedback in an IDE is tricky!

But I still want to recommend graphql-eslint as a command line/continuous integration tool. I've noticed that some error messages lack enough context to fix the issue, but I'll file some bugs or PRs about that soon.

I'm also looking forward to collaborating! Thanks for taking on this project!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Bug :-( stage/5-alpha-release-testing The pull request is merged, an alpha release is available, to be tested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants