[Typescript][Fetch] switch to vars from allVars

[someone1]

Signed-off-by: Prateek Malhotra [email protected]

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 - 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

[wing328]

Why is allVars blank?

Right. I think it's a bug that needs to be addressed.

[someone1]

Regarding testing - I see that the petstore-with-fake-endpoints-models-for-testing.yaml is run as part of test-fake-petstore-for-all.sh. This file has a lot of good example but does not have the example I am opening a bug report for here.

Does anyone object to these changes?

• Add the allOf composition example that this PR is trying to address (e.g. composition without the discriminator)
• Have some sort of post-process for compiling the typescript generated code after running test-fake-petstore-for-all.sh - does anyone have a suggestion on how to do this? I see that this is only referenced in the Shippable CI config - would it make sense to add a script file and attach it at the end there? Currently, the result it generates can't compile for what seems like a number of different bugs that should probably be addressed.

EDIT:

Actually - I see that a number of generator scripts in ./bin are using petstore-with-fake-endpoints-models-for-testing.yaml over the basic example, maybe we could just switch the typescript samples to do the same and add the compile step to each after?

So the changes would be to:

• Add a new example to petstore-with-fake-endpoints-models-for-testing.yaml
• Update the typescript-fetch* scripts in ./bin to try and compile the code after generating

[macjohnny]

@wing328 is the allVars empty thing still an issue?

[wing328]

Yes, I do not have an ETA so I'll merge this PR into master for the time being.

[nolanlocke]

There are "*AllOf.ts" models generated that seem erroneous - these shouldn't be generated.

Any word on this? This doesn't seem right to me either and was curious if this was intended or another issue needs to be created (if one doesn't exist)

[macjohnny]

@nolanlocke I think this is unintended and should be fixed, too.
There is an issue for that problem: #3338

[wing328]

UPDATE: allVars has been fixed with #3616