fix(typescript): typecheck generated samples + fixes #19903

joscha · 2024-10-18T02:14:16Z

This pull request adds type checking to generated samples via .github/workflows/samples-typescript-typecheck.yaml.

There are various issues in the current generated samples, from simple missing overrides, over incorrect types up to missing code pieces like classes.

I will leave explanations on each fix needed via code comments on the change.

PR checklist

Read the contribution guidelines.
Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
Run the following to build the project and update samples:
```
./mvnw clean package 
./bin/generate-samples.sh ./bin/configs/*.yaml
./bin/utils/export_docs_generators.sh
```
(For Windows users, please run the script in Git BASH)
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.
File the PR against the correct branch: master (upcoming 7.x.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
If your PR is targeting a particular programming language, @mention the technical committee members: @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)

joscha · 2024-10-18T02:21:51Z

.github/workflows/samples-typescript-typecheck.yaml

@@ -0,0 +1,25 @@
+name: TypeScript clients type checks


This workflow runs the type check shell file

joscha · 2024-10-18T02:22:21Z

bin/ts-typecheck-all.sh

+    root_dir=$(git rev-parse --show-toplevel)
+    local dir
+
+    for dir in $(git ls-files samples | grep 'tsconfig.json$' | xargs -n1 dirname | sort -u); do


We find all checked in samples that have a tsconfig.json

joscha · 2024-10-18T02:23:26Z

bin/ts-typecheck-all.sh

+        log "➤ ${dir}"
+        pushd "${root_dir}/${dir}" > /dev/null
+        npm_install \
+            || npm_install --force # --force because we have some incompatible peer-dependencies that can't be fixed


you can see in the CI logs, but basically there are a couple really really outdated dependencies, which can't even be updated anymore, like https://github.com/angular/tsickle/ for example. Hence we need to sometimes override incompatible peer dependencies, etc.

joscha · 2024-10-18T02:23:46Z

bin/ts-typecheck-all.sh

+        pushd "${root_dir}/${dir}" > /dev/null
+        npm_install \
+            || npm_install --force # --force because we have some incompatible peer-dependencies that can't be fixed
+        npm exec [email protected] --yes -- tsc --noEmit


this does the actual type checking and fails the job if there are errors

joscha · 2024-10-18T02:26:32Z

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

        typeMapping.put("Array", "Array");
        typeMapping.put("array", "Array");
        typeMapping.put("boolean", "boolean");
+        typeMapping.put("decimal", "string");


without this change,

openapi-generator/modules/openapi-generator/src/test/resources/3_0/snakecase-discriminator.yaml

Lines 1556 to 1558 in 6b0e581

decimal:

type: string

format: number

for example would exopect a full-blown Decimal class, which is never generated. You can see code references to it that are not resolved as the file is not there. For example:

openapi-generator/samples/client/petstore/typescript-fetch/builds/snakecase-discriminator/models/FormatTest.ts

Line 16 in ce09134

import type { Decimal } from './Decimal';

joscha · 2024-10-18T02:27:00Z

modules/openapi-generator/src/main/resources/typescript-aurelia/Api.ts.mustache

  protected ensureParamIsSet<T>(context: string, params: T, paramName: keyof T): void {
    if (null === params[paramName]) {
-      throw new Error(`Missing required parameter ${paramName} when calling ${context}`);
+      throw new Error(`Missing required parameter ${String(paramName)} when calling ${context}`);


Fixes:

Api.ts:73:53 - error TS2731: Implicit conversion of a 'symbol' to a 'string' will fail at runtime. Consider wrapping this expression in 'String(...)'. 73 throw new Error(`Missing required parameter ${paramName} when calling ${context}`); ~~~~~~~~~ Found 1 error in Api.ts:73

joscha · 2024-10-18T02:27:42Z

modules/openapi-generator/src/main/resources/typescript-fetch/runtime.mustache

    return `${encodeURIComponent(fullKey)}=${encodeURIComponent(String(value))}`;
 }

+export function exists(json: any, key: string) {


Fixes:

src/models/GetPetRegionsResponse.ts:15:10 - error TS2305: Module '"../runtime"' has no exported member 'exists'. 15 import { exists, mapValues } from '../runtime'; ~~~~~~

joscha · 2024-10-18T02:28:21Z

modules/openapi-generator/src/main/resources/typescript-fetch/runtimeSagasAndRecords.mustache

            return knownIndexedSetByKey.indexOf(key) !== -1 ? value.toSet() : value.toList();
        } // we're reviving an array -> it's a List
-        const MatchingType = knownRecordFactories.get(value.get('recType')) as { new(input?: any): any }; // check if we know a Record with this type
+        const MatchingType = knownRecordFactories.get(value.get('recType') as string) as { new(input?: any): any }; // check if we know a Record with this type


Fixes

src/runtimeSagasAndRecords.ts:16:55 - error TS2345: Argument of type 'unknown' is not assignable to parameter of type 'string'. 16 const MatchingType = knownRecordFactories.get(value.get('recType')) as { new(input?: any): any }; // check if we know a Record with this type ~~~~~~~~~~~~~~~~~~~~

joscha · 2024-10-18T02:29:21Z

modules/openapi-generator/src/main/resources/typescript-fetch/sagas.mustache

        return undefined;
 {{/returnType}}
-    } catch (error) {
+    } catch (error: any) {


no implicit any

joscha · 2024-10-18T02:29:25Z

modules/openapi-generator/src/main/resources/typescript-fetch/sagas.mustache

 }

-export function *{{nickname}}SagaImp(_action_: Action<Payload{{#lambda.titlecase}}{{#lambda.camelcase}}{{nickname}}{{/lambda.camelcase}}{{/lambda.titlecase}}>) {
+export function *{{nickname}}SagaImp(_action_: Action<Payload{{#lambda.titlecase}}{{#lambda.camelcase}}{{nickname}}{{/lambda.camelcase}}{{/lambda.titlecase}}>){{^returnType}}: any{{/returnType}} {


no implicit any

joscha · 2024-10-18T02:30:48Z

modules/openapi-generator/src/main/resources/typescript-nestjs/configuration.mustache

@@ -1,4 +1,9 @@
+{{#useAxiosHttpModule}}


@nestjs/axios is only in the package.json when useAxiosHttpModule is truthy. If not, the package is not a dependency, which then fails in the type checks, as it can't be resolved. Other code uses the HttpService from nestjs/common in that case, so we do that here as well now, too.

joscha · 2024-10-18T02:32:18Z

modules/openapi-generator/src/main/resources/typescript-nestjs/package.mustache

    "@nestjs/testing": "~{{nestVersion}}",
    "@types/express": "^4.16.0",
    "@types/jest": "^24.0.15",
-    "@types/node": "^14.8.2",


These types are so old that some definitions clash with the current typescript version.

joscha · 2024-10-18T02:32:51Z

modules/openapi-generator/src/main/resources/typescript/http/http.mustache

        }

-        const parameters = this.headers[headerName].split(";");
+        const parameters = this.headers[headerName]!.split(";");


this is guarded further up, but some of the older TS versions we have can't determine this, yet.

joscha · 2024-10-18T02:33:00Z

modules/openapi-generator/src/main/resources/typescript/http/http.mustache

+        const parameters = this.headers[headerName]!.split(";");
        for (const parameter of parameters) {
            let [key, value] = parameter.split("=", 2);
+            if (!key) {


undefined guard

joscha · 2024-10-18T02:33:32Z

modules/openapi-generator/src/main/resources/typescript/model/ObjectSerializer.mustache

 *
 * This helper transforms a string mime-type into an internal representation.
 * This simplifies the implementation of predicates that in turn define common rules for parsing or stringifying
 * the payload.


undefined guards here too

joscha · 2024-10-18T02:34:40Z

modules/openapi-generator/src/main/resources/typescript/model/model.mustache

    */
 {{/description}}
    '{{name}}'{{^required}}?{{/required}}: {{#isEnum}}{{{datatypeWithEnum}}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}{{#isNullable}} | null{{/isNullable}};
 {{/vars}}


noImplicitOverride fixes

joscha · 2024-10-18T02:35:33Z

modules/openapi-generator/src/main/resources/typescript/services/index.mustache

 class InjectableConfiguration implements AbstractConfiguration {
    public httpApi: HttpLibrary = new DefaultHttpLibrary();
    public middleware: Middleware[] = [];
    public authMethods: AuthMethods = {};


services/index.ts:26:9 - error TS2322: Type 'ServerConfiguration<{}> | undefined' is not assignable to type 'AbstractServerConfiguration'. Type 'undefined' is not assignable to type 'AbstractServerConfiguration'. 26 @inject(AbstractServerConfiguration) @optional() public baseServer: AbstractServerConfiguration = servers[0], ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ services/index.ts:68:54 - error TS2345: Argument of type 'ServerConfiguration<{}> | undefined' is not assignable to parameter of type 'AbstractServerConfiguration'. Type 'undefined' is not assignable to type 'AbstractServerConfiguration'. 68 this.bindServerConfiguration.toConstantValue(servers[idx]); ~~~~~~~~~~~~

joscha · 2024-10-18T02:35:49Z

modules/openapi-generator/src/main/resources/typescript/package.mustache

    {{/useRxJS}}
    {{#useInversify}}
-    "inversify": "^5.0.1",
+    "inversify": "^6.0.1",


Inversify 5.x doesn't work properly with our current TS version. It's this issue here: https://stackoverflow.com/questions/75776252/inversify-typescript-5-ts1239-unable-to-resolve-signature-of-parameter-decor

joscha · 2024-10-18T07:31:02Z

.github/workflows/samples-typescript-typecheck.yaml

+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:


if we were to generate this file, we could used the samples axis to produce one for each project with tsconfig. This would run the checks in parallel and make failures easier to understand. It would be the first time we do a dynamic build step though. Let me know what you prefer. We can also improve this later.

.github/workflows/samples-typescript-typecheck.yaml

joscha · 2024-10-18T07:36:02Z

modules/openapi-generator/src/main/resources/typescript/auth/auth.mustache

 export type HttpBasicConfiguration = { "username": string, "password": string };
 export type HttpBearerConfiguration = { tokenProvider: TokenProvider };
 export type OAuth2Configuration = { accessToken: string };
+export type HttpSignatureConfiguration = unknown; // TODO: Implement


I am not actually sure what this has to look like. Currently it's not supported as we don't have it implemented. This fix is to produce valid syntactic code, the implementation is probably a bit more involved.
You can see the fix it produces below in samples/openapi3/client/petstore/typescript/builds/explode-query/auth/auth.ts.
And how it is currently broken (syntactically invalid TS), here:

openapi-generator/samples/openapi3/client/petstore/typescript/builds/explode-query/auth/auth.ts

Lines 164 to 166 in ce09134

"bearer_test"?: HttpBearerConfiguration,

"http_signature_test"?:

}

joscha · 2024-10-18T07:36:48Z

modules/openapi-generator/src/main/resources/typescript/services/index.mustache

     * return value;
     */
    public bindServerConfigurationToPredefined(idx: number) {
+        if (!servers[idx]) {


servers[idx] can be undefined

joscha · 2024-10-18T07:37:24Z