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

printSchemaWithDirectives from @graphql-tools/utils uses incorrect descriptions #5508

Open
1 of 4 tasks
velias opened this issue Aug 11, 2023 · 3 comments
Open
1 of 4 tasks

Comments

@velias
Copy link

velias commented Aug 11, 2023

Issue workflow progress

Progress of the issue based on the
Contributor Workflow

  • 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
Copy link
Author

velias commented Sep 15, 2023

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.

@yaacovCR
Copy link
Collaborator

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

@velias
Copy link
Author

velias commented Sep 15, 2023

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants