Skip to content

Conditional copy properties from parent #1015

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

Merged
merged 3 commits into from
Jul 31, 2015

Conversation

tandrup
Copy link
Contributor

@tandrup tandrup commented Jul 25, 2015

Pull request #946 broke the TypeScript generation with inheritance where a superclass defined an enum. This pull request makes it up to each language to decide the behavior appropriate for the language.

tandrup added 2 commits July 25, 2015 07:45
Pull request swagger-api#946 broke the TypeScript generation with inheritance where a superclass defined an enum. This makes it up to each language to decide the behavior appropriate for the language.
@wing328
Copy link
Contributor

wing328 commented Jul 25, 2015

@tandrup thanks for the PR.

Do you mind giving an example on how allOf (model composition, not inheritance) broke the Typescript code generation?

FYI. #1005 is also related to inheritance and its author happens to be the one who generously contributed the typescript template.

@tandrup
Copy link
Contributor Author

tandrup commented Jul 25, 2015

@wing328 Sure, assume you have this model:

{
  "definitions": {
    "Pet": {
      "discriminator": "petType",
      "properties": {
        "name": { "type": "string" },
        "petType": { "type": "string" },
        "huntingSkill": {
          "type": "string",
          "enum": ["clueless", "lazy", "adventurous"]
        }
      },
      "required": ["name", "petType"]
    }
  },
  "Cat": {
    "allOf": [
      {
        "$ref": "#/definitions/Pet"
      },
      {
        "properties": {
          "miaw": { "type": "integer" }
        }
      }
    ]
  },
  "Dog": {
    "allOf": [
      {
        "$ref": "#/definitions/Pet"
      },
      {
        "properties": {
          "packSize": { "type": "integer" }
        }
      }
    ]
  }
}

In the code by @mhardorf the enum values are represented as an enum for each field. Since there's no notion in the swagger spec of shared enums.

If the generated code does not use inheritance we will end up with a Cat.huntingSkillEnum and a Dog. huntingSkillEnum. But it's not valid typescript to assign a enum of one type to another. You can't do:

var x = Cat.huntingSkillEnum.lazy
x = Dog.huntingSkillEnum.clueless

With inheritance we would only end up with one enum.

@tandrup
Copy link
Contributor Author

tandrup commented Jul 25, 2015

BTW. I'm working together with @mhardorf on the TypeScript generation, so I know about the sorting PR #1005. That pull request targets another problem. TypeScript does not handle it well if the child class comes before the parent class in the code.

@mhardorf
Copy link
Contributor

@tandrup looks good

…copy-from-parent

# Conflicts:
#	modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/TypeScriptNodeClientCodegen.java
wing328 added a commit that referenced this pull request Jul 31, 2015
Conditional copy properties from parent
@wing328 wing328 merged commit dc62f68 into swagger-api:develop_2.0 Jul 31, 2015
ahgittin added a commit to ahgittin/swagger-codegen that referenced this pull request Aug 20, 2015
as per swagger-api#1015 (and swagger-api#946) this is now optional and disabled by default.
this results in extra unnecessary code, masking fields in some cases, and
breaks code generated for jax-rs in certain cases (enums with inheritance).

(not having inheritance
ahgittin added a commit to ahgittin/swagger-codegen that referenced this pull request Aug 20, 2015
as per swagger-api#1015 (and swagger-api#946) this is now optional and disabled by default.
this results in extra unnecessary code, masking fields in some cases, and
breaks code generated for jax-rs in certain cases (enums with inheritance).

(not having inheritance
ahgittin added a commit to ahgittin/swagger-codegen that referenced this pull request Aug 20, 2015
as per swagger-api#1015 (and swagger-api#946) this is now optional and disabled by default.
this results in extra unnecessary code, masking fields in some cases, and
breaks code generated for jax-rs in certain cases (enums with inheritance).
ahgittin added a commit to ahgittin/swagger-codegen that referenced this pull request Aug 20, 2015
as per swagger-api#1015 (and swagger-api#946) this is now optional and disabled by default.
this results in extra unnecessary code, masking fields in some cases, and
breaks code generated for jax-rs in certain cases (enums with inheritance).
ahgittin added a commit to ahgittin/swagger-codegen that referenced this pull request Aug 24, 2015
as per swagger-api#1015 (and swagger-api#946) this is now optional and disabled by default.
this results in extra unnecessary code, masking fields in some cases, and
breaks code generated for jax-rs in certain cases (enums with inheritance).
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.

3 participants