Skip to content

Commit 9d43c58

Browse files
committed
Improve mutation engine input/output type handling
- Mutate operations in schema-mutator so type references point to correctly mutated types (input params → mutated input models, return types → mutated output models) - Fix nullable union replacement to mutate inner type with context (Address | null → AddressInput in input context) - Add Input suffix to models in input context with double-suffix prevention (BookInput stays BookInput, not BookInputInput) - Add comprehensive tests for input suffix, nullable union replacement, and operation type references
1 parent 4fe4c58 commit 9d43c58

4 files changed

Lines changed: 183 additions & 11 deletions

File tree

packages/graphql/src/mutation-engine/mutations/model.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,13 @@ export class GraphQLModelMutation extends SimpleModelMutation<SimpleMutationOpti
3434
}
3535

3636
mutate() {
37-
// Apply GraphQL name sanitization
3837
this.mutationNode.mutate((model) => {
38+
// Apply GraphQL name sanitization
3939
model.name = sanitizeNameForGraphQL(model.name);
40+
// Input models get "Input" suffix (unless they already have it)
41+
if (this.typeContext === GraphQLTypeContext.Input && !model.name.endsWith("Input")) {
42+
model.name = model.name + "Input";
43+
}
4044
});
4145
super.mutate();
4246
}

packages/graphql/src/mutation-engine/mutations/union.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,16 @@ export class GraphQLUnionMutation extends UnionMutation<MutationOptions, any, Mu
8787
}
8888

8989
mutate() {
90-
// T | null is not a real union — replace with the inner type.
91-
// Don't mark the replacement as nullable here; it's a shared singleton.
92-
// Nullability is tracked by the container (ModelProperty or Operation).
90+
// T | null is not a real union — replace with the inner type, mutated
91+
// with the same context so that e.g. Address | null → AddressInput in
92+
// input context. Don't mark the replacement as nullable here; it's a
93+
// shared singleton. Nullability is tracked by the container (ModelProperty
94+
// or Operation).
9395
const innerType = unwrapNullableUnion(this.sourceType);
9496
if (innerType) {
95-
this.#mutationNode.replace(innerType);
97+
// Mutate the inner type with the same options (preserving input/output context)
98+
const innerMutation = this.engine.mutate(innerType, this.options);
99+
this.#mutationNode.replace(innerMutation.mutationNode.mutatedType);
96100
return;
97101
}
98102

packages/graphql/src/mutation-engine/schema-mutator.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -240,10 +240,10 @@ export function mutateSchema(
240240
wrapperModels.push(...mutation.wrapperModels);
241241
},
242242
operation: (node: Operation) => {
243-
// Operations pass through unmutated. classifyOperation collects void
244-
// ops for the emitter to report on, and routes the rest by
245-
// @query/@mutation/@subscription.
246-
classifyOperation(node);
243+
// Mutate the operation so parameter types point to mutated input models
244+
// and return types point to mutated output models.
245+
const mutation = engine.mutateOperation(node);
246+
classifyOperation(mutation.mutatedType);
247247
},
248248
});
249249

packages/graphql/test/mutation-engine/graphql-mutation-engine.test.ts

Lines changed: 166 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -776,8 +776,8 @@ describe("GraphQL Mutation Engine - Input/Output Context", () => {
776776

777777
// Different mutation objects (different cache entries)
778778
expect(inputMutation).not.toBe(outputMutation);
779-
// Both produce valid mutated types
780-
expect(inputMutation.mutatedType.name).toBe("Book");
779+
// Input context adds "Input" suffix, output context doesn't
780+
expect(inputMutation.mutatedType.name).toBe("BookInput");
781781
expect(outputMutation.mutatedType.name).toBe("Book");
782782
});
783783

