-
Notifications
You must be signed in to change notification settings - Fork 240
fix(protographic): enforce using __typename in composite types for requires #2680
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
Merged
Noroth
merged 13 commits into
main
from
ludwig/eng-9277-include-__typename-for-composite-types-in-protographic
Mar 24, 2026
Merged
Changes from 5 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
69be54b
chore: add linting rule for __typename in requires inline fragments
Noroth 80a3107
chore: remove fix logic
Noroth 6fcbf83
Merge branch 'main' into ludwig/eng-9277-include-__typename-for-compo…
Noroth 67c1619
chore: cleanup code and add union test cases
Noroth 65d4fae
chore: add test coverage
Noroth e8a18b5
chore: only add ancestors to selection set
Noroth 7b54ee0
chore: improve error messages with a path
Noroth 2ba634c
chore: add null check
Noroth 8ab3126
chore: update description
Noroth 7c2d4b5
chore: remove unnecessary enabled field
Noroth ef67ab2
chore: update docs
Noroth 1b530d6
chore: apply linter fixes
Noroth c653b65
chore: remove fix in the visitor
Noroth File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,4 +2,5 @@ dist | |
| out | ||
| node_modules | ||
| .env | ||
| .eslintcache | ||
| .eslintcache | ||
| coverage | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,195 @@ | ||
| import { | ||
| ASTNode, | ||
| ASTVisitor, | ||
| DocumentNode, | ||
| FieldNode, | ||
| GraphQLObjectType, | ||
| GraphQLSchema, | ||
| InlineFragmentNode, | ||
| Kind, | ||
| SelectionSetNode, | ||
| visit, | ||
| print, | ||
| } from 'graphql'; | ||
| import { VisitContext } from './types.js'; | ||
| import { ValidationResult } from './sdl-validation-visitor.js'; | ||
| import { AbstractSelectionRewriter } from './abstract-selection-rewriter.js'; | ||
|
|
||
| /** | ||
| * Validates selection sets within @requires directive field sets. | ||
| * | ||
| * This visitor traverses a parsed field set document and ensures that inline | ||
| * fragments on composite types (interfaces, unions) include `__typename` for | ||
| * type discrimination in protobuf. The `__typename` field can appear either | ||
| * in the parent field's selection set or within each inline fragment's | ||
| * selection set — at least one of these locations must contain it. | ||
| * | ||
| * Before validation, the selection set is normalized by the | ||
| * {@link AbstractSelectionRewriter}, which distributes parent-level fields | ||
| * (including `__typename`) into each inline fragment. | ||
| */ | ||
| export class SelectionSetValidationVisitor { | ||
| private currentFieldSelectionSet: SelectionSetNode | undefined; | ||
| private fieldSelectionSetStack: SelectionSetNode[] = []; | ||
| private readonly operationDocument: DocumentNode; | ||
|
|
||
| private readonly schema: GraphQLSchema; | ||
| private readonly objectType: GraphQLObjectType; | ||
| private readonly fix: boolean = false; | ||
|
endigma marked this conversation as resolved.
Outdated
|
||
|
|
||
| private validationResult: ValidationResult = { | ||
| errors: [], | ||
| warnings: [], | ||
| }; | ||
|
|
||
| /** | ||
| * Creates a new SelectionSetValidationVisitor. | ||
| * | ||
| * @param operationDocument - The parsed GraphQL document representing the field set | ||
| * @param objectType - The root GraphQL object type to validate against | ||
| * @param schema - The full GraphQL schema, used for normalization of abstract type selections | ||
| * @param fix - When true, missing `__typename` fields are added automatically instead of reporting errors | ||
| */ | ||
| constructor(operationDocument: DocumentNode, objectType: GraphQLObjectType, schema: GraphQLSchema, fix: boolean) { | ||
| this.operationDocument = operationDocument; | ||
| this.objectType = objectType; | ||
| this.schema = schema; | ||
| this.fix = fix; | ||
|
|
||
| this.normalizeSelectionSet(); | ||
| } | ||
|
|
||
| /** | ||
| * Executes the validation by traversing the operation document. | ||
| * After calling this method, use `getValidationResult()` to retrieve any errors or warnings. | ||
| */ | ||
| public visit(): void { | ||
| visit(this.operationDocument, this.createASTVisitor()); | ||
| } | ||
|
|
||
| /** | ||
| * Returns the validation result containing any errors and warnings found during traversal. | ||
| * | ||
| * @returns The validation result with errors and warnings arrays | ||
| */ | ||
| public getValidationResult(): ValidationResult { | ||
| return this.validationResult; | ||
| } | ||
|
|
||
| /** | ||
| * Normalizes the parsed field set operation by rewriting abstract selections. | ||
| * This ensures consistent handling of interface and union type selections. | ||
| */ | ||
| private normalizeSelectionSet(): void { | ||
| const visitor = new AbstractSelectionRewriter(this.operationDocument, this.schema, this.objectType); | ||
| visitor.normalize(); | ||
| } | ||
|
|
||
| /** | ||
| * Creates the AST visitor configuration for traversing the document. | ||
| * | ||
| * @returns An ASTVisitor object with enter/leave handlers for SelectionSet nodes | ||
| */ | ||
| private createASTVisitor(): ASTVisitor { | ||
| return { | ||
| SelectionSet: { | ||
| enter: (node, key, parent, path, ancestors) => { | ||
| return this.onEnterSelectionSet({ node, key, parent, path, ancestors }); | ||
| }, | ||
| leave: (node, key, parent, path, ancestors) => { | ||
| this.onLeaveSelectionSet({ node, key, parent, path, ancestors }); | ||
| }, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Handles entering a selection set node during traversal. | ||
| * | ||
| * @param ctx - The visit context containing the selection set node and its parent | ||
| */ | ||
| private onEnterSelectionSet(ctx: VisitContext<SelectionSetNode>): void { | ||
| // When we have no parent, we are at the root of the selection set. | ||
| if (!ctx.parent) { | ||
| return; | ||
| } | ||
|
|
||
| // We store the stack for field selection sets. We ignore the root selection set and the inline fragments in the stack | ||
| if (this.isFieldNode(ctx.parent)) { | ||
| this.currentFieldSelectionSet = ctx.node; | ||
| this.fieldSelectionSetStack.push(ctx.node); | ||
| return; | ||
| } | ||
|
|
||
| // We currently only check for inline fragments. | ||
| if (!this.isInlineFragment(ctx.parent)) { | ||
| return; | ||
| } | ||
|
|
||
| // either the selection set of the inline fragment or the parent selection set must contain __typename. | ||
| if ( | ||
| !this.selectionSetContainsTypename(ctx.node) && | ||
| !this.selectionSetContainsTypename(this.currentFieldSelectionSet) | ||
| ) { | ||
| if (!this.fix) { | ||
| this.validationResult.errors.push( | ||
| `Selection set must contain __typename for inline fragment ${ctx.parent.typeCondition?.name.value}`, | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| this.ensureTypenameInSelection(ctx.node); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| private ensureTypenameInSelection(selectionSet: SelectionSetNode): void { | ||
| selectionSet.selections = [ | ||
|
endigma marked this conversation as resolved.
Outdated
|
||
| { | ||
| kind: Kind.FIELD, | ||
| name: { kind: Kind.NAME, value: '__typename' }, | ||
| }, | ||
| ...selectionSet.selections, | ||
| ]; | ||
| } | ||
|
|
||
| private selectionSetContainsTypename(selectionSet: SelectionSetNode | undefined): boolean { | ||
| if (!selectionSet) { | ||
| return false; | ||
| } | ||
|
|
||
| return selectionSet.selections.some( | ||
| (selection) => selection.kind === Kind.FIELD && selection.name.value === '__typename', | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Handles leaving a selection set node during traversal. | ||
| * Restores the previous field selection set from the stack when leaving a field's selection set. | ||
| * | ||
| * @param ctx - The visit context containing the selection set node and its parent | ||
| */ | ||
| private onLeaveSelectionSet(ctx: VisitContext<SelectionSetNode>): void { | ||
| if (!ctx.parent) { | ||
| return; | ||
| } | ||
|
|
||
| if (this.isFieldNode(ctx.parent)) { | ||
| this.currentFieldSelectionSet = this.fieldSelectionSetStack.pop(); | ||
| } | ||
|
Noroth marked this conversation as resolved.
|
||
| } | ||
|
|
||
| private isInlineFragment(node: ASTNode | readonly ASTNode[]): node is InlineFragmentNode { | ||
| if (Array.isArray(node)) { | ||
| return false; | ||
| } | ||
|
|
||
| return (node as ASTNode).kind === Kind.INLINE_FRAGMENT; | ||
| } | ||
|
|
||
| private isFieldNode(node: ASTNode | ReadonlyArray<ASTNode>): node is FieldNode { | ||
| if (Array.isArray(node)) { | ||
| return false; | ||
| } | ||
| return (node as ASTNode).kind === Kind.FIELD; | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.