Skip to content

Add path to input validation error #40

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.

Sign up for GitHub

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
adampetro merged 2 commits into from
Nov 10, 2025
Merged

Add path to input validation error #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d80bd91
Add path to input validation error
adampetro Nov 7, 2025
df053cb
Add changeset
adampetro Nov 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fifty-schools-behave.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@shopify/shopify-function-test-helpers": patch
---

Add path to fixture input validation errors
156 changes: 99 additions & 57 deletions src/methods/validate-fixture-input.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,23 @@ import {
visit,
visitWithTypeInfo,
BREAK,
ASTNode,
} from "graphql";

import { inlineNamedFragmentSpreads } from "../utils/inline-named-fragment-spreads.js";

export interface FixtureInputValidationError {
message: string;
path: (string | number)[];
}

export interface ValidateFixtureInputResult {
errors: string[];
errors: FixtureInputValidationError[];
}

interface NestedValue {
value: any;
path: (string | number)[];
}

/**
Expand All @@ -46,15 +57,15 @@ export function validateFixtureInput(
): ValidateFixtureInputResult {
const inlineFragmentSpreadsAst = inlineNamedFragmentSpreads(queryAST);
const typeInfo = new TypeInfo(schema);
const valueStack: any[][] = [[value]];
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[] = [];
Comment on lines +60 to +68
Copy link
Collaborator

@lopert lopert Nov 10, 2025

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


visit(
inlineFragmentSpreadsAst,
Expand Down Expand Up @@ -87,7 +98,7 @@ export function validateFixtureInput(
},
},
Field: {
enter(node) {
enter(node, _key, _parent, _path, ancestors) {
const currentValues = valueStack[valueStack.length - 1];
const nestedValues = [];

Expand All @@ -108,14 +119,19 @@ export function validateFixtureInput(

const fieldDefinition = typeInfo.getFieldDef();
if (fieldDefinition === undefined || fieldDefinition === null) {
errors.push(
`Cannot validate ${responseKey}: missing field definition`,
);
const currentPath = pathFromAncestors(ancestors);
Copy link
Collaborator

@lopert lopert Nov 10, 2025

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

@adampetro adampetro Nov 10, 2025

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

errors.push({
message: `Cannot validate ${responseKey}: missing field definition`,
path: [...currentPath, responseKey],
});
return BREAK;
}
const fieldType = fieldDefinition.type;

for (const currentValue of currentValues) {
for (const {
value: currentValue,
path: currentPath,
} of currentValues) {
const valueForResponseKey = currentValue[responseKey];

// Field is missing from fixture
Expand All @@ -134,7 +150,10 @@ export function validateFixtureInput(
typenameResponseKey,
)
) {
errors.push(`Missing expected fixture data for ${responseKey}`);
errors.push({
message: `Missing expected fixture data for ${responseKey}`,
path: [...currentPath, responseKey],
});
}
}
// Scalars and Enums (including wrapped types)
Expand All @@ -146,8 +165,11 @@ export function validateFixtureInput(
coerceInputValue(
valueForResponseKey,
fieldType,
(path, _invalidValue, error) => {
errors.push(`${error.message} At "${path.join(".")}"`);
(_path, _invalidValue, error) => {
errors.push({
message: error.message,
path: [...currentPath, responseKey],
});
},
);
}
Expand All @@ -169,14 +191,15 @@ export function validateFixtureInput(
processNestedArrays(
valueForResponseKey,
unwrappedFieldType,
responseKey,
[...currentPath, responseKey],
);
nestedValues.push(...flattened);
errors.push(...flattenErrors);
} else {
errors.push(
`Expected array for ${responseKey}, but got ${typeof valueForResponseKey}`,
);
errors.push({
message: `Expected array, but got ${typeof valueForResponseKey}`,
path: [...currentPath, responseKey],
});
}
}
// Objects - validate and add to traversal stack
Expand All @@ -185,29 +208,36 @@ export function validateFixtureInput(
isAbstractType(unwrappedFieldType)
) {
if (valueForResponseKey === null) {
errors.push(
`Expected object for ${responseKey}, but got null`,
);
errors.push({
message: `Expected object, but got null`,
path: [...currentPath, responseKey],
});
} else if (typeof valueForResponseKey === "object") {
nestedValues.push(valueForResponseKey);
nestedValues.push({
value: valueForResponseKey,
path: [...currentPath, responseKey],
});
} else {
errors.push(
`Expected object for ${responseKey}, but got ${typeof valueForResponseKey}`,
);
errors.push({
message: `Expected object, but got ${typeof valueForResponseKey}`,
path: [...currentPath, responseKey],
});
}
}
// Unexpected type - defensive check that should never be reached
else {
errors.push(
`Unexpected type for ${responseKey}: ${unwrappedFieldType}`,
);
errors.push({
message: `Unexpected type, expected ${unwrappedFieldType}`,
path: [...currentPath, responseKey],
});
}
}
// No type information - should not happen with valid query
else {
errors.push(
`Cannot validate ${responseKey}: missing type information`,
);
errors.push({
message: `Cannot validate ${responseKey}: missing type information`,
path: [...currentPath, responseKey],
});
}
}

Expand Down Expand Up @@ -236,7 +266,7 @@ export function validateFixtureInput(
},
},
SelectionSet: {
enter(node, _key, parent) {
enter(node, _key, parent, _path, ancestors) {
// If this SelectionSet belongs to a Field, prepare to track expected fields
if (parent && "kind" in parent && parent.kind === Kind.FIELD) {
expectedFieldsStack.push(new Map());
Expand Down Expand Up @@ -281,9 +311,11 @@ export function validateFixtureInput(
).length;

if (!hasTypename && fragmentSpreadCount > 1) {
errors.push(
`Missing __typename field for abstract type ${getNamedType(typeInfo.getType())?.name}`,
);
const currentPath = pathFromAncestors([...ancestors, parent!]);
errors.push({
message: `Missing __typename field for abstract type ${getNamedType(typeInfo.getType())?.name}`,
path: currentPath,
});
return BREAK;
}
}
Expand Down Expand Up @@ -348,42 +380,43 @@ export function validateFixtureInput(
function processNestedArrays(
value: any[],
listType: GraphQLList<any>,
fieldName: string,
): { values: any[]; errors: string[] } {
const result: any[] = [];
const errors: string[] = [];
path: (string | number)[],
): { values: NestedValue[]; errors: FixtureInputValidationError[] } {
const nestedValues: NestedValue[] = [];
const errors: FixtureInputValidationError[] = [];
const elementType = listType.ofType;

for (const [index, element] of value.entries()) {
if (element === null) {
if (!isNullableType(elementType)) {
errors.push(
`Null value found in non-nullable array at ${fieldName}[${index}]`,
);
errors.push({
message: "Null value found in non-nullable array",
path: [...path, index],
});
}
} else if (isListType(elementType)) {
// Element type is a list - expect nested array and recurse
if (Array.isArray(element)) {
const nested = processNestedArrays(
element,
elementType,
`${fieldName}[${index}]`,
);
result.push(...nested.values);
const nested = processNestedArrays(element, elementType, [
...path,
index,
]);
nestedValues.push(...nested.values);
errors.push(...nested.errors);
} else {
// Error: fixture structure doesn't match schema nesting
errors.push(
`Expected array at ${fieldName}[${index}], but got ${typeof element}`,
);
errors.push({
message: `Expected array, but got ${typeof element}`,
path: [...path, index],
});
}
} else {
// Non-list type - add directly
result.push(element);
nestedValues.push({ value: element, path: [...path, index] });
}
}

return { values: result, errors };
return { values: nestedValues, errors };
}

/**
Expand Down Expand Up @@ -461,13 +494,13 @@ function isValueExpectedForType(
* - No schema lookups needed - possible types were pre-computed during traversal
*/
function checkForExtraFields(
fixtureObjects: any[],
fixtureObjects: NestedValue[],
expectedFields: Map<string, Set<string>>,
typenameResponseKey: string | undefined,
): string[] {
const errors: string[] = [];
): FixtureInputValidationError[] {
const errors: FixtureInputValidationError[] = [];

for (const fixtureObject of fixtureObjects) {
for (const { value: fixtureObject, path } of fixtureObjects) {
if (
typeof fixtureObject === "object" &&
fixtureObject !== null &&
Expand Down Expand Up @@ -502,13 +535,22 @@ function checkForExtraFields(
// Check each field in the fixture object
for (const fixtureField of fixtureFields) {
if (!expectedForThisObject.has(fixtureField)) {
errors.push(
`Extra field "${fixtureField}" found in fixture data not in query`,
);
errors.push({
message: `Extra field "${fixtureField}" found in fixture data not in query`,
path: [...path, fixtureField],
});
}
}
}
}

return errors;
}

function pathFromAncestors(
ancestors: ReadonlyArray<ASTNode | ReadonlyArray<ASTNode>>,
): (string | number)[] {
return ancestors
.filter((ancestor) => "kind" in ancestor && ancestor.kind === Kind.FIELD)
.map((ancestor) => ancestor.alias?.value || ancestor.name.value);
}
7 changes: 5 additions & 2 deletions src/methods/validate-test-assets.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import { determineMutationFromTarget } from "../utils/determine-mutation-from-ta

import { validateInputQuery } from "./validate-input-query.js";
import { validateFixtureOutput } from "./validate-fixture-output.js";
import { validateFixtureInput } from "./validate-fixture-input.js";
import {
validateFixtureInput,
FixtureInputValidationError,
} from "./validate-fixture-input.js";
import { FixtureData } from "./load-fixture.js";

/**
Expand All @@ -28,7 +31,7 @@ export interface CompleteValidationResult {
errors: ReadonlyArray<GraphQLError>;
};
inputFixture: {
errors: string[];
errors: FixtureInputValidationError[];
};
outputFixture: {
errors: { message: string }[];
Expand Down
Loading