ardatan / ardatan/graphql-tools

Improve handling of multiple non-null types in buildOperationNodeForField

Open
#6,481 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
5.4k
Forks
830
Avg merge
10h 59m
Merged PRs (30d)
45

Description

### Issue workflow progress

_Progress of the issue based on the
[Contributor Workflow](https://github.com/the-guild-org/Stack/blob/master/CONTRIBUTING.md#a-typical-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`](https://github.com/ardatan/graphql-tools/blob/f60ac886c285e51badcc87c8a9aa47c6da764c44/packages/utils/src/build-operation-for-field.ts#L479C1-L485C4) 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)

```graphql
interface Dessert {
name: String!
}

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

type Cake implements Dessert {
name: String!
specialFeature: [String!]!
}
```

2. Use the `buildOperationNodeForField` function to generate an operation that includes both of these fields
3. 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:

```typescript
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.

Contributor guide

Open the contributing guide

Research direction

Start in packages/utils/src/build-operation-for-field.ts at buildOperationNodeForField and its resolveField logic. Run the linked StackBlitz reproduction and the provided failing test to observe aliases for nested non-null GraphQL types. Done means generated aliases represent every non-null marker distinctly when fields share a path.

Written by the indexing model from the issue text.

Assessment

Tech stack
graphql, typescript
Domain
api, backend-api-design
Issue type
Bug
Difficulty
1/5
Estimated time
Under an hour
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.