Revert "fix(utils/astFromSchema): support GraphQLSchema instances without operation types defined"
#7683
Conversation
…ithout o…" This reverts commit dddc5f6.
🦋 Changeset detectedLatest commit: 9cd34f3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughSummary by CodeRabbit
WalkthroughUpdated schema AST kind selection to treat empty Changes
Sequence Diagram(s)(omitted — changes are a small conditional logic update and a test removal; no new control flow warranting a sequence diagram) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/utils/src/print-schema-with-directives.ts(1 hunks)packages/utils/tests/print-schema-with-directives.spec.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/utils/tests/print-schema-with-directives.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Unit Test on Node 18 (windows-latest) and GraphQL v16
- GitHub Check: Unit Test on Node 18 (ubuntu-latest) and GraphQL v15
- GitHub Check: Unit Test on Node 22 (ubuntu-latest) and GraphQL v16
- GitHub Check: Unit Test on Node 24 (ubuntu-latest) and GraphQL v16
- GitHub Check: Unit Test on Node 18 (ubuntu-latest) and GraphQL v16
- GitHub Check: Full Check on GraphQL v16
- GitHub Check: deployment
|
|
||
| const schemaNode: SchemaDefinitionNode | SchemaExtensionNode = { | ||
| kind: operationTypes.length ? Kind.SCHEMA_DEFINITION : Kind.SCHEMA_EXTENSION, | ||
| kind: operationTypes != null ? Kind.SCHEMA_DEFINITION : Kind.SCHEMA_EXTENSION, |
There was a problem hiding this comment.
Critical: Condition always evaluates to true, making SCHEMA_EXTENSION unreachable.
The check operationTypes != null will always be true because operationTypes is defined on line 173 as an array (the result of filter(isSome)). Arrays are never null—they can only be empty. This means the code will always produce Kind.SCHEMA_DEFINITION, and the SCHEMA_EXTENSION branch is unreachable.
Apply this diff to fix the condition:
- kind: operationTypes != null ? Kind.SCHEMA_DEFINITION : Kind.SCHEMA_EXTENSION,
+ kind: operationTypes.length > 0 ? Kind.SCHEMA_DEFINITION : Kind.SCHEMA_EXTENSION,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| kind: operationTypes != null ? Kind.SCHEMA_DEFINITION : Kind.SCHEMA_EXTENSION, | |
| kind: operationTypes.length > 0 ? Kind.SCHEMA_DEFINITION : Kind.SCHEMA_EXTENSION, |
🤖 Prompt for AI Agents
In packages/utils/src/print-schema-with-directives.ts around line 182, the
ternary uses `operationTypes != null` which is always true because
`operationTypes` is an array; change the condition to check the array length
(e.g., `operationTypes.length > 0`) so that Kind.SCHEMA_DEFINITION is chosen
only when there are operation types and Kind.SCHEMA_EXTENSION is used when the
array is empty.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/tricky-yaks-unite.md (1)
5-5: Wrap the bare URL to satisfy markdown linting.The URL on line 5 is flagged by the markdown linter (MD034) as a bare URL and should be wrapped in angle brackets or converted to a proper markdown link.
Apply this diff to wrap the URL:
-Revert https://github.com/ardatan/graphql-tools/pull/7683 which can cause unexpected breaking +Revert <https://github.com/ardatan/graphql-tools/pull/7683> which can cause unexpected breaking
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/tricky-yaks-unite.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
.changeset/tricky-yaks-unite.md
5-5: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: deployment
- GitHub Check: Unit Test on Node 18 (ubuntu-latest) and GraphQL v15
- GitHub Check: Unit Test on Node 18 (windows-latest) and GraphQL v16
- GitHub Check: Unit Test on Node 18 (ubuntu-latest) and GraphQL v16
- GitHub Check: Unit Test on Node 24 (ubuntu-latest) and GraphQL v16
- GitHub Check: Unit Test on Node 22 (ubuntu-latest) and GraphQL v16
- GitHub Check: Unit Test on Bun
- GitHub Check: Full Check on GraphQL v16
- GitHub Check: alpha / snapshot
💻 Website PreviewThe latest changes are available as preview in: https://pr-7683.graphql-tools.pages.dev |
Reverts #7679 which can cause unexpected breaking
changes so as before the schema extension node will always be converted to a schema definition node