Skip to content
GitLab
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • O openapi-generator
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 3,476
    • Issues 3,476
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 402
    • Merge requests 402
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Package Registry
    • Infrastructure Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • OpenAPI Tools
  • openapi-generator
  • Merge requests
  • !2899

[Typescript][Fetch] switch to vars from allVars

  • Review changes

  • Download
  • Email patches
  • Plain diff
Merged Administrator requested to merge github/fork/someone1/fix-2739 into master May 15, 2019
  • Overview 0
  • Commits 1
  • Pipelines 0
  • Changes 1

Created by: someone1

Signed-off-by: Prateek Malhotra someone1@gmail.com

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
  • Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Attempt to address #2739 (closed) - In a previous PR, I switched to allVars in the typescript model templates as vars only included the defined properties for that model, not any properties from any (at that time) inherited classes. Since then, the inheritance/composition models changed internally and vars includes all properties for the model as the model composition is now flattened and no longer inherits from any base class.

This gets the code in a compilable state but there are some worries here:

  • Why is allVars blank?
  • There are "*AllOf.ts" models generated that seem erroneous - these shouldn't be generated.
  • Can we get better testing in place? I suggest using a yaml spec that's more comprehensive and possibly has examples from previous issues found (I think this exists somewhere already in this project, I just can't find it again). I also suggest that for the typescript tests that tsc (or even npm install) is run on the generated code to ensure it compiles. This combination would have found these regressions during development.

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

CC @wing328

Assignee
Assign to
Reviewers
Request review from
Time tracking
Source branch: github/fork/someone1/fix-2739