lukeautry / tsoa

Build OpenAPI-compliant REST APIs using TypeScript and Node
MIT License
3.48k stars 498 forks source link

After update 6.1.5 Types of property 'successStatus' are incompatible #1594

Closed cesco69 closed 4 months ago

cesco69 commented 6 months ago

Sorting

Expected Behavior

After update from

{
  "@tsoa/runtime": "^6.0.0",
 "tsoa": "^6.0.4"
}

to

{
  "@tsoa/runtime": "^6.1.5",
 "tsoa": "^6.1.5"
}

running

tsoa spec-and-routes

I have this error:

\ts-node@10.9.2_@types+node@20.11.28_typescript@5.4.2\node_modules\ts-node\src\index.ts:859
    return new TSError(diagnosticText, diagnosticCodes, diagnostics);
           ^
TSError: ⨯ Unable to compile TypeScript:
dist/routes.ts(880,42): error TS2379: Argument of type '{ methodName: string; controller: ProjectsController; response: ExResponse<any, Record<string, any>>; next: any; validatedArgs: any[]; successStatus: undefined; }' is not assignable to parameter of type 'ExpressApiHandlerParameters' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
  Types of property 'successStatus' are incompatible.
    Type 'undefined' is not assignable to type 'number'.

dist/routes.ts(880,42)

              templateService.apiHandler({
                methodName: 'getProjects',
                controller,
                response,
                next,
                validatedArgs,
                successStatus: undefined,
              });

Context (Environment)

Version of the library: 6.1.5 Version of NodeJS: 20

Breaking change?

Yes, same code works with 6.0.0

jackey8616 commented 6 months ago

@cesco69 Can you provide some code of your controller or some context? thanks.

successStatus should allow pass w/ undefined in design.

WoH commented 6 months ago

@jackey8616 Since ts is also breaking stuff in between minors in the name of 'well that was broken before anyway', unless we used to handle this before properly, this could be ok as a fix.

jackey8616 commented 6 months ago

@jackey8616 Since ts is also breaking stuff in between minors in the name of 'well that was broken before anyway', unless we used to handle this before properly, this could be ok as a fix.

Not sure what cause this, some of test cases does covered the situation that give a undefined value at successStatus. I'm afraid there is some bad things covered by dynamic generate template w/ hbs before.

cesco69 commented 6 months ago

@jackey8616

Can you provide some code of your controller or some context? thanks.

This can help?

export interface ProjectsResponse {
    id: number;
    name: string;
    state: string;
    active_users_count?: number;
    update_date: string;
    external_data: string;
    has_shared: boolean;
    codice: string;
    subject: string;
    logo: string;
    start_date: string;
    end_date: string;
    tagbim: string;
    latitudine: string;
    longitudine: string;
    preferito: number;
    is_system: boolean;
    is_default: boolean;
    application: string;
    creation_date: string;
    state_values_enabled: boolean;
    usage: number;
    owner_name?: string;
    owner_surname?: string;
    owner_avatar?: string;
    owner_id?: number;
    owner_email?: string;
    owner_state?: string;
    subscription_id?: number;
    subscription_type?: string;
    owner_subscription_name?: string;
    owner_subscription_surname?: string;
    owner_subscription_avatar?: string;
    owner_subscription_id?: number;
    owner_subscription_email?: string;
    owner_subscription_state?: string;
    info_state: Array<{ state: number; message: string }>;
    space_gb: number;
}

@Route()
@Security({ serviceKey: [], accessToken: [] })
@Middlewares(myMiddleware)
export class ProjectsController extends Controller {

    @Get('/projects')
    public async getProjects(
        @Request() req: express.Request,
        @Query('start') start: number = -1,
        @Query('size') size: number = -1,
        @Query('filter') filter?: string,
        @Query('infoowner') infoowner: 0 | 1 = 1,
        @Query('infosubscription') infosubscription: 0 | 1 = 1,
        @Query('userscount') userscount: 0 | 1 = 1,
        @Query('public') publicc: 0 | 1 = 0,
        @Query('platform') platform: 0 | 1 = 0,
        @Query('shared') shared: 0 | 1 = 1,
        @Query('order') order: 0 | 1 = 1
    ): Promise<{ projects: Array<ProjectsResponse> }> {
        return { projects: [] }
    }
}
jackey8616 commented 6 months ago

@cesco69 Also need the routes.ts please. Just the rows relative with getProjects method is ok.

cesco69 commented 6 months ago

@jackey8616 all the file or just this:

// ###########################################################################################################
    //  NOTE: If you do not see routes for all of your controllers in this file, then you might not have informed tsoa of where to look
    //      Please look into the "controllerPathGlobs" config option described in the readme: https://github.com/lukeautry/tsoa
    // ###########################################################################################################
        app.get('/projects',
            authenticateMiddleware([{"serviceKey":[],"accessToken":[]}]),
            ...(fetchMiddlewares<RequestHandler>(ProjectsController)),
            ...(fetchMiddlewares<RequestHandler>(ProjectsController.prototype.getProjects)),

            function ProjectsController_getProjects(request: ExRequest, response: ExResponse, next: any) {
            const args: Record<string, TsoaRoute.ParameterSchema> = {
                    req: {"in":"request","name":"req","required":true,"dataType":"object"},
                    start: {"default":-1,"in":"query","name":"start","dataType":"double"},
                    size: {"default":-1,"in":"query","name":"size","dataType":"double"},
                    filter: {"in":"query","name":"filter","dataType":"string"},
                    infoowner: {"default":1,"in":"query","name":"infoowner","dataType":"union","subSchemas":[{"dataType":"enum","enums":[0]},{"dataType":"enum","enums":[1]}]},
                    infosubscription: {"default":1,"in":"query","name":"infosubscription","dataType":"union","subSchemas":[{"dataType":"enum","enums":[0]},{"dataType":"enum","enums":[1]}]},
                    userscount: {"default":1,"in":"query","name":"userscount","dataType":"union","subSchemas":[{"dataType":"enum","enums":[0]},{"dataType":"enum","enums":[1]}]},
                    publicc: {"default":0,"in":"query","name":"public","dataType":"union","subSchemas":[{"dataType":"enum","enums":[0]},{"dataType":"enum","enums":[1]}]},
                    platform: {"default":0,"in":"query","name":"platform","dataType":"union","subSchemas":[{"dataType":"enum","enums":[0]},{"dataType":"enum","enums":[1]}]},
                    shared: {"default":1,"in":"query","name":"shared","dataType":"union","subSchemas":[{"dataType":"enum","enums":[0]},{"dataType":"enum","enums":[1]}]},
                    order: {"default":1,"in":"query","name":"order","dataType":"union","subSchemas":[{"dataType":"enum","enums":[0]},{"dataType":"enum","enums":[1]}]},
            };

            // WARNING: This file was auto-generated with tsoa. Please do not modify it. Re-run tsoa to re-generate this file: https://github.com/lukeautry/tsoa

            let validatedArgs: any[] = [];
            try {
                validatedArgs = templateService.getValidatedArgs({ args, request, response });

                const controller = new ProjectsController();

              templateService.apiHandler({
                methodName: 'getProjects',
                controller,
                response,
                next,
                validatedArgs,
                successStatus: undefined,
              });
            } catch (err) {
                return next(err);
            }
        });
cesco69 commented 6 months ago

side note... I have an aggressive (all strict check enabled) tsconfig setup:

