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

Improve handling of multiple non-null types in buildOperationNodeForField #6481

Open
3 of 4 tasks
TosinJs opened this issue Aug 29, 2024 · 0 comments
Open
3 of 4 tasks

Comments

@TosinJs
Copy link

TosinJs commented Aug 29, 2024

Issue workflow progress

Progress of the issue based on the
Contributor Workflow

  • 1. The issue provides a reproduction available on Github, Stackblitz or CodeSandbox
  • 2. A failing test has been provided
  • 3. A local solution has been provided
  • 4. A pull request is pending review

Describe the bug

In the resolveField function, when creating unique aliases for fields with the same path but different types, the code only replaced the first occurrence of ! with NonNull. This could lead to incorrect handling of nested non-null types when the condition fieldTypeMap.has(fieldPathStr) && fieldTypeMap.get(fieldPathStr) !== field.type.toString() is true.

To Reproduce
Steps to reproduce the behavior:

https://stackblitz.com/edit/node-egt5vq?file=index.ts

  1. Create a GraphQL schema with fields that have the same path but different nested non-null types (e.g., field: String! and field: String!! in different types)
  interface Dessert {
    name: String!
  }

  type IceCream implements Dessert {
    name: String!
    specialFeature: String
  }

  type Cake implements Dessert {
    name: String!
    specialFeature: [String!]!
  }
  1. Use the buildOperationNodeForField function to generate an operation that includes both of these fields
  2. Observe that the generated field alias for the second occurrence doesn't replace all occurences of '!' with 'NotNull'

Expected behavior

When the condition fieldTypeMap.has(fieldPathStr) && fieldTypeMap.get(fieldPathStr) !== field.type.toString() is true, all non-null indicators (!) in the field type should be replaced with NonNull in the generated field alias, ensuring unique and correct representation of the field type.

Environment:

  • OS: [Ubuntu 22.04]
  • @graphql-tools/utils: [10.5.4]
  • NodeJS: [18.20.3]

Additional context

The fix involves changing the string replacement method from .replace('!', 'NonNull') to .replace(/!/g, 'NonNull') within the condition:

if (fieldTypeMap.has(fieldPathStr) && fieldTypeMap.get(fieldPathStr) !== field.type.toString()) {
  fieldName += (field.type as any)
    .toString()
    .replace(/!/g, 'NonNull')
    .replace('[', 'List')
    .replace(']', '');
}

This uses a regular expression with the global flag to replace all occurrences of ! in the type string, not just the first one, when creating the unique alias.

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

1 participant