Fix circular references in JSON Schema discriminated unions with polymorphic-models-strategy

.chronus/changes/mictaylor-json-schema-discriminator-circular-ref-fix-2025-10-21-6-44-36.md

@@ -0,0 +1,7 @@
+---
+changeKind: fix
+packages:
+  - "@typespec/json-schema"
+---
+
+avoid circular references in discriminated unions with polymorphic-models-strategy

packages/json-schema/src/json-schema-emitter.ts

@@ -113,13 +113,22 @@ export class JsonSchemaEmitter extends TypeEmitter<Record<string, any>, JSONSche
       return this.#createDiscriminatedUnionDeclaration(model, name, discriminator, strategy);
     }
 
+    // Check if base model is using discriminated union strategy
+    // If so, don't use allOf (to avoid circular references), inline properties instead
+    const shouldInlineBase =
+      model.baseModel && this.#isBaseUsingDiscriminatedUnion(model.baseModel);
+
     const schema = this.#initializeSchema(model, name, {
       type: "object",
-      properties: this.emitter.emitModelProperties(model),
-      required: this.#requiredModelProperties(model),
+      properties: shouldInlineBase
+        ? this.#getAllModelProperties(model)
+        : this.emitter.emitModelProperties(model),
+      required: shouldInlineBase
+        ? this.#getAllRequiredModelProperties(model)
+        : this.#requiredModelProperties(model),
     });
 
-    if (model.baseModel) {
+    if (model.baseModel && !shouldInlineBase) {
       const allOf = new ArrayBuilder();
       allOf.push(this.emitter.emitTypeReference(model.baseModel));
       setProperty(schema, "allOf", allOf);
@@ -131,6 +140,16 @@ export class JsonSchemaEmitter extends TypeEmitter<Record<string, any>, JSONSche
     return this.#createDeclaration(model, name, schema);
   }
 
+  #isBaseUsingDiscriminatedUnion(model: Model): boolean {
+    const discriminator = getDiscriminator(this.emitter.getProgram(), model);
+    const strategy = this.emitter.getOptions()["polymorphic-models-strategy"];
+    return (
+      (strategy === "oneOf" || strategy === "anyOf") &&
+      !!discriminator &&
+      model.derivedModels.length > 0
+    );
+  }
+
   modelLiteral(model: Model): EmitterOutput<object> {
     const schema = new ObjectBuilder({
       type: "object",
@@ -187,6 +206,76 @@ export class JsonSchemaEmitter extends TypeEmitter<Record<string, any>, JSONSche
     return requiredProps.length > 0 ? requiredProps : undefined;
   }
 
+  // Get all required properties including inherited ones (for when base uses discriminated union)
+  #getAllRequiredModelProperties(model: Model): string[] | undefined {
+    const requiredProps: string[] = [];
+    const visited = new Set<Model>();
+
+    const collectRequired = (m: Model) => {
+      if (visited.has(m)) return;
+      visited.add(m);
+
+      // Collect from base first
+      if (m.baseModel) {
+        collectRequired(m.baseModel);
+      }
+
+      // Then collect from this model
+      for (const prop of m.properties.values()) {
+        if (!prop.optional && !requiredProps.includes(prop.name)) {
+          requiredProps.push(prop.name);
+        }
+      }
+
+      // Add discriminator property if defined on this model
+      const discriminator = getDiscriminator(this.emitter.getProgram(), m);
+      if (
+        discriminator &&
+        !m.properties.has(discriminator.propertyName) &&
+        !requiredProps.includes(discriminator.propertyName)
+      ) {
+        requiredProps.push(discriminator.propertyName);
+      }
+    };
+
+    collectRequired(model);
+    return requiredProps.length > 0 ? requiredProps : undefined;
+  }
+
+  // Get all properties including inherited ones (for when base uses discriminated union)
+  #getAllModelProperties(model: Model): EmitterOutput<object> {
+    const props = new ObjectBuilder();
+    const visited = new Set<Model>();
+
+    const collectProperties = (m: Model) => {
+      if (visited.has(m)) return;
+      visited.add(m);
+
+      // Collect from base first
+      if (m.baseModel) {
+        collectProperties(m.baseModel);
+      }
+
+      // Then collect from this model (overriding base if needed)
+      for (const [name, prop] of m.properties) {
+        const result = this.emitter.emitModelProperty(prop);
+        setProperty(props, name, result);
+      }
+
+      // Add discriminator property if defined on this model and not already added
+      const discriminator = getDiscriminator(this.emitter.getProgram(), m);
+      if (discriminator && !(discriminator.propertyName in props)) {
+        setProperty(props, discriminator.propertyName, {
+          type: "string",
+          description: `Discriminator property for ${m.name}.`,
+        });
+      }
+    };
+
+    collectProperties(model);
+    return props;
+  }
+
   modelProperties(model: Model): EmitterOutput<object> {
     const props = new ObjectBuilder();
 
@@ -697,7 +786,9 @@ export class JsonSchemaEmitter extends TypeEmitter<Record<string, any>, JSONSche
       variants.push(derivedRef);
     }
 
-    // Include base model properties and the discriminated union
+    // For discriminated unions, emit the oneOf/anyOf with base model properties
+    // Since derived models inline properties (not using allOf), we can include
+    // base properties here without creating circular references
     const schema = this.#initializeSchema(model, name, {
       type: "object",
       properties: this.emitter.emitModelProperties(model),

packages/json-schema/test/discriminator.test.ts

@@ -232,10 +232,10 @@ describe("discriminated union with polymorphic-models-strategy option", () => {
     deepStrictEqual(petSchema.oneOf[0], { $ref: "Cat.json" });
     deepStrictEqual(petSchema.oneOf[1], { $ref: "Dog.json" });
 
-    // Should have base model properties along with oneOf
+    // Pet schema should contain both oneOf and base properties
     strictEqual(petSchema.type, "object");
-    ok(petSchema.properties, "Should have properties");
-    strictEqual(petSchema.properties.name.type, "string");
+    ok(petSchema.properties, "Pet should have properties");
+    strictEqual(petSchema.properties.name.type, "string", "Pet should have name property");
     deepStrictEqual(petSchema.properties.kind, {
       type: "string",
       description: "Discriminator property for Pet.",
@@ -271,13 +271,17 @@ describe("discriminated union with polymorphic-models-strategy option", () => {
     ok(catSchema, "Cat schema should exist");
     strictEqual(catSchema.properties.kind.const, "cat");
     strictEqual(catSchema.properties.meow.type, "integer");
-    deepStrictEqual(catSchema.allOf, [{ $ref: "Pet.json" }]);
+    // Cat should include base properties (name) inline to avoid circular refs
+    strictEqual(catSchema.properties.name.type, "string");
+    strictEqual(catSchema.allOf, undefined, "Should not have allOf to avoid circular refs");
 
     const dogSchema = schemas["Dog.json"];
     ok(dogSchema, "Dog schema should exist");
     strictEqual(dogSchema.properties.kind.const, "dog");
     strictEqual(dogSchema.properties.bark.type, "string");
-    deepStrictEqual(dogSchema.allOf, [{ $ref: "Pet.json" }]);
+    // Dog should include base properties (name) inline to avoid circular refs
+    strictEqual(dogSchema.properties.name.type, "string");
+    strictEqual(dogSchema.allOf, undefined, "Should not have allOf to avoid circular refs");
   });
 
   it("works with multiple levels of inheritance", async () => {
@@ -308,7 +312,9 @@ describe("discriminated union with polymorphic-models-strategy option", () => {
 
     const dogSchema = schemas["Dog.json"];
     ok(dogSchema, "Dog schema should exist");
-    deepStrictEqual(dogSchema.allOf, [{ $ref: "Pet.json" }]);
+    // Dog should inline Pet properties, not use allOf
+    strictEqual(dogSchema.properties.name.type, "string");
+    strictEqual(dogSchema.allOf, undefined, "Should not have allOf to avoid circular refs");
   });
 
   it("does not emit oneOf when option is ignore", async () => {
@@ -381,10 +387,10 @@ describe("discriminated union with polymorphic-models-strategy option", () => {
     deepStrictEqual(petSchema.anyOf[0], { $ref: "Cat.json" });
     deepStrictEqual(petSchema.anyOf[1], { $ref: "Dog.json" });
 
-    // Should have base model properties along with anyOf
+    // Pet schema should contain both anyOf and base properties
     strictEqual(petSchema.type, "object");
-    ok(petSchema.properties, "Should have properties");
-    strictEqual(petSchema.properties.name.type, "string");
+    ok(petSchema.properties, "Pet should have properties");
+    strictEqual(petSchema.properties.name.type, "string", "Pet should have name property");
     deepStrictEqual(petSchema.properties.kind, {
       type: "string",
       description: "Discriminator property for Pet.",