[Question] [language server] implied fragments and support for imports

[p-szm]

In my GraphQL build system that I set up (built on top of graphql-code-generator and @graphql-tools/import) I require explicit fragment imports:

# import SomeFragment from "./some-path.graphql"

query MyQuery {
    someField { ...SomeFragment }
}

I do this in order to minimize the potential for fragment name conflicts and make it easier to share fragments across packages.

However, this language server seems to assume that all fragments are globally available which is not always the case. Is there a way to configure this? If not, would you accept a contribution to make this configurable?

I'm also curious to learn how others deal with sharing fragments across packages. Is using explicit imports good practice? Or is it better to make all fragments globally available and then be careful to make all fragment names unique across the codebase?

[acao]

All excellent questions!

There is no agreed on/specified way to handle fragment imports, so we opted for the easier implied path for now

It would great to support graphql import style syntax as an opt-in feature

[dotansimha]

As part of the group that maintains graphql-codegen and graphql-tools, I have to admit that I'm not a huge fan of the import syntax. It's fine at first but very hard to make it compatible with all tools in the stack.

Also, within GraphiQL - it might not be possible to load a relative/local files...

@ardatan what do you think?

[p-szm]

@dotansimha I can believe that it's hard to implement, and from what I've seen so far all the tooling support for imports is pretty variable (which is understandable given that this is not something that's standarised).

However, I find it hard to imagine a world where we don't support imports. Implied fragments work fine for small codebases, but for large monorepos where fragments are shared between many packages I feel like it's going to be really hard to ensure that all fragments are unique.

[acao]

@dotansimha great points! we do a lot of local file loading in the language server, which lives in this repo, which is what @patrickszmucer is discussing, not the GraphiQL react component as you suggest. It already uses graphql config as you may know

@patrickszmucer this is a non standard specification unfortunately, as @dotansimha points out, it’s hard to get it working with the underlying libraries the language server uses such as graphql-config and graphql-tools. If you’d like to see support for this in the language server, those projects are where you want to start

[p-szm]

@acao seems like graphql-tools has decent support for imports (https://www.graphql-tools.com/docs/api/modules/import_src), but the language server doesn't use the loaders from graphql-tools so it doesn't benefit from that.

I can believe there is some work to be done in graphql-tools and graphl-config, but I think the bulk of the work will be in the language server which right now assumes that all fragments are global. In multiple places (autocomplete, query validation, etc) it just uses the list of all fragments that it finds in the project.

In my project I used the following config to stop the language server from asuming that everything is globally available:

schema: "packages/my-app/schema.graphql"
extensions:
  languageService:
    useSchemaFileDefinitions: true

ie I omitted the documents field (which I'm not actually sure if it's supported, but it seems to work). I think it's just now a matter of teaching the language server to resolve #import comments so that we don't get errors when we try to use fragments that are imported.

If both of you are happy to have some support for this in the language server I can first do a bit more research to list things that would need to be done. Even if we don't end up shipping this feature, I believe it would still be good to refactor the code a bit to open ourselves up of supporting different "modes" ("implied", "#import", or any other specification that will pop up in the future).

[acao]

@dotansimha is a co-maintainer of graphql tools and graphql config (which uses graphql-tools loaders) so I will defer to his judgement on that. There have been extensive discussions in the working groups about why this pattern should be deprecated as an ad-hoc pattern, and why it won’t be accepted as a proposal. There are some alternatives iirc

we use graphql config in the LSP, and graphql-tools partially, thus we are using the default loaders, just not for everything. We will need to use custom loaders anyways when moving entirely to graphql tools for embedded support across languages.

[acao]

It would be cool to support this with a graphql config extension somehow. i think it would need to be handled internally by graphql-tools somehow

the lack of an import pattern is actually very painful for developing a language feature. any file you are editing may depend on any number of non-referenced files throughout a project, so on certain changes you essentially have to re-index all schema files all the time, and even worse all the files with all the operations that contain queries and maybe fragments, because you can't use editor events to determine if these other files have changed because there may be codegen/etc.

using an imports pattern clears this up for us in many ways, but it's also non-standard, and many frameworks have built other conventions around the lack thereof.