Multi-project configurations cannot be linted all at once

[benasher44]

Issue workflow progress

Progress of the issue based on the
Contributor Workflow

• 1. The issue provides a reproduction available on GitHub, Stackblitz or CodeSandbox

  For example, you can start off by editng the
  'basic' example on Stackblitz.

  Please make sure the graphql-eslint version under package.json matches yours.

• 2. A failing test has been provided

• 3. A local solution has been provided

• 4. A pull request is pending review

---

Describe the bug

The lint plugin does load per-project schemas, but it loads it once and ends up reusing it for all files. This results in using one project's schema to lint schemas of other projects.

To Reproduce Steps to reproduce the behavior:

1. Configure multi-project graphql files setup (e.g. src/project1//*.graphql, src/project2//*.graphql)
2. Run lint for the whole folder

Expected behavior

Lint should pass, but it instead claims that (for example) types are unreachable, since it's checking those types against the project that was loaded for the lint pass.

Environment:

• OS: macOS
• @graphql-eslint/eslint-plugin: 3.20.1
• Node.js:

Additional context

Having looked at the code a little bit, it looks like each rule looks at the loaded schema, which will be the schema loaded by the plugin at the start of the lint pass. Instead, each rule should reuse the code that loads the schema from the project-specific schema cache.

[benasher44]

Maybe this is a FlatConfig-specific thing? I can't reproduce this in the example project, but I can definitely reproduce it in ours.

[benasher44]

Confirmed this reproduces in the multiple-project sample project. It doesn't reproduce today because both schemas define a User type. If you just change one of them to be called AdminUser instead, it reproduces:

This will say that the AdminUser is unreachable, if eslint loads the other schema first.

[user1736]

Also run into a similar issue. Any news on the fix?

[user1736]

@dotansimha could you take a look into this? I can allocate some time into fixing it, but I need some guidance from the maintainers as to what the fix should look like.

The problem comes from here. Plugin caches the first config it reads and uses it for everything.
If we were to extend the caching logic the question is what should be the key in that case. Using just path to schema is not enough, while using path to schema and all the operation documents seems like overkill 🤔 I tried to poke and see if it's possible to cache just schema and swap operation documents in and out but underlying classes that handle config are bad fit for it so it ended up being too hacky :(

[dotansimha]

@user1736 can you please share the patch fix you made locally? Or what's the suggested workaround for that? @dimaMachina can help with adjusting it.

[dimaMachina]

@user1736 hi, could you create a minimal reproduction with steps to reproduce issues with caching?

[benasher44]

It's reproducible in the sample project in this repo. See this patch: #2260 (comment)

[user1736]

So for my case specifically I used the following patch:

diff --git a/node_modules/@graphql-eslint/eslint-plugin/cjs/graphql-config.js b/node_modules/@graphql-eslint/eslint-plugin/cjs/graphql-config.js
index 5a18a0a..00fe919
--- a/node_modules/@graphql-eslint/eslint-plugin/cjs/graphql-config.js
+++ b/node_modules/@graphql-eslint/eslint-plugin/cjs/graphql-config.js
@@ -37,7 +37,7 @@ var import_code_file_loader = require("@graphql-tools/code-file-loader");
 var import_debug = __toESM(require("debug"));
 var import_graphql_config = require("graphql-config");
 const debug = (0, import_debug.default)("graphql-eslint:graphql-config");
-let graphQLConfig;
+const configCache = new Map();
 function loadOnDiskGraphQLConfig(filePath) {
   return (0, import_graphql_config.loadConfigSync)({
     // load config relative to the file being linted
@@ -47,9 +47,12 @@ function loadOnDiskGraphQLConfig(filePath) {
     extensions: [codeFileLoaderExtension]
   });
 }
+
 function loadGraphQLConfig(options) {
-  if (graphQLConfig) {
-    return graphQLConfig;
+  // FIXME: Requires revisiting if we stop using parserOptions in eslintrc.js to feed the plugin schema and document files
+  const cacheKey = `${options.schema}#${options.documents?.length}`;
+  if (configCache.has(cacheKey)) {
+    return configCache.get(cacheKey);
   }
   const onDiskConfig = !options.skipGraphQLConfig && loadOnDiskGraphQLConfig(options.filePath);
   debug("options.skipGraphQLConfig: %o", options.skipGraphQLConfig);
@@ -64,13 +67,14 @@ function loadGraphQLConfig(options) {
     include: options.include,
     exclude: options.exclude
   };
-  graphQLConfig = onDiskConfig || new import_graphql_config.GraphQLConfig(
+  const graphQLConfig = onDiskConfig || new import_graphql_config.GraphQLConfig(
     {
       config: configOptions,
       filepath: "virtual-config"
     },
     [codeFileLoaderExtension]
   );
+  configCache.set(cacheKey, graphQLConfig);
   return graphQLConfig;
 }
 const codeFileLoaderExtension = (api) => {

For my configuration having schema name + number of documents is enough to distinguish between config sections, but it's not going to fly as general solution. From my testing disabling cache all together is also not an option since everything slows down to a crawl.

[superpowered]

Hello!

I've also just run into this issue on an internal company project.

Updating:

  if (process.env.NODE_ENV !== 'test' && graphQLConfig) {
    return graphQLConfig;
  }

to:

  if (process.env.NODE_ENV !== 'test' && graphQLConfig && graphQLConfig['_rawConfig'].schema === options.schema) {
    return graphQLConfig;
  }

Seems to be enough to fix it in our use-case at least. But I could see a test for both .schema and .documents matching to be a bit more accurate.

[dimaMachina]

@benasher44 can confirm that no-unreachable-types and no-unused-fields were broken for multi schema projects. Fixed in @graphql-eslint/eslint-plugin@4.0.0-alpha.2 https://github.com/dimaMachina/graphql-eslint/releases/tag/release-1722466173246