Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid SDK generataion for POST API #266

Closed
kakasoo opened this issue Mar 5, 2023 · 5 comments
Closed

Invalid SDK generataion for POST API #266

kakasoo opened this issue Mar 5, 2023 · 5 comments
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers

Comments

@kakasoo
Copy link
Contributor

kakasoo commented Mar 5, 2023

I will write Korean and English at the same time. ( I'm not good at writing English. )

Summary

@Controller('api/v1/user')
export class UsersController {
  constructor() {}

  /**
   * correct case :
   * There is no problem at the moment if I let the same function receive data through body.
   * The problem is the case bellow.
   * 동일한 기능을 Body를 통해서 데이터를 받게 하면 아무런 문제가 없습니다.
   * 문제는 아래 케이스에 있습니다.
   */
  @TypedRoute.Post('follow1')
  async follow1(
    @UserId() userId: number,
    @TypedBody() { followerId }: FollowUserDto,
  ): Promise<true> {
    console.log(`user${userId} started following user${followerId}`);
    return true;
  }

  /**
   * error case :
   * In my opinion, in POST functions where only parameters exist, the method is not deduced to POST.
   * 제 개인적인 생각으로는, 오직 파라미터만 존재하는 POST API는 POST로 추론되지 않는 거 같습니다.
   */
  @TypedRoute.Post(':id/follow2')
  async follow2(
    @UserId() userId: number,
    @TypedParam('id', 'number') followerId: number,
  ): Promise<true> {
    console.log(`user${userId} started following user${followerId}`);
    return true;
  }
}
  • SDK Version:
"@nestia/core": "^1.0.15"
"@nestia/sdk": "^1.0.10"
"nestia": "^4.0.9"

I tested it after updating the version using ncu, but I still faced the same error.
이번에 ncu를 통해서 버전 업데이트를 진행해봤는데도 이 문제가 그대로 남아 있습니다.

  • Expected behavior:
    I think it is impossible to use the POST method without TypedBody. I don't think there must be a body just because it's a POST method.
    제 생각에는 TypedBody없이 POST 메서드 사용이 불가능한 거 같습니다. 저는 POST 메서드라고 꼭 Body가 있어야 한다고 생각하진 않았습니다.

  • Actual behavior:
    You don't have to worry because the red underscore is just an eslint, prettier error. If I write down the explanation of the error for those who do not understand Korean, follow2.METHOD has 'POST' as a type because I made the API POST method, but the expected type is 'GET' | 'DELETE', so the SDK is incorrectly generated.
    빨간색 밑줄은 단지 eslint, prettier 에러입니다. 제가 한국어를 모르는 분들을 위해 설명을 간략히 적자면, 제가 POST 메서드를 만들었기 때문에 follow2.METHOD'POST' 타입으로 추론이 정상적으로 되지만, 기대되는 타입은 'GET' | 'DELETE' 라고 나옵니다. 이는 정상적으로 SDK가 생성되었다고 생각되지 않습니다.

    image

Code occuring the bug

git clone https://github.com/kakasoo/nestia-demo
cd nestia-demo
npm i
npx nestia sdk
@samchon
Copy link
Owner

samchon commented Mar 5, 2023

I'd recommend to add at least empty request body like {} type.

However, listening your opinion, it seems too strict. I'll change it.

@samchon samchon self-assigned this Mar 5, 2023
@samchon samchon added bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers labels Mar 5, 2023
@kakasoo
Copy link
Contributor Author

kakasoo commented Mar 5, 2023

What a simple way! Okay. I'll fill it out as you told me.
However, this problem is more intuitive to infer the type of method as POST without specifying the type of body, and I think it will be a more acceptable library for junior developers like me.
So I appreciate your thoughtfulness!

@kakasoo
Copy link
Contributor Author

kakasoo commented Mar 5, 2023

// wrong
@TypedBody() body: {} // Cannot read properties of undefined (reading '0')
// right
@TypedBody() body: Record<string, never>,

Currently, people facing this problem can avoid the problem by specifying the Record type as shown above.

@samchon
Copy link
Owner

samchon commented Mar 5, 2023

Upgrade to @nestia/fetcher@1.0.1, then you can use it.

@samchon samchon closed this as completed in c50e649 Mar 5, 2023
samchon added a commit that referenced this issue Mar 5, 2023
Close #266 by accepting #247 - enhance Fetcher
@rojiwon123
Copy link
Contributor

현재 1.0.1 기준으로 @Body나 @TypedBody를 사용하지 않고 POST sdk를 만들면
Input type은 안생기는데, stringify는 Input타입을 사용하고 있는 버그가 발생했습니다.

at v1.0.1, If i don't use Body in POST api. sdk generated like that.
Input type is undefined, but stringify use the type.

export namespace signInGithub
{
    export type Output = IAuthUsecase.SignInResponse;

    export const METHOD = "POST" as const;
    export const PATH: string = "/sign-in/github";
    export const ENCRYPTED: Fetcher.IEncrypted = {
        request: false,
        response: false,
    };

    export function path(): string
    {
        return `/sign-in/github`;
    }
    export const stringify = (input: Input) => typia.assertStringify(input); // Inpu is undefined type.
}

제가 생각한 수정 방향입니다!

개발자가 body를 사용하지 않는 POST API를 설계했을 때,

  1. sdk는 알아서 Input Type을 undefined, unknown, {} 등으로 정의한다.
    • Input Type이 정의되므로 현재 발생된 문제 해결)
  2. Input Type을 생성하지 않은 것 처럼, stringify도 생성하지 않는다.

When a developer designs a POST API without using a body,

  1. the SDK automatically defines the Input Type as undefined, unknown, {}.
    • As the Input Type is defined, any issues that may arise from this can be resolved.
  2. Similarly, since the Input Type is not created, the SDK does not generate a stringify method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants