JetBrains / js-graphql-intellij-plugin

GraphQL language support for WebStorm, IntelliJ IDEA and other IDEs based on the IntelliJ Platform.
https://jimkyndemeyer.github.io/js-graphql-intellij-plugin/
MIT License
879 stars 97 forks source link

Add configContributor EP #639

Closed BoD closed 1 year ago

BoD commented 1 year ago

Hi!

I am working on an IntelliJ plugin for Apollo Kotlin. In its current form, the plugin declares a dependency on the GraphQL plugin for its edition and language features.

Now we’d like to improve the integration between the 2 plugins, by allowing the GraphQL plugin to have awareness of the location of GraphQL files that are managed by Apollo Kotlin.

Context

Within an Apollo Kotlin project, GraphQL schema and operation files must be located in specific folders (there’s a default value, which can be customized). It is also possible to declare several independent “services”, which have their own schema. Lastly, multiple modules are supported, in which case operation files can be scattered in different locations.

Current situation

For simple Apollo Kotlin projects (only 1 service), the GraphQL plugin already works well and a .graphqlconfig file isn’t even needed.

For Apollo Kotlin projects with several services, the GraphQL plugin isn’t aware of which schema an operation relates to. For instance, if service A’s schema defines a products field, and B’s schema defines a person field, when editing a query inside A’s folder, the user will have completion suggesting both fields, even though person doesn’t apply. Similarly if both schemas define a type with the same name, it will be underlined as an error, as the plugin thinks it is a re-declaration in the same schema.

This can be resolved by having a .graphqlconfig file in the project, similar to this:

{
  "projects": {
    "serviceA": {
      "includes": ["graphqlSchema/src/main/graphql/serviceA/*"]
    },
    "serviceB": {
      "includes": ["graphqlSchema/src/main/graphql/serviceB/*"]
    }
  }
}

But of course it is a bit of work to create and maintain the file, if the project’s layout change it could be out of sync, and generally, it would be nice if it “just worked out of the box”.

Suggested change

Have a way for plugins to inform the GraphQL plugin of the projects and file includes/excludes.

To that effect, in addition to being able to configure the plugin via the .graphqlconfig file as it exists today, an Extension Point can be added to the plugin, so other plugins can provide their configuration.

This PR

I’m opening this as a draft, as a means to start a discussion and see if I’m on the right track!

A configContributor Extension Point is added, as well as the corresponding GraphQLConfigContributor interface:

public interface GraphQLConfigContributor {
    /**
     * Provide GraphQL configs by path for a given project.
     *
     * @return key: a VirtualFile that must point to a directory, value: the GraphQLConfigData for that directory.
     */
    Map<VirtualFile, GraphQLConfigData> getGraphQLConfigurationsByPath(Project project);
}

Then GraphQLConfigManager.doBuildConfigurationModel() is updated to call getGraphQLConfigurationsByPath and add the returned value to the map being computed.

Note that this is done after reading the .graphqlconfig files, giving them precedence - this gives users the ability to override what’s configured automatically by a plugin, if needed.

Handling configuration changes

At the moment, I am not 100% sure of the conditions that re-trigger doBuildConfigurationModel() to be called, but we probably need a mechanism for a plugin to notify that its configuration has changed, and that therefore getGraphQLConfigurationsByPath must be called again.

Not sure yet if this could be handled with another method in GraphQLConfigContributor (something like setConfigChangeListener maybe), or via another mechanism (maybe the messaging API?).


Anyway, please don’t hesitate to tell me if this makes sense / if I’m missing anything :). Thanks!

vepanimas commented 1 year ago

@BoD Hi! Unfortunately, I'm in the process of a complete rewrite of configuration support right at the moment, I hope to complete it in a week or so. I'm going to add support for the modern graphql-config v3, instead of a legacy one used by the plugin now. I'll look at your PR after, because it doesn't have any sense to add something into GraphQLConfigManager now, it won't exist at all after the migration (GraphQLConfigData also will be removed).

And please note, that the plugin API will be changing significantly in the upcoming months (e.g., type evaluation, resolve, configuration), so I can't guarantee any backward compatibility 😞.

Regarding your proposal, I have one small question. Do you have a real config file in the FS that you want to contribute via your extension point? Or do you just need to override a resolve scope for specific graphql files?

BoD commented 1 year ago

Thanks a lot for replying! All right, it looks like this will have to wait a bit, if an overhaul of the configuration system is underway. And support for the current version of graphql-config is good news! 👍

Regarding your proposal, I have one small question. Do you have a real config file in the FS that you want to contribute via your extension point? Or do you just need to override a resolve scope for specific graphql files?

The idea was to contribute the scopes without any actual config file. So ideally the API of the EP would accept GraphQL ConfigData (or an equivalent in the new system) directly.

vepanimas commented 1 year ago

@BoD Could you please recommend any open-source project with apollo kotlin to test it?

BoD commented 1 year ago

For a sample project with multiple services and modules, have a look at this one.

You can also take a look at Confetti and Hedvig which are real apps, but with a simple Apollo setup (only 1 service).

Don't hesitate to ping me if you have questions!

vepanimas commented 1 year ago

Hi @BoD!

So, I'm still working on migration but a good amount of work is already completed. I added an API that should work exactly as you expect it. Just implement that endpoint in your plugin: https://github.com/JetBrains/js-graphql-intellij-plugin/blob/aecd25792bd423826f096384b3f0c47d94ab4281/src/main/com/intellij/lang/jsgraphql/ide/config/GraphQLConfigContributor.kt#L7-L15

An example implementation is here: https://github.com/JetBrains/js-graphql-intellij-plugin/blob/aecd25792bd423826f096384b3f0c47d94ab4281/src/test/com/intellij/lang/jsgraphql/config/GraphQLConfigScopeTest.kt#L94-L109

Note that it's a test code, so you shouldn't register an extension that way, it should be done using plugin.xml as usual. Also, note that the contributors are called from the background thread, so you need to run a read/write action when your code works with the internal model (e.g., PSI, VFS, project structure, etc).

As I said, the work is not finished yet, so it wouldn't be released for some time. But you still can load the repo, checkout a config-v3 local branch, and depend on these sources directly in your plugin.

A quick manual on how to configure dependencies correctly. So let’s call those plugins corePlugin and dependentPlugin. Leave corePlugin as it is. In dependentPlugin, edit the settings.gradle.kts file and add:

include("/path/to/the/corePlugin")
project(":corePlugin").projectDir = file("/path/to/the/corePlugin")

Then, in the build.gradle.kts:

intellij {
  plugins.set(listOf(
    project(":corePlugin")
  ))
}

You may try providing also a path to unzipped distribution corePlugin archive, but I'm not sure if it would work.

If you have any further questions feel free to reach me in the GraphQL discord channel (or here), you can find me there by the same nickname.

BoD commented 1 year ago

Thanks a lot @vepanimas, this is great! I will have a deeper look soon!

I have one initial quick question: what triggers calling contributeConfigs on plugins? Do you think it would be possible to have an API for a plugin to notify that it needs be called again (e.g. a way to "invalidate" the data)?

The reason I'm asking is that our plugin needs to gather configuration information from the Gradle build file, which can be a lengthy operation, and also can change if the user modifies it. Ideally at that points there would be a way for the Apollo plugin to ping the GraphQL plugin that it has the new config.

vepanimas commented 1 year ago

Ah, forgot to mention that. Of course, you need to pre-compute your data in the background, cache it somewhere, and then just return in the contributor. You can just call com.intellij.lang.jsgraphql.ide.config.GraphQLConfigProvider#scheduleConfigurationReload after, and it will poll the extensions and will update if needed.

Feel free to open a PR to the config-v3 branch if you find an issue or need something to add.

BoD commented 1 year ago

scheduleConfigurationReload

Perfect, I think this should work well!

Thanks again, and I will keep you posted.

BoD commented 1 year ago

Just wanted to give an update that my early testing seem to work as expected 👍.

I'm noticing that the "Schemas and Project Structure" toolwindow looks out of sync: it's showing Schema errors (e.g. redefinitions on Query), even if in the files themselves everything appears to be resolved. I am guessing this is work-in-progress, but just wanted to surface it in case it's unexpected.

Haven't tried scheduleConfigurationReload yet, it's next on my todo :)

vepanimas commented 1 year ago

Could you please describe the configuration of your project in more detail? Technically, reference resolve should work and try to navigate somewhere, whether the schema has errors or not. It also should be reworked, because this behavior is backed by a pretty ugly hack now, but some time ago both the schema build process and resolve silently failed if there were type definitions with the same name.

So, if you have multiple type definitions with the same name in the same scope, they will be reported as an error, and it's not a bug, it's how it is supposed to work now.

BoD commented 1 year ago

Sure! I made a simple version of my test project here.

Here we have 2 services with their own schema, and types A B which are defined in both.

When not using the extension point:

All of this is normal of course, since the plugin doesn't know these are 2 independent projects.

When using the extension point like this:

  override fun contributeConfigs(project: Project): Collection<GraphQLConfig> {
    return listOf(
        GraphQLConfig(
            project = project,
            dir = project.guessProjectDir()!!,
            file = null,
            rawData = GraphQLRawConfig(
                projects = mapOf(
                    // (hard-coded for now to test, but real data will come from our gradle conf)

                    "graphqlSchema/service-a" to GraphQLRawProjectConfig(
                        include = listOf(
                            "graphqlSchema/src/main/graphql/servicea/*.graphql",
                            "graphqlSchema/src/main/graphql/servicea/schema.graphqls",
                        ),
                    ),

                    "graphqlSchema/service-b" to GraphQLRawProjectConfig(
                        include = listOf(
                            "graphqlSchema/src/main/graphql/serviceb/*.graphql",
                            "graphqlSchema/src/main/graphql/serviceb/schema.graphqls",
                        ),
                    ),
                )
            )
        )
    )
  }
vepanimas commented 1 year ago

Thank you, I'll look into that a bit later. Just a quick note for now, in the new graphql config it's better to use schema property to configure where to look for type definitions instead of include. https://the-guild.dev/graphql/config/docs/user/schema

vepanimas commented 1 year ago

I fixed that, please, update your local branch, now it should work as expected.

BoD commented 1 year ago

Yes I can confirm it's working now 🎉

Screenshot 2023-02-22 at 14 53 48
BoD commented 1 year ago

Just wanted to report back that scheduleConfigurationReload is working as expected as well 👍. (Work-in-progress branch is here).

vepanimas commented 1 year ago

Well, that's good to hear 🎉!

I think the work on my side is mostly finished. We'll start testing it, updating the docs, and then release a new version. But I can’t give an exact estimation for that now.

vepanimas commented 1 year ago

Released a new plugin version with the requested changes.