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

GraphiQL VSCode warning: Undefined Fragment when spreading is used for an complex type #194

Closed
3 tasks done
alexdeia opened this issue Apr 12, 2024 · 12 comments · Fixed by #230
Closed
3 tasks done
Labels
editor support ✍️ An issue related to a specific editor, extension, editor or IDE, external from gql.tada

Comments

@alexdeia
Copy link

Describe the bug

Hello. I faced problem with types when using fragment spreading.

For example:

If you have this query and want to use in another query your IDE won't discover as a type

export const ItemListFragment = graphql(`
  fragment PageInfo on ItemsConnection {
    pageInfo {
      hasNextPage
      hasPreviousPage
      startCursor
      endCursor
    }
  }
`)
export const ItemListQuery = graphql(`
  query GetItems () {
    items() {
      totalCount
      ...PageInfo
      nodes {
        ...SomeAnotherFragment - same problem
      }
    }
  }
`, [ItemListFragment])

IDE WebStorm error:

{
  nodes: {
    [$tada.fragmentRefs]
  :
    {
      ItemListItem: "Flight";
    }
    ;
  }
  [] | null;
  [$tada.fragmentRefs]
:
  {
    PageInfo: "Undefined Fragment";
  }
  ;
  totalCount: number;
}

But if you use directly the object with its props without the fragment, everything works

export const ItemListQuery = graphql(`
  query GetItems () {
    items() {
      totalCount
      pageInfo {
        hasNextPage
        hasPreviousPage
        startCursor
        endCursor
      }
      nodes {
        ...SomeAnotherFragment - same problem
      }
    }
  }
`, [ItemListFragment])

Reproduction

No response

gql.tada version

gql.tada: 1.4.3

Validations

  • I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
  • Read the docs.
  • Follow our Code of Conduct
@alexdeia alexdeia changed the title Undefined Fragment when spreading is used Undefined Fragment when spreading is used for an complex type Apr 12, 2024
@JoviDeCroock
Copy link
Member

JoviDeCroock commented Apr 12, 2024

Hey,

With these sorts of issues we require you to create a reproduction, I also think your code fragments are either incorrect or out of date as you seem to allude to an issue being present with your description that that one works correctly 😅

But if you use directly the object with its props without the fragment, everything works

Then in the snippet ...SomeAnotherFragment - same problem

We can't really look at this and predict what your issue is as there isn't even remotely enough information nor does your description make sense.

Do note that there are also issues with JetBrains/Webstorm as they implement a custom TS LSP which doesn't have all the features. #80

Also note that doing items() isn't valid GraphQL, not sure if you did this for brevity or that's real though.

@JoviDeCroock JoviDeCroock added the needs more info ✋ A question or report that needs more info to be addressable label Apr 12, 2024
@alexdeia
Copy link
Author

OK, sorry, I'll try to prepare a real example.

@echocrow
Copy link

echocrow commented Apr 17, 2024

We ran into a similar problem in VSCode. Code executes as expected, but the IDE would complain about undefined fragments (when the fragment is defined in another variable and passed in as array in the second argument to graphql()) and unknown directives for e.g. @_unmask.

Turns out we not only had the Syntax Highlighting extension enabled, but also the official LSP extension. Once we disabled the LSP extension, typing for separately defined fragments works as expected. - Perhaps there is an incompatibility with the official GraphQL LSP extension that's worth calling out in the docs?

Long shot, but maybe OP is facing a similar issue in their IDE.

@kitten
Copy link
Member

kitten commented Apr 17, 2024

@echocrow: It's likely not an incompatibility, but rather, the LSP extension is issuing its own warnings that look very similar to ours, given that there's an overlap with the GraphiQL diagnostics that both output.

There's not really much we can do about that.

We can check whether a .vscode/extensions.json file is present in the repo, and whether that contains the plugin during the gql.tada doctor check. But some people may still have it installed for legitimate reasons, and other may have it installed without .vscode/extensions.json being present.

Do you have a sample of your graphql-config file? That's the only other thing we could check for, and I'd assume that'd have to be present for this to become an issue even.

@echocrow
Copy link

@kitten cheers for the response!

(this may be getting a bit off-topic from OP's post - unless OP was facing that exact issue too?)

Well, by "incompatibility" I meant that, as you described, the LSP extension would run its own checks and validation in the IDE. For most basic queries and mutations, that does not pose any issues. However, false errors do arise when using more "advanced" features, particularly around fragments. Those errors get flagged not by gql.tada or TS, but by the separate GraphQL LSP from the official extension. I assume these arise because that LSP does not recognize gql.tada's @_unmask directive, or imported fragments passed into queries via the fragments argument. These errors are safe to ignore once you know about this, but it certainly confused us initially when we evaluated gql.tada (and almost had us reconsider using it, thinking fragments weren't fully supported).

In an ideal world, maybe there's a way to either extend the extension LSP, helping it recognize gql.tada's extra features, or some way of instructing the GraphQL LSP to ignore gql.tada's queries function? I have no clue if either is possible though, as I know little to nothing about how that LSP works under the hood.