@@ -1137,3 +1137,167 @@ describe("GraphQL Mutation Engine - oneOf Input Objects", () => {
11371137
expect(isNullable(clone)).toBe(true);
11381138
});
11391139
});
1140+
1141+
describe("GraphQL Mutation Engine - Input suffix", () => {
1142+
let tester: Awaited<ReturnType<typeof Tester.createInstance>>;
1143+
beforeEach(async () => {
1144+
tester = await Tester.createInstance();
1145+
});
1146+
1147+
it("adds Input suffix when mutated with Input context", async () => {
1148+
const { Pet } = await tester.compile(t.code`model ${t.model("Pet")} { name: string }`);
1149+
1150+
const engine = createTestEngine(tester.program);
1151+
const inputMutation = engine.mutateModel(Pet, GraphQLTypeContext.Input);
1152+
1153+
expect(inputMutation.mutatedType.name).toBe("PetInput");
1154+
});
1155+
1156+
it("does not add Input suffix when mutated with Output context", async () => {
1157+
const { Pet } = await tester.compile(t.code`model ${t.model("Pet")} { name: string }`);
1158+
1159+
const engine = createTestEngine(tester.program);
1160+
const outputMutation = engine.mutateModel(Pet, GraphQLTypeContext.Output);
1161+
1162+
expect(outputMutation.mutatedType.name).toBe("Pet");
1163+
});
1164+
1165+
it("does not double-suffix models already ending in Input", async () => {
1166+
const { CreateBookInput } = await tester.compile(
1167+
t.code`model ${t.model("CreateBookInput")} { title: string; }`,
1168+
);
1169+
1170+
const engine = createTestEngine(tester.program);
1171+
const inputMutation = engine.mutateModel(CreateBookInput, GraphQLTypeContext.Input);
1172+
1173+
// Should remain "CreateBookInput", not become "CreateBookInputInput"
1174+
expect(inputMutation.mutatedType.name).toBe("CreateBookInput");
1175+
});
1176+
});
1177+
1178+
describe("GraphQL Mutation Engine - Nullable union replacement", () => {
1179+
let tester: Awaited<ReturnType<typeof Tester.createInstance>>;
1180+
beforeEach(async () => {
1181+
tester = await Tester.createInstance();
1182+
});
1183+
1184+
it("replaces nullable union with mutated inner type in input context", async () => {
1185+
const { Container } = await tester.compile(
1186+
t.code`
1187+
model ${t.model("Address")} { street: string; }
1188+
model ${t.model("Container")} { address: Address | null; }
1189+
`,
1190+
);
1191+
1192+
// Get the nullable union type from the property
1193+
const addressProp = Container.properties.get("address")!;
1194+
const nullableUnion = addressProp.type as Union;
1195+
1196+
const engine = createTestEngine(tester.program);
1197+
const unionMutation = engine.mutateUnion(nullableUnion, GraphQLTypeContext.Input);
1198+
1199+
// The replacement should be the mutated model with Input suffix
1200+
expect(unionMutation.mutatedType.kind).toBe("Model");
1201+
expect(unionMutation.mutatedType.name).toBe("AddressInput");
1202+
});
1203+
1204+
it("replaces nullable union with original type name in output context", async () => {
1205+
const { Container } = await tester.compile(
1206+
t.code`
1207+
model ${t.model("Address")} { street: string; }
1208+
model ${t.model("Container")} { address: Address | null; }
1209+
`,
1210+
);
1211+
1212+
// Get the nullable union type from the property
1213+
const addressProp = Container.properties.get("address")!;
1214+
const nullableUnion = addressProp.type as Union;
1215+
1216+
const engine = createTestEngine(tester.program);
1217+
const unionMutation = engine.mutateUnion(nullableUnion, GraphQLTypeContext.Output);
1218+
1219+
// In output context, should be unwrapped to Address (no Input suffix)
1220+
expect(unionMutation.mutatedType.kind).toBe("Model");
1221+
expect(unionMutation.mutatedType.name).toBe("Address");
1222+
});
1223+
});
1224+
1225+
describe("GraphQL Mutation Engine - Operation type references", () => {
1226+
let tester: Awaited<ReturnType<typeof Tester.createInstance>>;
1227+
beforeEach(async () => {
1228+
tester = await Tester.createInstance();
1229+
});
1230+
1231+
it("operation parameters reference mutated input models", async () => {
1232+
const { createBook } = await tester.compile(
1233+
t.code`
1234+
model ${t.model("Book")} { title: string; }
1235+
op ${t.op("createBook")}(book: Book): void;
1236+
`,
1237+
);
1238+
1239+
const engine = createTestEngine(tester.program);
1240+
const mutation = engine.mutateOperation(createBook);
1241+
1242+
// Get the parameter type from the mutated operation
1243+
const bookParam = mutation.mutatedType.parameters.properties.get("book");
1244+
expect(bookParam).toBeDefined();
1245+
1246+
// The parameter type should be the mutated model (BookInput), not the original (Book)
1247+
expect(bookParam!.type.kind).toBe("Model");
1248+
expect((bookParam!.type as Model).name).toBe("BookInput");
1249+
});
1250+
1251+
it("operation return type references mutated output models", async () => {
1252+
const { getBook } = await tester.compile(
1253+
t.code`
1254+
model ${t.model("Book")} { title: string; }
1255+
op ${t.op("getBook")}(): Book;
1256+
`,
1257+
);
1258+
1259+
const engine = createTestEngine(tester.program);
1260+
const mutation = engine.mutateOperation(getBook);
1261+
1262+
// The return type should be the mutated model (Book, no suffix for output)
1263+
expect(mutation.mutatedType.returnType.kind).toBe("Model");
1264+
expect((mutation.mutatedType.returnType as Model).name).toBe("Book");
1265+
});
1266+
1267+
it("operation with both input and output references correct variants", async () => {
1268+
const { updateBook } = await tester.compile(
1269+
t.code`
1270+
model ${t.model("Book")} { title: string; }
1271+
op ${t.op("updateBook")}(book: Book): Book;
1272+
`,
1273+
);
1274+
1275+
const engine = createTestEngine(tester.program);
1276+
const mutation = engine.mutateOperation(updateBook);
1277+
1278+
// Parameter should reference BookInput
1279+
const bookParam = mutation.mutatedType.parameters.properties.get("book");
1280+
expect((bookParam!.type as Model).name).toBe("BookInput");
1281+
1282+
// Return type should reference Book (output variant)
1283+
expect((mutation.mutatedType.returnType as Model).name).toBe("Book");
1284+
});
1285+
1286+
it("operation with nullable parameter references mutated type", async () => {
1287+
const { createOrder } = await tester.compile(
1288+
t.code`
1289+
model ${t.model("Address")} { street: string; }
1290+
op ${t.op("createOrder")}(address: Address | null): void;
1291+
`,
1292+
);
1293+
1294+
const engine = createTestEngine(tester.program);
1295+
const mutation = engine.mutateOperation(createOrder);
1296+
1297+
// The nullable union should be unwrapped and reference the mutated type
1298+
const addressParam = mutation.mutatedType.parameters.properties.get("address");
1299+
expect(addressParam).toBeDefined();
1300+
expect(addressParam!.type.kind).toBe("Model");
1301+
expect((addressParam!.type as Model).name).toBe("AddressInput");
1302+
});
1303+
});

0 commit comments

Comments
 (0)