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

Improve handling of multiple non-null types in buildOperationNodeForField

Open TosinJs opened this issue 1 year ago • 0 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
  • [x] 2. A failing test has been provided
  • [x] 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.

TosinJs avatar Aug 29 '24 21:08 TosinJs