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

Caching issue with getSchema function #1232

Closed
4 tasks
Berger92 opened this issue Oct 27, 2022 · 2 comments
Closed
4 tasks

Caching issue with getSchema function #1232

Berger92 opened this issue Oct 27, 2022 · 2 comments
Labels
kind/enhancement New feature or request stage/6-released The issue has been solved on a released version of the library

Comments

@Berger92
Copy link
Contributor

Issue workflow progress

Progress of the issue based on the Contributor Workflow

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

    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

Hi 👋

I've had some issues with tests using eslint's nodejs API and I realized it is because the parser uses a cached schema. This also seems to be the reason why the eslint service needs to be restarted in IDEs when there is a schema change and some rules, e.g no-unreachable-types, don't pick up the change.

I appreciate that caching the schema has performance implications, but without being able to reset the cache it can lead to unexpected behavior.

To Reproduce
Steps to reproduce the behavior:

Valid schema:

type Foo {
  bar: String!
}

type Query {
  foo: Foo!
}

Invalid schema:

type Foo {
  bar: String!
}

type Query {
  hello: String!
}
const pathToValidSchema = '...'
const pathToInvalidSchema = '...'

let eslint = new ESLint({
    useEslintrc: false,
    baseConfig: {
        parser: "@graphql-eslint/eslint-plugin",
        plugins: ["@graphql-eslint"],
        parserOptions: {
            schema: pathToValidSchema,
        },
        rules: {
           "@graphql-eslint/no-unreachable-types": "error"
        }
    },
})

let results = await eslint.lintFiles(pathToValidSchema)

expect(results[0].messages).toHaveLength(0) // ✅ 

eslint = new ESLint({
    useEslintrc: false,
    baseConfig: {
        parser: "@graphql-eslint/eslint-plugin",
        plugins: ["@graphql-eslint"],
        parserOptions: {
            schema: pathToInvalidSchema,
        },
        rules: {
           "@graphql-eslint/no-unreachable-types": "error"
        }
    },
})

results = await eslint.lintFiles(pathToInvalidSchema). 

expect(results[0].messages).toHaveLength(1) // 🟥  <--- This will return 0 errors because it is still using the previous schema

Expected behavior

It should use the new paserOptions.schema and therefore return the correct number of errors.

Environment:

  • OS: MacOS
  • @graphql-eslint/eslint-plugin: 3.12.0
  • Node.js: 16.16

Resolution

  1. As many of the rules have a test guard for caching, getSchema function could have one too.
if (process.env.NODE_ENV !== 'test' && schemaCache.has(schemaKey)) {
    return schemaCache.get(schemaKey);
  }
  1. Caching could be based on content and not filename(s)

While option 1) is quicker to do, option 2) would also make sure that this plugin is working to its fullest in IDEs.

@dimaMachina
Copy link
Contributor

Hi ! Future release with cache invalidation could be tested in the following alpha version 3.13.0-alpha-20221024000319-1089aa0

@dimaMachina
Copy link
Contributor

released in @graphql-eslint/[email protected]

@dimaMachina dimaMachina added stage/6-released The issue has been solved on a released version of the library kind/enhancement New feature or request labels Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request stage/6-released The issue has been solved on a released version of the library
Development

No branches or pull requests

2 participants