graphql-tools icon indicating copy to clipboard operation
graphql-tools copied to clipboard

`removeDescriptions` optimizer fails to remove descriptions from some nodes

Open gibson042 opened this issue 2 months ago • 3 comments

Issue workflow progress

Progress of the issue based on the Contributor Workflow

  • [x] 1. The issue provides a reproduction available on Github, Stackblitz or CodeSandbox

    Make sure to fork this template and run npm run generate in the terminal.

    Please make sure the GraphQL Tools package versions 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

optimizeDocumentNode: true fails to remove descriptions for operation/variable/fragment/etc. nodes in request documents.

To Reproduce Steps to reproduce the behavior:

https://stackblitz.com/edit/github-kwqmz3z2?file=document.graphql

Expected behavior

The description nodes are removed, leaving only semantically meaningful syntax.

Environment:

  • @graphql-codegen/client-preset: 5.1.1
  • @graphql-codegen/visitor-plugin-common: 6.1.0
  • @graphql-tools/optimize: 2.0.0

Proposed Fix:

The visit in removeDescriptions should apply transformNode to additional kinds OperationDefinition/VariableDefinition/FragmentDefinition/etc.

gibson042 avatar Nov 05 '25 07:11 gibson042

A summary of the changes CodeRabbit can apply:

  • Update packages/optimize/src/optimizers/remove-description.ts to add TypeScript imports, use a typed generic transformNode, and extend the AST visitor to handle OperationDefinition, VariableDefinition, FragmentDefinition, and SchemaDefinition by returning new nodes with description set to undefined, and add tests in packages/optimize/tests/remove-description.spec.ts covering operation, variable, fragment, and complex documents to verify all descriptions are removed.

  • Update packages/optimize/src/optimizers/remove-description.ts to import DocumentNode and StringValueNode, tighten types, make transformNode generic and return new nodes with description removed, and extend the visitor to handle SchemaDefinition, OperationDefinition, VariableDefinition, and FragmentDefinition; add corresponding tests in packages/optimize/tests/remove-description.spec.ts covering removal of descriptions from operation definitions, variable definitions, fragment definitions, and a complex document.

  • [ ] ✅ Create PR with these edits
  • [ ] 📋 Get copyable edits

coderabbitai[bot] avatar Nov 05 '25 07:11 coderabbitai[bot]

I don't think descriptions are only supported on type definitions. You can try it by running the following code;

import { parse } from 'graphql';

const document = parse(/* GraphQL */`
"OPERATION DESCRIPTION"
query user("VARIABLE DESCRIPTION" $id: ID!) {
    user(id: $id) {
        ...userFields
    }
}

"FRAGMENT DESCRIPTION"
fragment userFields on User {
  id
  username
}
`);

ardatan avatar Nov 11 '25 13:11 ardatan

@ardatan I'm not clear on what you're saying, but that input is a valid GraphQL document (cf. Descriptionopt at OperationDefinition, VariableDefinition, and FragmentDefinition). And the reproduction at https://stackblitz.com/edit/github-kwqmz3z2?file=document.graphql does work, it just doesn't work as well as expected.

gibson042 avatar Nov 15 '25 18:11 gibson042