Skip to content

Conversation

@adampetro
Copy link
Contributor

Add path to the fixture input validation error so it is easier to identify where the error is occurring.

@adampetro adampetro force-pushed the ap.add-path-to-input-validation-errors branch from 44743cf to d80bd91 Compare November 7, 2025 21:15
Comment on lines +60 to +68
const valueStack: NestedValue[][] = [[{ value, path: [] }]];
const typeStack: (GraphQLNamedType | undefined)[] = [];
const possibleTypesStack: Set<string>[] = [
new Set([schema.getQueryType()!.name]),
];
const typenameResponseKeyStack: (string | undefined)[] = [];
const expectedFieldsStack: Map<string, Set<string>>[] = [new Map()];

const errors: string[] = [];
const errors: FixtureInputValidationError[] = [];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for improving these types <3

errors.push(
`Cannot validate ${responseKey}: missing field definition`,
);
const currentPath = pathFromAncestors(ancestors);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was really surprised that we couldn't just use the path from the args here.
I added a bit of logging to understand things more, and saw it doesn't contain any of the field names.

  For this query visiting field name:
  query {
    data {
      items {
        name
      }
    }
  }

  Built-in path:
  ['definitions', 0, 'selectionSet', 'selections', 0, 'selectionSet', 'selections', 0, 'selectionSet', 'selections', 0]
  → AST structure path (technical)

  pathFromAncestors(ancestors):
  ['data', 'items', 'name']
  → GraphQL field path (semantic, useful for error messages)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I went through the same thing

@lopert
Copy link
Collaborator

lopert commented Nov 10, 2025

Could you add a pnpm changeset? patch level is still fine since we're still pre-release.

Copy link
Contributor

@saga-dasgupta saga-dasgupta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the path, LGTM.

@adampetro adampetro merged commit 245dbde into main Nov 10, 2025
11 checks passed
@adampetro adampetro deleted the ap.add-path-to-input-validation-errors branch November 10, 2025 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants