Strip T | null unions in mutation engine via replace() (#73)

## Summary

- Use `MutationNode.replace()` to swap `T | null` wrapper unions with the inner type, so downstream code sees the unwrapped type instead of a two-variant union
- Track nullability via the `setNullable()` state map 
- Extract `getNullableUnionType()` from `isNullableWrapper()` to support the `replace()` call

## Motivation

Previously, nullable wrapper unions (`string | null`, `Dog | null`) were kept structurally intact — the mutation called `mutate()` and marked the source union as nullable. This meant downstream code had to re-detect `T | null` structurally. With `replace()`, the union node is swapped out in the mutation graph and parent edges (e.g. a ModelProperty's type) update automatically. After mutation, `property.type` is the `string` scalar, not `string | null`.

## Test plan

- Updated 3 existing nullable union tests to assert `mutatedType.kind` is the inner type (`Scalar` / `Model`) rather than `Union`
- Updated assertions to check `isNullable` on `mutation.mutatedType` instead of the source union
- Added new test: `T | null` on a model property is unwrapped to the inner type at the property level

Author: FionaBronwen

packages/graphql/src/lib/type-utils.ts

@@ -29,19 +29,23 @@ import { camelCase, constantCase, pascalCase, split, splitSeparateNumbers } from
 import { reportDiagnostic } from "../lib.js";
 
 /**
- * Check if a union exists solely to express nullability of a single type.
+ * Extract the inner type from a nullable wrapper union (e.g., `string | null` → `string`).
  * Matches only the `T | null` pattern (exactly 2 variants, one of which is null).
  *
  * These unions are not "real" unions in GraphQL terms — they're just TypeSpec's
- * way of spelling "nullable T". The mutation engine skips further union processing for these.
+ * way of spelling "nullable T". The mutation engine replaces them with the inner type.
  *
  * For multi-variant unions that contain null (e.g. `Cat | Dog | null`),
  * use {@link stripNullVariants} instead.
+ *
+ * @returns The non-null variant type if this is a nullable wrapper, otherwise undefined.
  */
-export function isNullableWrapper(union: Union): boolean {
-  if (union.variants.size !== 2) return false;
+export function unwrapNullableUnion(union: Union): Type | undefined {
+  if (union.variants.size !== 2) return undefined;
   const variants = Array.from(union.variants.values());
-  return variants.some((v) => isNullType(v.type));
+  const nullVariant = variants.find((v) => isNullType(v.type));
+  if (!nullVariant) return undefined;
+  return variants.find((v) => v !== nullVariant)?.type;
 }
 
 /**

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

@@ -13,8 +13,8 @@ import { reportDiagnostic } from "../../lib.js";
 import { setNullable } from "../../lib/nullable.js";
 import { setOneOf } from "../../lib/one-of.js";
 import {
+  unwrapNullableUnion,
   getUnionName,
-  isNullableWrapper,
   sanitizeNameForGraphQL,
   stripNullVariants,
   toTypeName,
@@ -106,12 +106,15 @@ export class GraphQLUnionMutation extends UnionMutation<MutationOptions, any, Mu
 
   mutate() {
     // A nullable wrapper (e.g. `string | null`) is not a real union —
-    // it's just TypeSpec's way of spelling "nullable T". Skip union processing,
-    // but mark as nullable so the emitter knows not to emit `!`.
-    if (isNullableWrapper(this.sourceType)) {
-      setNullable(this.engine.$.program, this.sourceType);
-      this.#mutationNode.mutate();
-      super.mutate();
+    // it's just TypeSpec's way of spelling "nullable T". Replace the union
+    // with the inner type so downstream code sees the unwrapped type.
+    // Nullability is tracked via the state map.
+    const innerType = unwrapNullableUnion(this.sourceType);
+    if (innerType) {
+      this.#mutationNode.replace(innerType);
+      setNullable(this.engine.$.program, this.mutatedType);
+      // Don't call super.mutate() — replace() swaps the union out of the
+      // graph, so there are no variants to iterate.
       return;
     }
 

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

@@ -488,19 +488,21 @@ describe("GraphQL Mutation Engine - Unions", () => {
     tester = await Tester.createInstance();
   });
 
-  it("skips wrapper creation for nullable unions", async () => {
+  it("replaces nullable scalar union with inner type", async () => {
     const { NullableString } = await tester.compile(
       t.code`union ${t.union("NullableString")} { string, null }`,
     );
 
     const engine = createTestEngine(tester.program);
     const mutation = engine.mutateUnion(NullableString, GraphQLTypeContext.Output);
 
+    // T | null is replaced with the inner type (string scalar)
+    expect(mutation.mutatedType.kind).toBe("Scalar");
     expect(mutation.wrapperModels).toHaveLength(0);
-    expect(isNullable(tester.program, NullableString)).toBe(true);
+    expect(isNullable(tester.program, mutation.mutatedType)).toBe(true);
   });
 
-  it("skips union processing for nullable model wrapper", async () => {
+  it("replaces nullable model union with inner type", async () => {
     const { MaybeDog } = await tester.compile(
       t.code`
         model ${t.model("Dog")} { breed: string; }
@@ -511,11 +513,10 @@ describe("GraphQL Mutation Engine - Unions", () => {
     const engine = createTestEngine(tester.program);
     const mutation = engine.mutateUnion(MaybeDog, GraphQLTypeContext.Output);
 
-    // This is a nullable wrapper (Dog | null), not a real union —
-    // it should pass through without union processing
-    expect(mutation.mutatedType.kind).toBe("Union");
+    // Dog | null is replaced with the inner type (Dog model)
+    expect(mutation.mutatedType.kind).toBe("Model");
     expect(mutation.wrapperModels).toHaveLength(0);
-    expect(isNullable(tester.program, MaybeDog)).toBe(true);
+    expect(isNullable(tester.program, mutation.mutatedType)).toBe(true);
   });
 
   it("creates wrapper models for scalar variants", async () => {
@@ -594,6 +595,23 @@ describe("GraphQL Mutation Engine - Unions", () => {
 
     expect(mutation.mutatedType.name).toBe("ValidUnion");
   });
+
+  it("strips T | null on model property to inner type and marks nullable", async () => {
+    const { Foo } = await tester.compile(
+      t.code`model ${t.model("Foo")} { name: string | null; }`,
+    );
+
+    const engine = createTestEngine(tester.program);
+    const mutation = engine.mutateModel(Foo, GraphQLTypeContext.Output);
+
+    // The property's type should be the inner type (string scalar), not the union
+    const nameProp = mutation.mutatedType.properties.get("name");
+    expect(nameProp).toBeDefined();
+    expect(nameProp!.type.kind).toBe("Scalar");
+
+    // The inner type should be marked as nullable
+    expect(isNullable(tester.program, nameProp!.type)).toBe(true);
+  });
 });
 
 describe("GraphQL Mutation Engine - Input/Output Context", () => {
@@ -878,18 +896,6 @@ describe("GraphQL Mutation Engine - oneOf Input Objects", () => {
     expect(model.properties.has("bird")).toBe(true);
   });
 
-  it("nullable union in input context is not replaced", async () => {
-    const { MaybeString } = await tester.compile(
-      t.code`union ${t.union("MaybeString")} { string, null }`,
-    );
-
-    const engine = createTestEngine(tester.program);
-    const mutation = engine.mutateUnion(MaybeString, GraphQLTypeContext.Input);
-
-    // Nullable unions are not real unions — union is kept, not replaced
-    expect(mutation.mutatedType.kind).toBe("Union");
-  });
-
   it("strips null from multi-variant union in output context", async () => {
     const { Pet } = await tester.compile(
       t.code`