{
    "compilerOptions": {
        "lib": [
            "es2023"
        ],
        "target": "es2022",
        "module": "node16",
        "moduleResolution": "node16",
        "allowSyntheticDefaultImports": true,
        "jsx": "preserve",
        "importHelpers": true,
        "alwaysStrict": true,
        "strict": true,
        "useUnknownInCatchVariables": true,
        "strictFunctionTypes": true,
        "allowUnusedLabels": false,
        "allowUnreachableCode": false,
        "exactOptionalPropertyTypes": true,
        "noImplicitOverride": true,
        "allowJs": true,
        "sourceMap": true,
        "skipLibCheck": true,
        "forceConsistentCasingInFileNames": true,
        "noUncheckedIndexedAccess": true,
        "isolatedModules": true,
        "noFallthroughCasesInSwitch": true,
        "noImplicitReturns": true,
        "noUnusedLocals": true,
        "noUnusedParameters": true,
        "strictBindCallApply": true,
        "noImplicitAny": true,
        "noImplicitThis": true,
        "strictNullChecks": true,
        "strictPropertyInitialization": true,
        "noPropertyAccessFromIndexSignature": false,
        "declaration": true,
        "outDir": "dist",
        "emitDecoratorMetadata": true,
        "experimentalDecorators": true,
        "removeComments": false,
        "esModuleInterop": true,
        "resolveJsonModule": true
    },
    "include": [
        "src/**/*"
    ],
    "exclude": [
        "node_modules"
    ],
    "typeRoots": [
        "node_modules/@types"
    ],
    "types": [
        "node",
        "express"
    ]
}
cesco69 commented 6 months ago

@jackey8616 with my tsconfig the correct ExpressApiHandlerParameters is (explicit number | undefined)

type ExpressApiHandlerParameters = {
  methodName: string;
  controller: Controller | Object;
  response: ExResponse;
  next: ExNext;
  validatedArgs: any[];
  successStatus?: number | undefined;
};

type ExpressApiHandlerParameters = {

jackey8616 commented 6 months ago

@cesco69 I think the problem is exactOptionalPropertyTypes If you turn it off, the error is gone.

FYI

cesco69 commented 6 months ago

@jackey8616 mhh yep.. in both case, this PR https://github.com/lukeautry/tsoa/pull/1595 fix the problem

jackey8616 commented 6 months ago

@cesco69 To fix this, we got lots of types to fix.

@WoH If we want to allow this exactOptionalPropertyTypes, We are going to fix every optional property, I suggest we can fix it. What do you think?

cesco69 commented 6 months ago

@cesco69 To fix this, we got lots of types to fix.

@WoH If we want to allow this exactOptionalPropertyTypes, We are going to fix every optional property, I suggest we can fix it. What do you think?

with the tsconfig skipLibCheck (default is true) it's importat only for the types used by routes.ts, not all the tsoa library.

jackey8616 commented 6 months ago

with the tsconfig skipLibCheck (default is true) it's importat only for the types used by routes.ts, not all the tsoa library.

IMO, If we are going to fix it, I'm more prefer to fix everything, in case next time someone with 'skipLibCheck: false & exactOptionalPropertyTypes: true` fire another issue like this.

cesco69 commented 6 months ago

@jackey8616 I have tried to override the interface with types.d.ts and works!

declare module '@tsoa/runtime' {
    type ExpressApiHandlerParameters = {
        methodName: string;
        // eslint-disable-next-line @typescript-eslint/ban-types
        controller: Controller | Object;
        response: ExResponse;
        next: ExNext;
        // eslint-disable-next-line @typescript-eslint/no-explicit-any
        validatedArgs: any[];
        successStatus?: number | undefined;
    };
    interface ExpressTemplateService extends TemplateService<ExpressApiHandlerParameters, ExpressValidationArgsParameters, ExpressReturnHandlerParameters> { 
        apiHandler(params: ExpressApiHandlerParameters);
    }
}

The only fix for support exactOptionalPropertyTypes is make successStatus?: number | undefined;

github-actions[bot] commented 5 months ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

cesco69 commented 5 months ago

Any news @jackey8616 ?

github-actions[bot] commented 4 months ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days