apollographql / apollo-tooling

✏️ Apollo CLI for client tooling (Mostly replaced by Rover)
https://apollographql.com
MIT License
3.04k stars 468 forks source link

vscode-apollo: Find All References doesn't work for FieldDefinition #1418

Open TLadd opened 5 years ago

TLadd commented 5 years ago

Intended outcome:

I have a workspace similar to this open in vscode:

packages/graphql-server
packages/client1
packages/client2

client1 and client2 both have their own apollo.config.js similar to this:

module.exports = {
  client: {
    addTypename: true,
    service: {
      name: "graphql-server",
      localSchemaFile: "schema.json"
    }
  }
};

schema.json is generated using apollo:schema:download.

If I right-click and select Find All References on a field definition in graphql-server, it should find references to that field in queries/mutations defined in client1/client2.

Actual outcome:

Find All References always returns No Results.

How to reproduce the issue:

right-click and select Find All References on a field definition

Versions Running latest master as of Jul 18, 2019 https://github.com/apollographql/apollo-tooling/commit/e700ffad860b01492f885bd6176c6ac8fccd6ecb

So I was debugging the Find All References functionality myself to try and understand how it works. https://github.com/apollographql/apollo-tooling/blob/master/packages/apollo-language-server/src/languageProvider.ts#L387 is the relevant piece of code. It looks to only work if the file you're running the Find All References from is contained within a client project and the type of the node you selected in either a FieldDefinition or FragmentDefinition. And then it looks in that same project for references. The FragmentDefinition block works as expected because fragment definitions are defined in the client.

But for the FieldDefinition case, wouldn't they always be wherever the graphql server is implemented, outside of a client project? I did attempt to manually add the files in graphql-server into my client projects by doing something like this:

module.exports = {
  client: {
    addTypename: true,
    includes: [
      "./packages/client1/src/**/*",
      "./packages/graphql-server/**/*"
    ],
    service: {
      name: "graphql-server",
      localSchemaFile: "schema.json"
    }
  }
};

but I end up getting errors when trying to load the schema.

Error initializing Apollo GraphQL project "graphql-server": Error: Field "Query.account" already exists in the schema. It cannot also be defined in this type extension. Field "Query.accounts" already exists in the schema. It cannot also be defined in this type extension.

Happy to contribute if this is indeed broken and I can get some input on how this is supposed to work.

wtgtybhertgeghgtwtg commented 5 years ago

The part that trips it up is getting the project for the file. I tried a quick and dirty edit to the extension to pick the project, and the feature does work, from there. image So, that half of the problem is a matter of matching SDL to projects. The other half is that Find All References is available on a Field as well as a FieldDefinition, but provideReferences has no code path to deal with the former.

TLadd commented 5 years ago

@wtgtybhertgeghgtwtg For me, it bails out on this line: https://github.com/apollographql/apollo-tooling/blob/master/packages/apollo-language-server/src/languageProvider.ts#L391

if (!isClientProject(project)) return null;

because the project where my schema is defined (the graphql-server) is not a client project.

If you're getting past that, your schema must be defined in the client project as well as the queries? Is that a realistic scenario though? Don't most people have their schema defined in a server project and the queries within client project(s)?

wtgtybhertgeghgtwtg commented 5 years ago

From what I can tell, it needs the project where the queries are defined (so it has the documents to look in for getOperationFieldsFromFieldDefinition), not the one where the schema is. So, for a FieldDefinition, it needs to get a single consuming project.

TLadd commented 5 years ago

The project variable is whatever project Find All References was triggered from: https://github.com/apollographql/apollo-tooling/blob/master/packages/apollo-language-server/src/languageProvider.ts#L354. And then it searches within that project, if it's a client project. So as far as I can tell, the requirements for this to work are:

And I'm confused what real-world projects actually meet those requirements, since I would think the FieldDefinitions would be specified in the server project and the queries in the client project.

wtgtybhertgeghgtwtg commented 5 years ago

I think we're saying the same thing. What I did to get it to work was rigged projectForFile so that it'd give me the client project. When Find All References on foo was triggered in schema.graphql, it got the client project and found references to foo from there. So, if projectForFile can get the consuming client project for the SDL, it will be able to find references in that client project. The problem is that it doesn't do that (since schema.graphql isn't in the includes, and having it in there would cause double declaration), and likely wouldn't have enough information to properly do so in most setups.

venkatd commented 5 years ago

@TLadd per our e-mail discussion I'm adding a reproduction of a related find-all-references issue and a summary of your diagnosis.

find-all-references.zip

Steps to reproduce:

My understanding from your explanation is:

If the schema.graphql file is not part of any project, the code bails out early. However, in the case of where apollo.config.js is configured to use the introspection schema of an endpoint, the generated schema.graphql won't be part of any project.

So this call to this.workspace.projectForFile(uri); would bail out early which is causing the empty results: https://github.com/apollographql/apollo-tooling/blob/335bc695382056d100645649871dd16bbcdfae1d/packages/apollo-language-server/src/languageProvider.ts#L355