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

printSchemaWithDirectives from `@graphql-tools/utils` uses incorrect descriptions

Open velias opened this issue 2 years 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 yarn 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

Problem is that existing schema descriptions (for fields, objects etc.) modified over schema transformation (eg. using mapSchema) are not printed correctly by printSchemaWithDirectives. This method prefers descriptions from astNode instead from schema config object itself. But astNode is immutable in transformation, only schema config object can be changed.

To Reproduce Steps to reproduce the behavior:

Reproducer is available here https://github.com/velias/graphqlToolsUtils-printSchemaWithDirectives-reproducer

Expected behavior

printSchemaWithDirectives method will use description from schema config object (for fields, objects etc.) in final schema string, not from astNode.

Environment:

  • OS: Ubuntu Linux
  • @graphql-tools/utils: 10.0.5
  • NodeJS: 18

Additional context

velias avatar Aug 11 '23 14:08 velias

Hi @yaacovCR,

I found you as recent functionality committer to the printSchemaWithDirectives code. I'd like to provide some patch to this problem, but I'd like to discuss it first with someone who knows why this code works as it works for now. As I looked through code, I feel that preferred use of astNode description is probably intent, as commits are talking about "schema first" approach? If that's true, would it be a suitable solution to pass some config option to this method, which inverses that logic and will prefer description from schema config object? This way, current behavior will be preserved, but user can also achieve second behavior for cases where he needs it.

Thanks in advance for any help or hint.

velias avatar Sep 15 '23 07:09 velias

direction of comprehensive fix is for mapSchema should be updated to rebuild ast from modified nodes….

Workaround until then is to manually update the AST as you modify it

This is a tricky area because we have multiple sources of truth

yaacovCR avatar Sep 15 '23 07:09 yaacovCR

Thanks, I tried to update AST object, but it is readonly in my code invoked from mapSchema :-( Is there any trick how to modify it?

velias avatar Sep 15 '23 08:09 velias