possible-type-extension erroring when extending types defined in separate files.

[JustinTRoss]

Describe the bug

possible-type-extension erroring when extending types defined in separate files.

Error:

Cannot extend type "Query" because it is not defined  @graphql-eslint/possible-type-extension

Additionally, if this error triggers for any file, an error shows up in lint results for every file in which a type was extended. Also, I notice messaging is slightly different when extending known types likeQuery vs custom types like NotQuery.

To Reproduce
Steps to reproduce the behavior:

Run yarn lint in below repo.
https://github.com/JustinTRoss/graphql-eslint-issue-repro

Expected behavior

Ideally, no errors would be thrown when extending a type defined in another file.

Environment:

• OS: MacOS 11.4
• @graphql-eslint/...: 2.5
• NodeJS: 14.17.5

Additional context

[dimaMachina]

Thank you for the reproduction repo 🙂, it looks like this is a similar problem to this issue. I will look in detail to find a solution after the major v3 version will be released.
Until this problem not resolved, I'll remove this rule from the recommended and all config.

[dimaMachina]

@JustinTRoss Fixed in 3.3.0-alpha-b07557f.0 can you test this before we add this rule to recommended config? :)

[JustinTRoss]

Good work 💪 . That appears to have mostly solved it. Parsing errors throw it for a loop though. That issue extends more broadly than this rule in some circumstances, but some seem specific to this rule.

Broadly, a parsing error in one file seems to throw a parsing error in all files. I can open another issue for this if it doesn't already exist.

Specific to this rule, if I put a parsing error in one file, I get an erroneous possible-type-extension error in another file.

Repro repo updated with example.

In case it isn't obvious, here is parsing error, and here is where erroneous 'possible-type-extension' error triggers.

[dimaMachina]

Thanks @JustinTRoss, I’ll took a look and fix it ;)

[JustinTRoss]

You're quick!

I notice it happens in other scenarios as well. If you make a second valid extension in another file, you get the error. See this branch for details.

Second valid extension is here.

[dimaMachina]

@JustinTRoss Both errors were fixed in 3.3.0-alpha-1c01242.0 can you confirm that now everything is okay? :)

[JustinTRoss]

You're quick! Those two scenarios look mitigated. I come bearing another 🙌 . 😛

If you define a duplicate version of the same type with violations in it, like duplicate conflicting fields, depending the order of loaded files, it will report either as an error merging types or an error defining type. At the same time, it errors out and inaccurately conveys that the eslint parserOptions.schema is not set, but is required for the possible-type-extension rule.

Try running yarn lint on this branch.

Offending type/field def here.

[JustinTRoss]

Possibly getting away from the scope of this rule and the changes made here, but I found I could invalidly define a union and extend it without any issue.

See this branch for details.

Further, if I defined union in certain invalid ways, it errors regarding merging of unrelated valid types in a different file.

See this branch for details.

[dimaMachina]

You're quick! Those two scenarios look mitigated. I come bearing another 🙌 . 😛

If you define a duplicate version of the same type with violations in it, like duplicate conflicting fields, depending the order of loaded files, it will report either as an error merging types or an error defining type. At the same time, it errors out and inaccurately conveys that the eslint parserOptions.schema is not set, but is required for the possible-type-extension rule.

Try running yarn lint on this branch.

Offending type/field def here.

this is expected behavior, we can't build a schema that contains validation errors like duplicated fields, in this case, we aren't able to provide a GraphQL schema for rules that require it.

Possibly getting away from the scope of this rule and the changes made here, but I found I could invalidly define a union and extend it without any issue.

See this branch for details.

Fixed in 3.4.0-alpha-735b6ae.0. Please not you'll get an erroneous possible-type-extension error in another file because the error Union type ... can only include ... goes from validate() function and not from possible-type-extension rule.

/Users/dimitri/Desktop/graphql-eslint-issue-repro/src/index.ts
  5:1  error  Union type Product can only include Object types, it cannot include Genre  @graphql-eslint/possible-type-extension

/Users/dimitri/Desktop/graphql-eslint-issue-repro/src/modules/Schema/first/firstSchema.ts
  3:1  error  Union type Product can only include Object types, it cannot include Genre  @graphql-eslint/possible-type-extension

/Users/dimitri/Desktop/graphql-eslint-issue-repro/src/modules/Schema/second/secondSchema.ts
  3:1  error  Union type Product can only include Object types, it cannot include Genre  @graphql-eslint/possible-type-extension

✖ 3 problems (3 errors, 0 warnings)

Further, if I defined union in certain invalid ways, it errors regarding merging of unrelated valid types in a different file.

See this branch for details.

It's related to how graphql validate SDL, I can't do anything here

[dotansimha]

Fixed in @graphql-eslint/[email protected].

[JustinTRoss]

Thanks for the improvements. I'll upgrade to 3.4.

this is expected behavior, we can't build a schema that contains validation errors like duplicated fields, in this case, we aren't able to provide a GraphQL schema for rules that require it.

That makes sense. It was surprising that it conveyed to the user that they had not provided a parserOptions.schema though, as it makes debugging difficult. It seems like it'd be easier to understand if we swallowed that error and conveyed the validation error causing it. Not sure how feasible that is.