feat: introduce graphql linting#2856
Conversation
🦋 Changeset detectedLatest commit: b69ae8e The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Performance Benchmark (Lower is Faster)
|
| '@redocly/cli': minor | ||
| --- | ||
|
|
||
| Added initial support for linting GraphQL SDL schema files (`.graphql` / `.gql`). |
There was a problem hiding this comment.
Let's mark it as experimental for now
| "operation-summary": "error", | ||
| }, | ||
| "graphql": { | ||
| "no-empty-servers": "error", |
There was a problem hiding this comment.
Those are unused rules propagated from the root rules config section.
| import type { Config } from './config/index.js'; | ||
| import { initRules } from './config/rules.js'; | ||
| import { isGraphqlRef } from './graphql/extensions.js'; | ||
| import { runGraphqlRules, type InitializedGraphqlRule } from './graphql/run.js'; |
There was a problem hiding this comment.
let's use dynamic import here so it won't load for everyone and slow them down
There was a problem hiding this comment.
I haven't noticed any perf degradation so far. Would you still like to have it imported dynamcally?
tatomyr
force-pushed
the
feat/lint-graphql
branch
from
7f3750d to
7bd109c
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
tatomyr
force-pushed
the
feat/lint-graphql
branch
2 times, most recently
from
8723a0d to
73e2aff
Compare
tatomyr
force-pushed
the
feat/lint-graphql
branch
from
73e2aff to
6bdd8fe
Compare
| ## GraphQL rules | ||
|
|
||
| To expand the linting checks for a GraphQL schema, enable the built-in GraphQL rules. | ||
| Unlike the shared `struct` rule (configured under `rules`), GraphQL-specific built-in rules are configured under the `graphqlRules` section. |
There was a problem hiding this comment.
Looks like we have mismatch in docs. We can also configure graphQL specific rules under rules section, you are listed this below.
| for (const kind of Object.keys(visitor) as Array<keyof GraphqlVisitor>) { | ||
| const handler = visitor[kind]; | ||
| if (!handler) continue; | ||
| const enter = typeof handler === 'function' ? handler : handler.enter; |
There was a problem hiding this comment.
Did you miss the skip visitor or you leave it for the future implementation?
There was a problem hiding this comment.
I haven't found a proper use case for it in GraphQL. We can implement it later if needed.
| "cookie": "^0.7.2", | ||
| "dotenv": "16.4.7", | ||
| "glob": "^13.0.5", | ||
| "graphql": "^16.14.1", |
There was a problem hiding this comment.
I haven't found where we use this package. Could you please explain the purpose of this package?
There was a problem hiding this comment.
For example, in packages/core/src/graphql/visitor.ts or in packages/core/src/graphql/lint-graphql.ts for parsing the docs AST.
There was a problem hiding this comment.
Oh, you mean in the CLI package? Yes, it was a transitive one, not needed anymore.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2b29c2e. Configure here.
Co-authored-by: Jacek Łękawa <164185257+JLekawa@users.noreply.github.com>
tatomyr
force-pushed
the
feat/lint-graphql
branch
from
2b29c2e to
c580a1b
Compare
| config: Config; | ||
| }): NormalizedProblem[] { | ||
| const { document, config } = opts; | ||
| const source = document.source; |
There was a problem hiding this comment.
Could you please add a guard for the empty GraphQL file to return Unexpected <EOF> error.
|
|
||
| // GraphQL SDL is not a JSON/YAML tree, so it runs through a separate engine. | ||
| if (isGraphqlRef(document.source.absoluteRef)) { | ||
| return lintGraphqlDocument({ document, config }); |
There was a problem hiding this comment.
I think, that we can add dynamic imports for the GraphQL in the lint command, because now we have esbuild and chunks, so the dynamic import should go to the separate chunk. It should decrease the influence of the GraphQL on the performance.
| @@ -481,6 +487,7 @@ function createAssertionDefinitionSubject(nodeNames: string[]): NodeType { | |||
| type: { | |||
| enum: [...new Set(['any', ...nodeNames, 'SpecExtension'])], | |||
| description: 'REQUIRED. Locates the OpenAPI node type that the lint command evaluates.', | |||
There was a problem hiding this comment.
I am wondering if we add assertions for the GraphQL, do we need to add GraphQL mention there? Or currently it's just an experimental feature?
4 participants
What/Why/How?
Added experimental graphql linting.
Reference
Testing
Screenshots (optional)
Check yourself
Security
Note
Medium Risk
New lint code path and
graphqldependency touch corelint/resolve/config; behavior is experimental but isolated from OpenAPI bundle/ref resolution for GraphQL files.Overview
Adds experimental lint support for GraphQL SDL (
.graphql/.gql) in Redocly CLI and@redocly/openapi-core, wired like other spec types but with a dedicated AST pipeline using thegraphqlpackage.Lint path:
.graphql/.gqlfiles skip YAML parsing;lintDocumentroutes them tolintGraphqlDocument, which parses SDL, runs built-in and configurable rules via AST visitors, and reports line/column locations. GraphQL SDL is excluded from bundle$refresolution.Rules & config: New
graphqlspec version withgraphqlRulesin presets (struct,no-unused-types,type-description) plus configurablerule/*assertions targeting GraphQL AST kinds (ObjectTypeDefinition, etc.).no-unresolved-refsis omitted from the GraphQL ruleset. Plugins may exposegraphqlrules; CLIcheckIfRulesetExistcountsgraphqlrules.Docs & release: New lint-graphql guide, lint command/docs updates, sidebar entry, changeset (minor for core + cli). Config validation for invalid
subject.typenow links to docs instead of dumping huge enum lists when values are large.Reviewed by Cursor Bugbot for commit b69ae8e. Bugbot is set up for automated code reviews on this repo. Configure here.