[Bug][typescript-fetch] Typescript fetch one of addtl props imports #21656
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #21587
The typescript-fetch client generator has a bug when generating model code where the list of imports will be missing items if the OAS spec includes a schema a
oneOf
section and theoneOf
schema hasadditionalProperties: true
.An example OAS would be
The model code generated for
OneOfResponse.ts
includes the following type:export type OneOfResponse = OptionOne | OptionTwo;
However, the imports in
OneOfResponse.ts
will only have imports forOptionTwo
.The underlying issue is that TypeScriptFetchClientCodegen.processCodeGenModel is creating a set of model imports by filtering out schemas using the
CodegenProperty::getIsModel
method.Schemas which have additional properties set will not be marked as models because IJsonSchemaValidationProperties.setTypeProperties because ModelUtils.isModelWithPropertiesOnly evaluates to false due to the presence of the additional properties.
That probably wasn't the intention of the original PR #21057 (addressing #19909), which was to filter out imports for built-in language primitive types like
string
,object
,Array
, etc.This PR changes the filter used in
TypeScriptFetchClientCodegen
to use.filter(cp -> !cp.getIsPrimitiveType() && !cp.isArrayType())
to filter out schemas for language built-in types or arrays.The language built-in types are defined in
AbstractTypeScriptClientCodegen
as a combination between the -languageSpecificPrimitives variable and the default typeMapping. Types will be mapped first using thetypeMapping
map and then be compared against the list of language specific primitives.I've added a unit test for this bug using the OAS
issue_21587.yaml
which uses aoneOf
composed of a schema withadditionalProperties: true
and also a bunch of primitive types which should hit most of primitives available with the default type mappings.One area I wasn't entirely 100% on is the use of
Set
as a primitive. My understanding is that theSet
type is only available starting with ES6. The typescript client generators have a setting,supportsES6
, which is false by default. I'm not sure how valid it is to haveSet
as a language primitive type ifsupportsES6
is false.PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*
.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master
(upcoming7.x.0
minor release - breaking changes with fallbacks),8.0.x
(breaking changes without fallbacks)"fixes #123"
present in the PR description)Typescript: @TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @topce (2018/10) @akehir (2019/07) @petejohansonxo (2019/11) @amakhrov (2020/02) @davidgamero (2022/03) @mkusaka (2022/04) @joscha (2024/10)
Prior work: PR #21057 - @GregoryMerlet