Short of a solution to have the two co-exist, that's why I suggested perhaps a note in the docs about this caveat (akin to the note about requiring VSCode to use the workspace TS version).

Do you have a sample of your graphql-config file?

Our exact setup is a little overly complex (monorepo and all). However, we're noticing this issue even with a relatively barebone graphql config file:

// .graphqlrc.ts
export default {
  projects: {
    foo: {
      schema: './packages/foo/schema.graphql',
      documents: './packages/foo/src/components/**/*.graphql',
    },
  },
}

For some reason, even this config has the GraphQL LSP validate e.g. packages/bar/my-gql-tada-query.ts (notice the unrelated foo vs bar directory). Perhaps this is actually a bug in the GraphQL LSP extension(?).

@kitten
Copy link
Member

kitten commented Apr 18, 2024

I just took a look, and this doesn't seem to be reproducible if the output is limited to:
documents: './packages/foo/src/components/**/*.graphql',
i.e. if the documents are limited to *.graphql files. Otherwise, it does seem like by default VSCode GraphQL (or rather graphql-language-service-server) will try to discover documents in *.{ts,tsx} files.

On a side-note, if your .graphqlrc.ts doesn't contain any project entries (or the root entry) that aren't limited to documents, then that's a bug. However, I didn't investigate this further since that'd require me to reproduce a larger config file and reproduction environment. it's possible that a schema entry on the root object of the config without a documents entry is sufficient to cause issues.

There's obviously still the question of whether it's possible to avoid the conflict entirely, and this is likely something I'd like to add to our doctor command, but I'm still looking into what we could do about this

@kitten
Copy link
Member

kitten commented Apr 18, 2024

There doesn't seem to be a better option here than to issue a warning to check that your documents setting in your graphql-config file only includes .graphql files, but that's basically the solution we'll have to go for for now.

@echocrow
Copy link

@kitten I'm pretty confident we ran into this issue even when documents was scoped to just .graphql files, or set to []. I can try to put together a repro later this week if that might be useful?

@kitten
Copy link
Member

kitten commented Apr 18, 2024

That'd be very helpful! 🙌 but potentially something that should also be reported to the graphql-config project itself.

Even if it turns out that, for some reason, documents isn't always fully respected, that's an issue for vscode-graphql and/or graphql-config.

There's also not a good setting in vscode-graphql that could be flipped in .vscode/settings.json (in your repo) to fully force-disable .{ts,tsx} parsing, but I'm hesitant to raise an issue there for now, since there's a limit to how many incompatibilities we can anticipate anyway.

@echocrow
Copy link

@kitten repro available here:

As per your comment, I suspect that this is a bug in the GraphQL LSP, validating gql.tada's GraphQL code, even when JS/TS files are not part of the configured documents.

Issue opened here:

Until (if?) this is addressed, I'd still recommend calling out this caveat in the gql.tada docs. The only workaround we've found so far was to outright disable the VSCode extension.

@kitten
Copy link
Member

kitten commented Apr 20, 2024

@echocrow: we don't have editor-specific docs yet and I have a feeling that's something we'd want to avoid for as long as possible for instead leaning into TS LSP+plugin support.

The problem is, as you can see in the WebStorm issue, it's often external problems and it's unclear how people arrive at solutions. We may even be talking about very personal setups here. While VSCode is the most common, there's also issues with the extension marketplace (for instance the official extensions aren't even the first that pop up in the search)

So, for now, the warnings have been added to the gql.tada doctor command instead, since there we can add setup-specific rules and checks

@echocrow
Copy link

echocrow commented Apr 20, 2024

we don't have editor-specific docs yet

except you kinda do 😅 (there is a VSCode specific note RE using the workspace version of TypeScript; but I understand the aversion, and arguably that note is more crucial and less niche.)

I was picturing a similar note (even if buried on the Fragment Colocation page) that other GraphQL LSPs should be configured to not check these ts(x) files, as they will incorrectly flag gql.tada's features as errors, and compete with gql.tada's auto-completion. (the latter is less of an issue, but we were also confused why ever field was suggested twice, thinking we spotted a minor gql.tada bug. now we know that both gql.tada and GraphQL LSP were echoing suggestions at the same time.) just thought this could be a useful note to other newcomers who may already be using an LSP in the same repo.

in any case, won't press the matter further! 🫡 thanks for the time and support! we have the LSP disabled for now, and hope the extraneous file checking gets fixed sometime (or we migrate fully to gql.tada before!)

(and sorry again @ OP for completely hijacking this issue 🙃)

@kitten kitten changed the title Undefined Fragment when spreading is used for an complex type GraphiQL VSCode warning: Undefined Fragment when spreading is used for an complex type Apr 26, 2024
@kitten kitten added editor support ✍️ An issue related to a specific editor, extension, editor or IDE, external from gql.tada and removed needs more info ✋ A question or report that needs more info to be addressable labels Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editor support ✍️ An issue related to a specific editor, extension, editor or IDE, external from gql.tada
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants