Skip to content
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

Fix: Remove spec 3 references keys in imports for TypeScript #1932

Merged
merged 31 commits into from
Jul 29, 2019

Conversation

smasala
Copy link
Contributor

@smasala smasala commented Jan 17, 2019

PR checklist

Description of the PR

Sanitizes model names by removing anyOf and oneOf spec reference keys in model imports.

Fixes: #1913

@TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @nicokoenig (2018/09) @topce (2018/10)

Copy link
Member

@macjohnny macjohnny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@smasala
Copy link
Contributor Author

smasala commented Feb 4, 2019

Override names - requires: #2046

@smasala
Copy link
Contributor Author

smasala commented Feb 4, 2019

@wing328 don't think we are going to be able to create a sensible anyOf|oneOf implementation with the current template injection methodology... see file changes. Output:

import { FirstRuleDefinition | SecondRuleDefinition } from './firstRuleDefinitionSecondRuleDefinition';

export interface CustomRule { 
    ruleId?: string | null;
    definition?: FirstRuleDefinition | SecondRuleDefinition | null;
}

I think we need to approach this in a new manner.
The imports need to be an array and the types shouldn't be manipulated with strings to get the desired output of:

import { FirstRuleDefinition} from './firstRuleDefinition';
import { SecondRuleDefinition } from './secondRuleDefinition';

export interface CustomRule { 
    ruleId?: string | null;
    definition?: FirstRuleDefinition | SecondRuleDefinition | null;
}

@wing328
Copy link
Member

wing328 commented Feb 9, 2019

Let's have another chat in the coming week when you've time.

@smasala
Copy link
Contributor Author

smasala commented Mar 14, 2019

@wing328 updated the solution. Works for TS

@macjohnny
Copy link
Member

@smasala could you please fix the merge conflicts?

@macjohnny
Copy link
Member

@smasala the CI is failing, can you have a look at it?

…rator into name-ref-fix

# Conflicts:
#	modules/openapi-generator/src/test/java/org/openapitools/codegen/typescript/typescriptnode/TypeScriptNodeClientCodegenTest.java
Copy link
Member

@macjohnny macjohnny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@macjohnny macjohnny merged commit b4f1581 into OpenAPITools:master Jul 29, 2019
@wing328 wing328 added this to the 4.1.0 milestone Jul 29, 2019
@smasala smasala deleted the name-ref-fix branch August 1, 2019 08:05
@AlexanderSchiff
Copy link

Hello, is there any chance you could release this to the NPM package? as a minor release? Thanks! @wing328 @macjohnny

@macjohnny
Copy link
Member

this will be released in 4.1.0, probably this week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] anyOf causes incorrect import paths - openapi spec v3.0.0
5 participants