typescript-axios anytype is not defined #6335

codeserk · 2020-05-16T21:48:40Z

Proposed solution for this issue:
bug typescript AnyType is not defined #6332

PR checklist

Read the contribution guidelines.
If contributing template-only or documentation-only changes which will change sample output, build the project before.
Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@TiFu @taxpon @sebastianhaas @kenisteward @Vrolijkx @macjohnny @nicokoenig @topce @akehir @petejohansonxo @amakhrov

… `oneOf` Exclude if there are other meaningful types

auto-labeler · 2020-05-16T21:48:45Z

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

amakhrov · 2020-05-16T22:08:54Z

...erator/src/main/java/org/openapitools/codegen/languages/AbstractTypeScriptClientCodegen.java

@@ -845,6 +845,11 @@ public String toAllOfName(List<String> names, ComposedSchema composedSchema) {
            }
            return schemaType;
        }).distinct().collect(Collectors.toList());
-        return String.join(" & ", types);
+
+        if (types.size() > 1 && types.contains("any")) {


nit: it might be simpler to add .filter() call to the stream instead of post-processing the resulting list.

EDIT: Aha, I was wrong here. Apparently, if we're about to return a single any type, it's ok - we only want to remove any when it's used in conjuction with some other type (in allOf/oneOf)

amakhrov

Thanks for the PR!
Looks good to me.

I'm a bit vague about impact on allOf / oneOf. Generally, we erase the information that one of the members allOf/oneOf was any.

I'm not sure using any with allOf / oneOf makes any practical sense in the first place, though. So perhaps it's ok.
Would love to hear your thoughts on that!

codeserk · 2020-05-17T09:17:14Z

@amakhrov

🤔 actually I think the root of the problem is in the methods that get types for allOf / oneOf / anyOf, since they return AnyType when they don't find any valid type (properties, ref or type are not set, only description in this case), while I think they should ignore those lines.
I'll dig a bit more into that and I'll also check why so many tests failed!

Also, I'll check what happens when one of the types is a file or anything that is translated to any in typescript, that might lead to some trouble.

I'll keep you updated, but maybe is safer to first just add the translation for AnyType in a first iteration and investigate the root problem further.
With that fix the generated type would be Adult & any which means you probably can't get type information in the IDE (at least not in VSCode), but there won't be any type error, which was the main issue :D

- Included more examples using `oneOf, `allOf`, `anyOf` - Includede examples when types that are translated to `any` are involved (`file`)

codeserk · 2020-05-17T11:17:47Z

@amakhrov

I've pushed some more changes: Now I'm filtering out schemas that are translated to AnyType in the parent, instead of all the types that are any in typescript (because those include types like filter, map, which we don't want to ignore).

I've also extended the sample, to include allOf, anyOf and oneOf. It seems to work as expected: any is only used for file, but not when fields like description are part of the composed schema.
Example:

^ Left: new branch, Right: without changes in AbstractTypeScriptClientCodegen.java - notice that AnyType is not necessary here (and it's also causing type errors)

amakhrov · 2020-05-17T18:32:48Z

@codeserk could you please explain the reasoning behind filtering out AnyType from combined schemas?

codeserk · 2020-05-17T20:36:20Z

@amakhrov yes, I tried to explain in the previous comment 😅

The method that obtains types from the schemas is unable to determine the right type when this is not present (there is no type nor ref present in the schema definition). This happens in composed schemas where some lines just add description, example, etc. They add complementary information but not type:

Example for allOf: adding description to ref and object properties - The resulting type included & AnyType before the changes, caused by the line that adds description.
Example for allOf: adding description to file type - The type for the file was file?: any | AnyTpe before the changes, caused by the line that adds description.
Example for anyOf: adding description when you have multiple refs - The parameter was petByAgePetByTypeAnyType?: PetByAge | PetByType | AnyType before the changes, caused by the line that adds description.
Example for oneOf: adding description to a list of refs - The parameter was catDogAnyType?: Cat | Dog | AnyType before the changes , caused by the line that adds description.

These are all valid OpenAPI v3 definitions, and the result was wrong in all 4 because AnyType should not be present, that line in the schema was indicating a description, not adding another type to the composed schema.

Notice that:

any was not simply removed: There are some types that end in any and they are preserved (there's an example for file type that ends in any, and the same would happen with map and some other types.
AnyType is not removed if that's the only type extracted (I guess in composed schemas without any type information)
There is an active bug related to AnyType not being translated to any so I think AnyType is only used in the cases where the type cannot be determined (which should be few cases (?)).

Hope this clarifies it a bit!
and thanks for your time reviewing :)

amakhrov · 2020-05-18T16:41:11Z

Sounds good, thanks for clarifying!

I feel having indeterminate model in api spec, that results in AnyType, as a part of a combined schema is not something common. Or, frankly, something good at all :).

macjohnny · 2020-05-19T07:56:41Z

samples/client/petstore/typescript-axios/builds/composed-schemas/api.ts

+ * @export
+ * @interface CatAllOf
+ */
+export interface CatAllOf {


where is this interface used?

I think it's not used anywhere,
I've also noticed that, but testing without the changes in this branch generates the same unused interfaces - so I think it's another bug unrelated to this branch

It's probably a manifestation of this old bug: #5171

codeserk · 2020-05-25T07:03:38Z

@amakhrov @macjohnny
Hello, what are the next steps? Do you still see problems in this PR? I so, please let me know so I can fix them :)
Thanks!

* master: (71 commits) [PS] check if JSON properties is defined (#6419) Add C++ UE4 client generator (#6399) Add a link to the article in dev.to (#6421) typescript-axios anytype is not defined (#6335) [Java][jersey2] Make (de)serialization work for oneOf models, add convenience and comparison methods (#6323) Migrate OCaml petstore to use OAS v3 spec (#6348) [Python-experimental] Fix type error if oneof/anyof child schema is null type (#6387) [Python-server] Fix blueplanet 'file not found' error (#6411) [nodejs] Fix deprecation notice when running sample nodejs script (#6412) [java-jersey2] Conditionally include http signature mustache template (#6413) [bug] Fix path provider bug on CI (#6409) decomission nodejs server generator (#6406) [Java] Generate valid code if no Authentication implementations present (#5788) update java jersey2 samples [Java] Fix mustache tag in pom template for HTTP signature (#6404) [Python-experimental] Rename from_server variable to json_variable_naming (#6390) Add a link to medium blog post (#6403) Clean up debug in test (#6398) readding bin/swift5-petstore-readonlyProperties.json remove ./bin/swift5-petstore-readonlyProperties.json ...

* master: [kotlin][client] add support for coroutines with OkHttp (OpenAPITools#6362) update package-json.lock (OpenAPITools#6430) fix hardcoded match type (OpenAPITools#6431) java jersey2 enhance anyOf (OpenAPITools#6420) [Java][jersey2] minor improvement to jersey2 tests (OpenAPITools#6418) ps minor style change (OpenAPITools#6424) [JS] mark ES5 as deprecated (OpenAPITools#6408) [ci] Execute maven and verify with no-snapshot-updates (OpenAPITools#6415) add new file in ts axios samples Migrate all scala generators to use OAS3 (OpenAPITools#6407) migrate ruby samples to oas3 (OpenAPITools#6414) [PS] check if JSON properties is defined (OpenAPITools#6419) Add C++ UE4 client generator (OpenAPITools#6399) Add a link to the article in dev.to (OpenAPITools#6421) typescript-axios anytype is not defined (OpenAPITools#6335) [Java][jersey2] Make (de)serialization work for oneOf models, add convenience and comparison methods (OpenAPITools#6323) Migrate OCaml petstore to use OAS v3 spec (OpenAPITools#6348) [Python-experimental] Fix type error if oneof/anyof child schema is null type (OpenAPITools#6387) [Python-server] Fix blueplanet 'file not found' error (OpenAPITools#6411) [nodejs] Fix deprecation notice when running sample nodejs script (OpenAPITools#6412)

codeserk added 4 commits May 16, 2020 23:27

Include map for AnyType in typescript

f998e61

Exclude any from the list of types extracted from anyOf, allOf,…

1e313c7

… `oneOf` Exclude if there are other meaningful types

Include new scripts and yaml to test the new case

aaae081

Execute the new sample for typescript-axios

1694b9e

auto-labeler bot added Client: TypeScript Issue: Bug labels May 16, 2020

amakhrov reviewed May 16, 2020

View reviewed changes

amakhrov approved these changes May 16, 2020

View reviewed changes

codeserk added 2 commits May 17, 2020 13:08

Filter out only AnyType instead of all any types

a24bf82

Renamed and modified samples

53f099f

- Included more examples using `oneOf, `allOf`, `anyOf` - Includede examples when types that are translated to `any` are involved (`file`)

macjohnny reviewed May 19, 2020

View reviewed changes

macjohnny changed the title ~~Bugfix/6332 typescript anytype is not defined~~ typescript-axios anytype is not defined May 25, 2020

macjohnny merged commit 4dbb5c9 into OpenAPITools:master May 25, 2020

macjohnny mentioned this pull request May 28, 2020

[bug][typescript] AnyType is not defined #6332

Closed

6 tasks

wing328 added this to the 5.0.0 milestone Jul 3, 2020

bodograumann mentioned this pull request Jul 8, 2020

[Typescript] Fix array model of alias to array #6888

Merged

5 tasks

lwj5 mentioned this pull request Aug 3, 2021

[typescript] Fix: any type (8 liner PR) #10080

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

typescript-axios anytype is not defined #6335

typescript-axios anytype is not defined #6335

codeserk commented May 16, 2020 •

edited

Loading

auto-labeler bot commented May 16, 2020

amakhrov May 16, 2020 •

edited

Loading

amakhrov left a comment

codeserk commented May 17, 2020

codeserk commented May 17, 2020

amakhrov commented May 17, 2020

codeserk commented May 17, 2020

amakhrov commented May 18, 2020

macjohnny May 19, 2020

codeserk May 19, 2020

amakhrov May 19, 2020

codeserk commented May 25, 2020

typescript-axios anytype is not defined #6335

typescript-axios anytype is not defined #6335

Conversation

codeserk commented May 16, 2020 • edited Loading

PR checklist

auto-labeler bot commented May 16, 2020

amakhrov May 16, 2020 • edited Loading

Choose a reason for hiding this comment

amakhrov left a comment

Choose a reason for hiding this comment

codeserk commented May 17, 2020

codeserk commented May 17, 2020

amakhrov commented May 17, 2020

codeserk commented May 17, 2020

amakhrov commented May 18, 2020

macjohnny May 19, 2020

Choose a reason for hiding this comment

codeserk May 19, 2020

Choose a reason for hiding this comment

amakhrov May 19, 2020

Choose a reason for hiding this comment

codeserk commented May 25, 2020

codeserk commented May 16, 2020 •

edited

Loading

amakhrov May 16, 2020 •

edited

Loading