-
-
Notifications
You must be signed in to change notification settings - Fork 7.5k
[TypeScript] Implement oneOf type resolution without discriminator #22201
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.
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
Open
ksvirkou-hubspot
wants to merge
31
commits into
OpenAPITools:master
Choose a base branch
from
ksvirkou-hubspot:feature/typestcriptOneOfAlgorithmWithoutDiscriminator
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 12 commits
Commits
Show all changes
31 commits
Select commit
Hold shift + click to select a range
0e6998f
Create OneOfInterface for OneOfModels
ksvirkou-hubspot 451f488
Typescript OneOf: Find matching type if OneClass has no discriminator
ksvirkou-hubspot 74351c7
Typescript: Test oneOf
ksvirkou-hubspot fe10060
cs fixes
ksvirkou-hubspot 234643d
Update samples
ksvirkou-hubspot e03add7
Rewrite OneOfClass to standalone helper TypeMatcher
ksvirkou-hubspot 49e768a
RM unused OneOfClasses
ksvirkou-hubspot 97a775d
RM unused OneOfClasses
ksvirkou-hubspot eee422e
Improve oneOf type categorization in ExtendedCodegenModel
ksvirkou-hubspot 81acbbc
Improve type safety and error reporting for oneOf schema resolution
ksvirkou-hubspot 8e7a128
Restructured model.mustache
ksvirkou-hubspot de4e09e
Update instanceOfType metthod
ksvirkou-hubspot 7da0c2a
Merge master
ksvirkou-hubspot ca525a9
Add oneOf tests for TypeScript generator to CI
ksvirkou-hubspot 0bd35fc
Regenerate samples
ksvirkou-hubspot a117fcc
Regenerate samples
ksvirkou-hubspot c590359
Regenerate samples
ksvirkou-hubspot ded868c
Typescript test OneOf: added pow.xml
ksvirkou-hubspot c806d60
Merge pull request #3 from OpenAPITools/master
ksvirkou-hubspot 1a4b46d
Typescript test OneOf: added tests to CI
ksvirkou-hubspot 8fd54d2
Typescript test OneOf: fix CI errors
ksvirkou-hubspot 6be8f0c
Typescript test OneOf: fix CI errors
ksvirkou-hubspot 69954aa
Typescript test OneOf: fix CI errors
ksvirkou-hubspot 23a2dd7
Typescript test OneOf: fix CI errors
ksvirkou-hubspot 3bfe9cc
Typescript test OneOf: fix CI errors
ksvirkou-hubspot 14d1edc
Typescript test OneOf: fix CI errors
ksvirkou-hubspot d4984c0
Typescript test OneOf: Added module to build tsconfig
ksvirkou-hubspot b04a5ed
Typescript test OneOf: added error to tests
ksvirkou-hubspot f8c65fb
rm error
ksvirkou-hubspot 34c403a
Update tests
ksvirkou-hubspot 3cd8558
Update tests
ksvirkou-hubspot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
11 changes: 11 additions & 0 deletions
11
modules/openapi-generator/src/main/resources/typescript/model/ModelTypes.mustache
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| /** | ||
| * Represents a single attribute/property metadata entry in the attributeTypeMap. | ||
| * Used for validation and type matching in oneOf scenarios. | ||
| */ | ||
| export type AttributeTypeMapEntry = { | ||
| name: string; | ||
| baseName: string; | ||
| type: string; | ||
| format: string; | ||
| required: boolean; | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
36 changes: 36 additions & 0 deletions
36
modules/openapi-generator/src/main/resources/typescript/model/TypeMatcher.mustache
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import { AttributeTypeMapEntry } from '../models/ModelTypes{{importFileExtension}}'; | ||
|
|
||
| /** | ||
| * Validates if data contains all required attributes from the attributeTypeMap. | ||
| * | ||
| * @param data - The data object to validate | ||
| * @param attributeTypeMap - Array of attribute metadata including required flag | ||
| * @returns true if all required attributes are present in data, false otherwise | ||
| */ | ||
| export function instanceOfType(data: any, attributeTypeMap: Array<AttributeTypeMapEntry>): boolean { | ||
| for (const attribute of attributeTypeMap) { | ||
| if (attribute.required && data[attribute.baseName] === undefined) { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Attempts to find a matching type from an array of possible types by validating | ||
| * the data against each type's attribute requirements. | ||
| * | ||
| * @param data - The data object to match | ||
| * @param types - Array of possible type constructors | ||
| * @returns The name of the matching type, or undefined if no match found | ||
| */ | ||
| export function findMatchingType(data: any, types: Array<any>): string | undefined { | ||
| for (const type of types) { | ||
| if (instanceOfType(data, type.getAttributeTypeMap())) { | ||
| return type.name; | ||
| } | ||
| } | ||
|
|
||
| return undefined; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,14 @@ | ||
| {{>licenseInfo}} | ||
| {{#models}} | ||
| {{#model}} | ||
| {{#oneOf}} | ||
| {{#-first}}{{>model/modelOneOf}}{{/-first}} | ||
| {{/oneOf}} | ||
| {{^oneOf}} | ||
| {{#tsImports}} | ||
| import { {{classname}} } from '{{filename}}{{importFileExtension}}'; | ||
| {{/tsImports}} | ||
| import { AttributeTypeMapEntry } from '../models/ModelTypes{{importFileExtension}}'; | ||
| import { HttpFile } from '../http/http{{importFileExtension}}'; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are we sure this is not used in the oneOf case? |
||
|
|
||
| {{#description}} | ||
|
|
@@ -12,10 +17,6 @@ import { HttpFile } from '../http/http{{importFileExtension}}'; | |
| */ | ||
| {{/description}} | ||
| {{^isEnum}} | ||
| {{#oneOf}} | ||
| {{#-first}}{{>model/modelOneOf}}{{/-first}} | ||
| {{/oneOf}} | ||
| {{^oneOf}} | ||
| export class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{ | ||
| {{#vars}} | ||
| {{#description}} | ||
|
|
@@ -46,13 +47,14 @@ export class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{ | |
| {{/hasDiscriminatorWithNonEmptyMapping}} | ||
|
|
||
| {{^isArray}} | ||
| static {{#parent}}override {{/parent}}readonly attributeTypeMap: Array<{name: string, baseName: string, type: string, format: string}> = [ | ||
| static {{#parent}}override {{/parent}}readonly attributeTypeMap: Array<AttributeTypeMapEntry> = [ | ||
| {{#vars}} | ||
| { | ||
| "name": "{{name}}", | ||
| "baseName": "{{baseName}}", | ||
| "type": "{{#isEnum}}{{{datatypeWithEnum}}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}", | ||
| "format": "{{dataFormat}}" | ||
| "format": "{{dataFormat}}", | ||
| "required": {{required}} | ||
| }{{^-last}}, | ||
| {{/-last}} | ||
| {{/vars}} | ||
|
|
@@ -97,7 +99,6 @@ export enum {{classname}}{{enumName}} { | |
| {{/vars}} | ||
|
|
||
| {{/hasEnums}} | ||
| {{/oneOf}} | ||
| {{/isEnum}} | ||
| {{#isEnum}} | ||
| export enum {{classname}} { | ||
|
|
@@ -108,5 +109,6 @@ export enum {{classname}} { | |
| {{/allowableValues}} | ||
| } | ||
| {{/isEnum}} | ||
| {{/oneOf}} | ||
| {{/model}} | ||
| {{/models}} | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,8 @@ | ||
| {{#hasImports}} | ||
| import { | ||
| {{#imports}} | ||
| {{{.}}}{{importFileExtension}}, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this not needed anymore? |
||
| {{/imports}} | ||
| } from './'; | ||
| {{#tsImports}} | ||
| import { {{classname}} } from '{{filename}}{{importFileExtension}}'; | ||
| {{/tsImports}} | ||
| import { findMatchingType } from '../models/TypeMatcher{{importFileExtension}}'; | ||
|
|
||
| {{/hasImports}} | ||
| /** | ||
| * @type {{classname}} | ||
| * Type | ||
|
|
@@ -37,4 +34,16 @@ export class {{classname}}Class { | |
|
|
||
| static readonly mapping: {[index: string]: string} | undefined = undefined; | ||
| {{/hasDiscriminatorWithNonEmptyMapping}} | ||
|
|
||
| private static readonly arrayOfTypes: Array<{{#oneOfModels}}typeof {{{.}}}{{^-last}} | {{/-last}}{{/oneOfModels}}> = [{{#oneOfModels}}{{{.}}}{{^-last}}, {{/-last}}{{/oneOfModels}}]; | ||
|
|
||
| /** | ||
| * Determines which oneOf schema matches the provided data. | ||
| * | ||
| * @param data - The data object to match against oneOf schemas | ||
| * @returns The name of the matching type, or undefined if no unique match is found | ||
| */ | ||
| public static findMatchingType(data: any): string | undefined { | ||
| return findMatchingType(data, this.arrayOfTypes); | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we somehow do without an extended CodegenModel class? everytime the base class is updated (e.g. new properties are added), this would need to be updated here as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, we could consider moving our additional properties to the base class itself.
I've extended the
CodegenModelclass because I need to add new properties specific tooneOfconstructions. As far as I understand, extending the base class is a common approach — for example, thetypescript-fetchgenerator uses the same pattern with its ownExtendedCodegenModelclass.openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptFetchClientCodegen.java
Line 1533 in 03c13fb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe its better to have them in the base class, @wing328 wdyt?