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

[Bug Report] every property is cloned to required type even if there are optional property. #628

Closed
rojiwon123 opened this issue Sep 26, 2023 · 13 comments
Assignees
Labels
bug Something isn't working

Comments

@rojiwon123
Copy link
Contributor

rojiwon123 commented Sep 26, 2023

Bug Report

DTO's original property is optional. but in generated DTO, all property is required.

I write a test controller that replace UserNormalsController.ts in test/features/clone/src/controllers

this controller test

  • comment tag transformed to special tag

  • When namespaced DTO type comes, @nestia/sdk had taken a mistake that writing only the deepest type even in the top or middle level namespaced types.

  • When clone mode being used in SDK generator, it was not possible to clone recursive DTO type.

  • In propagation mode use with TypedException decorator, target T type had not been cloned.

  • optional property is cloned required property

  • use HttpCode decorator

import core from "@nestia/core";
import * as nest from "@nestjs/common";
import typia from "typia";

@nest.Controller("users")
export class UsersController {
    /**
     * - When namespaced DTO type comes, `@nestia/sdk` had taken a mistake that writing only the deepest type even in the top or middle level namespaced types.
     * - When `clone` mode being used in SDK generator, it was not possible to clone recursive DTO type.
     */
    @core.TypedException<ErrorCode.NotFound>(404)
    @core.TypedRoute.Get(":user_id")
    public async getOauthProfile(
        @core.TypedParam("user_id") user_id: string,
        @core.TypedQuery() query: IAuthentication,
    ): Promise<IAuthentication.IProfile> {
        user_id;
        query;
        return typia.random<IAuthentication.IProfile>();
    }


    /**
     * - When namespaced DTO type comes, `@nestia/sdk` had taken a mistake that writing only the deepest type even in the top or middle level namespaced types.
     * - When propagation mode being used with `@TypedException<T>()` decorator, target `T` type had not been cloned.
     * - When `clone` mode being used in SDK generator, it was not possible to clone recursive DTO type.
     * - check optional query DTO
     * - when use HttpCode decorator, sdk build fail code
     */
    @nest.HttpCode(nest.HttpStatus.ACCEPTED)
    @core.TypedException<ErrorCode.NotFound>(404)
    @core.TypedRoute.Get(":user_id")
    public async getUserProfile(
        @core.TypedParam("user_id") user_id: string,
        @core.TypedQuery() query: IUser.ISearch
        ): Promise<IUser> {
            user_id;
            query;
            return typia.random<IUser>();
        }    
    */

    /**
     * - check optional, nullable property
     */
    @core.TypedRoute.Post(":user_id")
    public async updateUserProfile(
        @core.TypedParam("user_id") user_id: string,
        @core.TypedBody() body: IUser.IUpdate
    ): Promise<IUser> {
        user_id;
        body;
        return typia.random<IUser>();
    }
}

interface IAuthentication {
    user_id: string;
    oauth_type: IAuthentication.OauthType;
}

namespace IAuthentication {
    export type OauthType = "google" | "github" | "kakao";
    export interface IProfile {
        id: string;
        name: string;
        /** @format email */
        email: string | null;
        oauth_type: OauthType;
    }
}

interface IUser {
    id: string;
    name: string;
    email: (string & typia.tags.Format<"email">) | null;
    optional_attr?: string;
    nullable_attr: string | null;
    optional_and_nullable_attr?: number | null;
    user_type: IUser.Type;
}

namespace IUser {
    export type Type = "admin" | "default" | "seller";
    /**
    * this type name expected to 'IUpdate', but cloned dto name is 'PartialPickIUsernameemailnullable_attr'
    */
    export type IUpdate = Partial<
        Pick<IUser, "name" | "email" | "nullable_attr" | "optional_attr">
    >;
    export interface ISearch {
        user_type?: Type;
    }
}

namespace ErrorCode {
    export type NotFound = "404 Not Found";
}
@rojiwon123 rojiwon123 changed the title [Bug Report] optional property generate required property. [Bug Report] every property is cloned to required type even if there are optional property. Sep 26, 2023
@samchon samchon self-assigned this Sep 27, 2023
@samchon samchon added the bug Something isn't working label Sep 27, 2023
@rojiwon123
Copy link
Contributor Author

this controller have another bug

  • exception info(TypedException data) is missing.
    => i think, when exception schema is just string type (like "NOT_FOUND_USER"), exception info is missing.
// success case
@core.TypedException<"404 Not Found">(nest.HttpStatus.NOT_FOUND)
@core.TypedRoute.Get("success")
async successCase(): Promise<string> {
    return "success";
}

// fail case
@core.TypedException<ErrorCode.NotFound>(nest.HttpStatus.NOT_FOUND)
@core.TypedRoute.Get("fail")
async failCase(): Promise<string> {
     return "fail";
}

namespace ErrorCode {
    export type NotFound = "404 Not Found";
}

samchon added a commit that referenced this issue Sep 27, 2023
Fix #628 - enhance test programs of DTO clone mode.
@samchon
Copy link
Owner

samchon commented Sep 27, 2023

Upgrade to v2.1.4, then everything would be fine.

@rojiwon123
Copy link
Contributor Author

rojiwon123 commented Sep 27, 2023

@samchon

bug condition
@nestia/sdk v2.1.4
tsconfig - "exactOptionalPropertyTypes": true // Distinguish between optional property and undefined type.

UtilType Partial is not work.
e.g) In clone-and-propagate/src/controllers/UserNormalsController.ts, IUser.IUpdate is not correct when exactOptionalPropertyTypes is true.


another small bug)
a DTO declared by type keyword generate different name clone type.

@samchon
Copy link
Owner

samchon commented Sep 28, 2023

Show me the difference name clone type case please.

@rojiwon123
Copy link
Contributor Author

rojiwon123 commented Sep 28, 2023

Show me the difference name clone type case please.

i think, it need another issue and features project.

because this bug is related with tsconfig.

so i will make it tonight!

@rojiwon123
Copy link
Contributor Author

@samchon
this repo is test project

@samchon
Copy link
Owner

samchon commented Sep 29, 2023

No problem when exactOptionalProperty mode. What is the problem?

May I always define the something?: undefined | T in this case? How do you think about?

Original type

optional_attr?: string;
undefindable_attr: string | undefined;
both_optional_and_undefindable?: string | undefined;

Cloned type

optional_attr?: string;
undefindable_attr: undefined | string;
both_optional_and_undefindable?: undefined | string;

@rojiwon123
Copy link
Contributor Author

rojiwon123 commented Sep 29, 2023

can you add test project to nestia/test/features/clone-and-exact-optional-property?
(Would you prefer me to submit a pull request (PR) with the project to add?)

i think,
if exactOptionalProperty is false, it is correct that a property is added undefined.
but exactOptionalProperty is true, a property have to just be added ?.

now in exactOptionalProperty mode, nothing is added.

test project can show this bug.

  • original interface
interface IOriginal {
    a:string;
    b:string;
    c:string;
    d:string;
    /** @format email */
    email: string | null;
    created_at: ( string & typia.tags.Format<'date-time'>) | null;
    original_optional?: boolean;
    undefinable_attr: string | undefined;
}

interface IPartialInterface extends Partial<Pick<IOriginal,"a" | "email" | "created_at" | "original_optional" | "undefinable_attr">> {};
type IPartialType = Partial<Pick<IOriginal,"b" | "email" | "created_at" | "original_optional" | "undefinable_attr">>;

namespace IOriginal {
   export interface IPartialInterface extends Partial<Pick<IOriginal,"c" | "email" | "created_at" | "original_optional" | "undefinable_attr">> {};
   export type IPartialType = Partial<Pick<IOriginal,"d" | "email" | "created_at" | "original_optional" | "undefinable_attr">>;
}
  • expected generated DTO
export type IOriginal = {
    a: string;
    b: string;
    c: string;
    d: string;
    email: null | (string & Format<"email">);
    created_at: null | (string & Format<"date-time">);
    original_optional?: boolean;
    undefinable_attr: undefined | string;
}
export namespace IOriginal {
    export type IPartialInterface = {
        email?: null | (string & Format<"email">);
        created_at?: null | (string & Format<"date-time">);
        original_optional?: boolean;
        undefinable_attr?: undefined | string;
        c?: string;
    }
}
export type IPartialInterface = {
    email?: null | (string & Format<"email">);
    created_at?: null | (string & Format<"date-time">);
    original_optional?: boolean;
    undefinable_attr?: undefined | string;
    a?: string;
}
export type PartialPickIOriginaldemailcreated_atoriginal_optionalundefinable_attr = {
    d?: string;
    email?: null | (string & Format<"email">);
    created_at?: null | (string & Format<"date-time">);
    original_optional?: boolean;
    undefinable_attr?: undefined | string;
}
export type PartialPickIOriginalemailcreated_atoriginal_optionalundefinable_attrb = {
    email?: null | (string & Format<"email">);
    created_at?: null | (string & Format<"date-time">);
    original_optional?: boolean;
    undefinable_attr?: undefined | string;
    b?: string;
}
  • actual generated dto in v2.1.4
export type IOriginal = {
    a: string;
    b: string;
    c: string;
    d: string;
    email: null | (string & Format<"email">);
    created_at: null | (string & Format<"date-time">);
    original_optional?: boolean;
    undefinable_attr: undefined | string;
}
export namespace IOriginal {
    export type IPartialInterface = {
        email: null | (string & Format<"email">);
        created_at: null | (string & Format<"date-time">);
        original_optional?: boolean;
        undefinable_attr: undefined | string;
        c: string;
    }
}
export type IPartialInterface = {
    email: null | (string & Format<"email">);
    created_at: null | (string & Format<"date-time">);
    original_optional?: boolean;
    undefinable_attr: undefined | string;
    a: string;
}
export type PartialPickIOriginaldemailcreated_atoriginal_optionalundefinable_attr = {
    d: string;
    email: null | (string & Format<"email">);
    created_at: null | (string & Format<"date-time">);
    original_optional?: boolean;
    undefinable_attr: undefined | string;
}
export type PartialPickIOriginalemailcreated_atoriginal_optionalundefinable_attrb = {
    email: null | (string & Format<"email">);
    created_at: null | (string & Format<"date-time">);
    original_optional?: boolean;
    undefinable_attr: undefined | string;
    b: string;
}

@samchon
Copy link
Owner

samchon commented Sep 29, 2023

Change to branch features/dto and look at the test/features/clone-and-propagate project.

@samchon
Copy link
Owner

samchon commented Sep 29, 2023

Then let's make every optional or undefindable to be variable?: undefined | T.

It would be better if considering the frontend environments.

@rojiwon123
Copy link
Contributor Author

rojiwon123 commented Sep 29, 2023

Then let's make every optional or undefindable to be variable?: undefined | T.

It would be better if considering the frontend environments.

but exactOptionalProperty setting is look at A and B differently.

and this bug is appear when exactOptionalProperty mode and use Partial util type.

@samchon
Copy link
Owner

samchon commented Sep 29, 2023

About the partial type, I will check typia again.

@rojiwon123
Copy link
Contributor Author

rojiwon123 commented Sep 29, 2023

now clone-and-propagate project show this bug. (original interface is IUser.IUpdate in UserNormalsController.ts)

Cloned type

https://github.com/samchon/nestia/blob/features/dto/test/features/clone-and-propagate/src/api/structures/PartialPickIUsernameemailnullable_attroptional_attr.ts

Expected type

export type PartialPickIUsernameemailnullable_attroptional_attr = {
    name?: string;
    email?: null | (string & Format<"email">);
    nullable_attr?: null | string;
    optional_attr?: string;
}

samchon added a commit to samchon/typia that referenced this issue Sep 29, 2023
…tial<T>` and `Required<T>` types.

If user configured `tsconfig.json` to turn on `exactOptionalPropertyTypes` option, and use `Partial<T>` or `Required<T>` type, `typia` could not identify whether each property is optional or not.

This PR fixes such bug, by enhancing analyzer of TypeScript types. Also, turned on the `exactOptionalPropertyTypes` option tor enhancing type safety for this case.
samchon added a commit to samchon/typia that referenced this issue Sep 29, 2023
Fix samchon/nestia#628 - `exactOptionalPropertyTypes`'s with `Partial<T>` and `Required<T>` types.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants