Incorrect behaviour with operations in .js/.ts files

[mikew]

Describe the bug
The @graphql-eslint/unique-fragment-name, @graphql-eslint/unique-operation-name, and @graphql-eslint/known-fragment-names rules seem to incorrectly report errors when operations are defined in .js/.ts files.

To Reproduce
Steps to reproduce the behavior:

https://github.com/mikew/graphql-eslint-woes

Expected behavior
There should be no issues in the repo above.

Environment:

• OS: macOS
• @graphql-eslint/...: latest
• NodeJS: 14

[dotansimha]

Hi @mikew , thank you for reporting this!
I think @B2o5T's changes in #511 might fix this?

[dimaMachina]

@dotansimha No :) it's another issue. Inside unique-fragment-name rule we make assertion

normalizePath(f.filePath) !== normalizePath(context.getFilename()

where context.getFilename() is virtual file /Users/dimitri/graphql-eslint-woes/src/index.js/0_document.graphql for first extracted document and f.filePath is sibling with real file path /Users/dimitri/graphql-eslint-woes/src/index.js

also loadSiblings returns strange result

[
  {
    location: '/Users/dimitri/graphql-eslint-woes/src/index.js',
    document: { kind: 'Document', definitions: [Array] },
    rawSDL: '# incorrect error re: already defined\n' +
      'fragment postFragment on Post {\n' +
      '  id\n' +
      '}\n' +
      '\n' +
      '# incorrect error re: already defined\n' +
      'query testQuery {\n' +
      '  getPosts {\n' +
      '    # incorrect error re: unknown fragment\n' +
      '    ...postFragment\n' +
      '  }\n' +
      '}'
  }
]

[mikew]

Thanks for looking into this so quickly! Curious, what about the loadSiblings snippet above seems strange?

[dotansimha]

Thanks for looking into this so quickly! Curious, what about the loadSiblings snippet above seems strange?

We have 3 different kinds of files loaded in this plugin:

1. Regular .graphql file - loaded with ESLint core using the plugin, and then we parse it with GraphQL and convert it to ESTree.
2. Code files with GraphQL code - we call these "virtual files" since we use ESLint processor to extract these from JS/TS files, and then we can a virtual file path (that doesn't exists in FS).
3. Siblings - these are operations loaded as "siblings" to either regular file or virtual files, and they are in use in order to support or enrich the plugins with more info (For example, you might use ESLint for lining a GraphQL file with a query, but if that query is using a fragment spread, we have a mechanism that loads the fragment)

The snippet @B2o5T mentioned is the one that compares file paths while loading siblings. When it's loaded against while linting a virtual file, we probably compare it in the wrong way (maybe we should use the original file path, not the virtual?).

[dimaMachina]

The false positive issue with known-fragment-names - currently it’s not possible to handle string interpolation because graphql-tag-pluck doesn’t support it (we use graphql-tag-pluck under the hood for extract documents from js/ts files)

And the issue with unique-fragment-name and unique-operation-name I’ll fix this week

[dotansimha]

Fixed in @graphql-eslint/eslint-plugin@1.1.3

[mikew]

Awesome work!

Tho there does seem to be an edge case. Now it doesn't error if there's two fragments/operations with the same name in the same file. I can make another issue for that.

[dimaMachina]

@mikew thank you for another report :)

@dotansimha It seems like siblings need to return virtual file path for the code files instead of the real file path like graphql-eslint processor does, so this will fix bug it doesn't error if there are two fragments/operations with the same name in the same file