How to handle circular references caused by polymorphism #456
Comments
|
Hi @evtk ! |
|
Hi @js2me awesome. Let me know for sure If I can be of any assistance! |
|
@evtk I think the best solution of this is Because if we based on your schema with duplicates of usage types inside BlockDTO I have another example without "oneOf" "Pet": {
"type": "object",
"required": [
"pet_type"
],
"properties": {
"pet_type": {
"type": "string"
}
},
"discriminator": {
"propertyName": "pet_type",
"mapping": {
"cachorro": "#/components/schemas/Dog",
"cat": "#/components/schemas/Cat"
}
}
},
"Cat": {
"allOf": [
{
"$ref": "#/components/schemas/Pet"
},
{
"type": "object",
"properties": {
"name": {
"type": "string"
}
}
}
]
},
"Dog": {
"allOf": [
{
"$ref": "#/components/schemas/Pet"
},
{
"type": "object",
"properties": {
"bark": {
"type": "string"
}
}
}
]
},
What do you think ? |
|
@js2me first of all: this is really awesome, never seen this proactive approach on topics like this before. Kuddos! Your proposal reflects my line of thoughts and I will solve the issues existing with the circular ref. I'm trying to understand though why you have chosen to generate the instead of: Only the latter will give me the type errors I would expect:
|
|
@evtk yeah, good point. "Pet": {
"type": "object",
"required": [
"pet_type"
],
"properties": {
"pet_type": {
"type": "string"
}
},
"discriminator": {
"propertyName": "pet_type",
"mapping": {
"cachorro": "#/components/schemas/Dog",
"cat": "#/components/schemas/Cat"
}
}
},
"Cat": {
"allOf": [
{
"$ref": "#/components/schemas/Pet"
},
{
"type": "object",
"properties": {
"name": {
"type": "string"
}
}
}
]
},
"Dog": {
"allOf": [
{
"$ref": "#/components/schemas/Pet"
},
{
"type": "object",
"properties": {
"bark": {
"type": "string"
}
}
}
]
},because currently it generates typescript code like interface AbstractPet {
pet_type: string;
}
export type Pet =
| AbstractPet
| (
| ({
pet_type: "cachorro";
} & Dog)
| ({
pet_type: "cat";
} & Cat)
);
export type Cat = AbstractPet & {
name?: string;
};
export type Dog = AbstractPet & {
bark?: string;
};Which is technically correct |
|
Main problem of this interesting schema is that "Dog": {
"allOf": [
{
"$ref": "#/components/schemas/Pet"
},
{
"type": "object",
"properties": {
"bark": {
"type": "string"
}
}
}
]
} |
|
Yes, well I'm then wondering if that swagger schema for Dog is valid. I think that I would expect the following: when Pet has this mapping inside the descriminator then But I'm not an expert on the validity of those schemas :). I'll contact my co-workers to bring this to their attention and to see what solutions/thoughts they can come up with. |
|
|
@js2me a side note I just discovered while working with the new
So we are assigning |
|
I think what needs to be generated is this, so there is just one enum used.
But I'm curious to know if that can be generated? |
Co-worker calling for duty! 😉 Disclaimer, I'm just as much an expert as @evtk is, so all I can do here is share my line of thoughts. To me it feels like we should focus on the BlockDTO:
required:
- title
type: object
properties:
title:
type: string
nullable: false
discriminator:
propertyName: type
mapping:
csv: '#/components/schemas/CsvBlockDTO'
file: '#/components/schemas/FileBlockDTO'
oneOf:
- $ref: '#/components/schemas/CsvBlockDTO'
- $ref: '#/components/schemas/FileBlockDTO'
CsvBlockDTO:
allOf:
- $ref: '#/components/schemas/BlockDTO'
- required:
- text
- type
type: object
properties:
type:
type: string
default: csv
enum:
- csv
text:
type: string
nullable: false
FileBlockDTO:
allOf:
- $ref: '#/components/schemas/BlockDTO'
- required:
- fileId
- type
type: object
properties:
type:
type: string
default: file
enum:
- file
fileId:
type: string
nullable: false// BlockDTO.discriminator
enum BlockDTOType {
// BlockDTO.discriminator.mapping[key]: BlockDTO.discriminator.mapping[key]
Csv = 'csv',
File = 'file'
}
interface AbstractBlockDTO {
// BlockDTO.discriminator.propertyName
type: BlockDTOType;
// BlockDTO.properties.title
title: string;
}
type BlockDTO = CsvBlockDTO | FileBlockDTO;
type CsvBlockDTO =
// CsvBlockDTO.allOf[0]
AbstractBlockDTO
// CsvBlockDTO.allOf[1]
& {
// CsvBlockDTO.allOf[1].properties.type
// > magic should be applied here to understand that
// 'type' refers to the 'discriminator' which is
// an enum. Referring to 'magic' since the produced
// swagger scheme would make you think it's simply
// an enum with a single value.
//
// Knowing this property is part of the discriminator we could
// do some reverse lookup to match its value against the
// discriminator.mapping. If the value exists (and refers to this schema)
// we take the declared Enum Type. If not we do "something" (create a new type?)
// [CsvBlockDTO.allOf[1].properties.type]: [BlockDTO.discriminator.mapping.key]
type: BlockDTOType.Csv;
// CsvBlockDTO.allOf[1].properties.text
text: string;
}
// same story as CsvBlock
type FileBlockDTO =
// FileBlockDTO.allOf[0]
AbstractBlockDTO
& {
type: BlockDTOType.File;
fileId: string;
}If we can make the btw @js2me very much appreciate your pro-activeness on this ticket - thanks! |
|
@dirkgroenen thanks for this detailed message! Also, using typescript code from your answer we will see the type BlockDTO = CsvBlockDTO | FileBlockDTO;Which is the only result of the oneOf operation. (If we create the same schema without mapping it will generate the same output typescript code) And this part of schema is ignoring as I understand |
Based on your schema, discriminator field is not needed for you, because it output the same result. And yes it can be generated, but need more time to do that :) |
|
Take a look at this example "Pet": {
"type": "object",
"required": [
"pet_type"
],
"properties": {
"pet_type": {
"type": "string"
}
},
"discriminator": {
"propertyName": "pet_type",
"mapping": {
"cachorro": "#/components/schemas/Dog",
"cat": "#/components/schemas/Cat"
}
}
},
"Cat": {
"allOf": [
{
"$ref": "#/components/schemas/Pet"
},
{
"type": "object",
"properties": {
"name": {
"type": "string"
}
}
}
]
},
"Dog": {
"allOf": [
{
"$ref": "#/components/schemas/Pet"
},
{
"type": "object",
"properties": {
"bark": {
"type": "string"
}
}
}
]
},With using your solution output typescript code should looks like type AbstractPet = {
pet_type: string;
}
export type Pet = Cat | Dog
export type Cat = AbstractPet & {
name?: string;
};
export type Dog = AbstractPet & {
bark?: string;
};But this is the main problem because discriminator.mapping is ignored inside the "Pet" schema But based on your swagger schema - pet_type or your "type" field is strictly declared with contstant values |
|
But if I will use the current solution with schema above interface AbstractPet {
pet_type: string;
}
export type Pet = AbstractPet &
(
| ({
pet_type: "cachorro";
} & Dog)
| ({
pet_type: "cat";
} & Cat)
);
export type Cat = AbstractPet & {
name?: string;
};
export type Dog = AbstractPet & {
bark?: string;
};
const pet: Pet = {
pet_type: "cat",
name: "Fluffy",
bark: "BARK BARK BARK"
}Then we will see that this is correctly typescript code |
|
Same thing if we will remove this lines from your swagger schema "BlockDTO": {
"required": [
"title"
],
"type": "object",
"properties": {
"title": {
"type": "string",
"nullable": false
}
},
"discriminator": {
"propertyName": "type",
"mapping": {
"csv": "#/components/schemas/CsvBlockDTO",
"file": "#/components/schemas/FileBlockDTO"
}
},
- "oneOf": [
- {
- "$ref": "#/components/schemas/CsvBlockDTO"
- },
- {
- "$ref": "#/components/schemas/FileBlockDTO"
- }
- ]
},
"CsvBlockDTO": {
"allOf": [
{
"$ref": "#/components/schemas/BlockDTO"
},
{
"required": [
"text",
"type"
],
"type": "object",
"properties": {
"type": {
"type": "string",
"default": "csv",
"enum": [
"csv"
]
},
"text": {
"type": "string",
"nullable": false
}
}
}
]
},
"FileBlockDTO": {
"allOf": [
{
"$ref": "#/components/schemas/BlockDTO"
},
{
"required": [
"fileId",
"type"
],
"type": "object",
"properties": {
"type": {
"type": "string",
"default": "file",
"enum": [
"file"
]
},
"fileId": {
"type": "string",
"nullable": false
}
}
}
]
}Then we will see good typescript safety solution interface AbstractBlockDto {
title: string;
}
export type BlockDTO = AbstractBlockDto &
(
| ({
type: "csv";
} & CsvBlockDTO)
| ({
type: "file";
} & FileBlockDTO)
);
export type CsvBlockDTO = AbstractBlockDto & {
/** @default "csv" */
type: "csv";
text: string;
};
export type FileBlockDTO = AbstractBlockDto & {
/** @default "file" */
type: "file";
fileId: string;
};
const block: BlockDTO = {
type:"file",
fileId: 'asdasd',
title: 'asdasd',
text: "test"
}
|
|
Because export type BlockDTO = AbstractBlockDto &
(
| (CsvBlockDTO | FileBlockDTO) // <--- THIS IS result of BlockDTO.oneOf
| ( // BlockDTO.mapping
| ({
type: "csv";
} & CsvBlockDTO)
| ({
type: "file";
} & FileBlockDTO)
)
); |
|
@js2me thanks! @evtk and I had a chat this morning to go over your answers and took a closer look at the Swagger OpenAPI specification, our example and your Cat/Dog example. We've noticed a few things: 💡 The example we've provided you with (the one that includes an enum for each sub-class) is wrong and not according to the spec. Let's ignore that one for now and focus simply on the Dog/Cat example provided by yourself. 🔴 The produced Typescript in your last comments is not entirely correct. Because there's a union of
🟢 @evtk and I still believe that the correct Typescript code should look like this: // [Pet.discriminator.propertyName]Type
enum PetPetType {
// Pet.discriminator.mapping[key]: Pet.discriminator.mapping[key]
Cachorro = 'cachorro',
Cat = 'Cat'
}
// [Pet.properties]
interface AbstractPet {
// Pet.discriminator.propertyName
// Pet.properties.pet_type
type: PetPetType;
}
interface Cat {
// acknowledged Pet being polymorphic, present of
// discriminator's mapping. "reverse lookup magic"
pet_type: PetPetType.Cat,
// object's own props
name: string;
}
interface Dog {
// acknowledged Pet being polymorphic, present of
// discriminator's mapping. "reverse lookup magic"
pet_type: PetPetType.Cachorro,
// object's own props
bark: string;
}
// Union of all schemas referring to the discriminator Pet
type Pet = Cat | Dog;
// --------------------------------------------------
const pet: Pet = {
pet_type: PetPetType.Cat,
name: "Fluffy",
bark: "BARK BARK BARK" // syntax error
}
const pet: Pet = {
pet_type: PetPetType.Cachorro,
name: "Fluffy", // syntax error
bark: "BARK BARK BARK"
}
const cat: Cat = {
pet_type: PetPetType.Cat, // pet_type is not enforced to be cat
name: 'ms cat',
}
const dog: Dog = {
pet_type: PetPetType.Cachorro, // pet_type is not enforced to be cat
bark: 'woef'
}We've taken another look at the OpenAPI specification and the Cat/Dog example and tried to be as explicit as possible on how to derive the correct interfaces out of it. Please check the below codeblock with the schema and some comments with our thoughts. {
"Pet": {
"type": "object",
"required": [
"pet_type"
],
"properties": {
// "ignored" since pet_type is declared as discriminator
// propertyName.
"pet_type": {
"type": "string"
}
},
// As soon as descriminator is defined:
// - declare interface as mappable and create
// abstract class
"discriminator": {
// declare PetTypeType (pet_typeType) enum based
// on discriminator.propertyName
"propertyName": "pet_type",
// If mapping: add each key to PetTypeType enum
"mapping": {
"cachorro": "#/components/schemas/Dog",
// notice we are not referring to Cat here
}
// What if no mapping?
// According to the spec it will be derived from
// the schemas referring to this one.
}
},
"Cat": {
// merge of all schemas
"allOf": [
{
// references schema with discriminator:
// 1. check if mapping contains reference to schema
// 2. if not: add schema as key to PetTypeType
// "PetTypeType[Cat] = Cat"
"$ref": "#/components/schemas/Pet"
},
{
// acknowledge schema's own types
"type": "object",
"properties": {
"name": {
"type": "string"
}
}
}
]
},
"Dog": {
"allOf": [
{
// references schema with discriminator:
// 1. check if mapping contains reference to schema
// 2. alrady referenced, so no additional action
"$ref": "#/components/schemas/Pet"
},
{
"type": "object",
"properties": {
"bark": {
"type": "string"
}
}
}
]
}
} |
|
@js2me just checking in to see if all the above is clear to you. Is there anything blocking that we can elaborate on? If we can be of any assistance please let us know. Many thanks! |
|
Hey @js2me how are you doing? I see it has been a bit quiet around here and I'm wondering if we can do anything to assist to get things going. We are on the verge of starting big migration of DTO's with the use of OpenAPI so we are definitely looking out for the above feature 👍🏻 |
|
@js2me we eventually changed the way how we generate the schema, creating the Abstract DTO in there, so there isn't a need for the typescript generator to implement this. I'll close the issue. |
Hello, I'm running into issues with circular references caused by a setup that contains polymorphism in the classes. I'm wondering how to handle these cases.
Take the below schema as an example.
BlockDTOhastypeproperty indicating the type ofBlockDTO. It can be aCsvBlockDTOor aFileBlockDTO.Schema
This schema is generated as below, but clearly results in typescript errors due to
BlockDTOreferencingCsvBlockDTOwhileCsvBlockDTOis also referencingBlockDTO.I think the only way to solve this is to create some sort of abstract class, but it isn't an option to configure on this generator. Nor did I find it to be an option on any of the other openapi->typescript generators I found so far.
The text was updated successfully, but these errors were encountered: