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

nestia/SDK issues: Invalid Types, and Incomplete API Generation #677

Open
Val0429 opened this issue Nov 6, 2023 · 11 comments
Open

nestia/SDK issues: Invalid Types, and Incomplete API Generation #677

Val0429 opened this issue Nov 6, 2023 · 11 comments
Assignees
Labels
duplicate This issue or pull request already exists enhancement New feature or request question Further information is requested

Comments

@Val0429
Copy link

Val0429 commented Nov 6, 2023

Summary

  • SDK Version: [email protected], @nestia/[email protected]
  • Expected behavior:
    1. The types inside SDK api should all be valid.
    2. The CRUD input / output types should all be generated.
    3. The distribution package should be successfully generated.
    4. The "shared" type should be copy over into packages/api.
  • Actual behavior:
    1. There are type named "___type" which are not valid from the export location.
    2. In my example repository, only "Create" api is correctly generated. "Read" / "Update" / "Delete" is missing.
    3. The distribution package is missing the functional folder.
    4. In the packages/api directory, designed for npm distribution, the "shared" type remains in relative directories. It should be copied over to the distribution folder for proper npm distribution.

Detail

I have created a GitHub repository to replicate these issues. You can find it here
To make it work, you can simply run npm i and then npx nestia sdk.

Issue 1: Invalid Types Named "___type"

There are multiple "__type" imports from locations where they are not valid.

image

Issue 2: Incomplete API Generation

This issue is closely related to the first one. For example, the "Read" API is not functioning properly due to Primitive<__type & __type>. Additionally, TypeScript warns about "ES2015 module syntax" being preferred over namespaces.

image
image

Issue 3: The distribution package functional folder is missing.

The functional folder exists inside src/api but is missing from the packages/api directory, which is set up for distribution.

image

Issue 4: "Shared" Type Location for NPM Distribution

There are relative path imports in the "distribution" directory, which may cause complications when publishing to npm. To resolve this, it's advisable to copy them into the distribution folder.

image

@samchon
Copy link
Owner

samchon commented Nov 6, 2023

Upgrade to the latest version, then such error report would be printed.

As Nestia SDK is targetting the client developers, it enforces API controller methods to define explicit types.

-----------------------------------------------------------
 Nestia SDK Generator
-----------------------------------------------------------
Analyzing reflections
  - controllers: #2
  - paths: #4
  - routes: #4
Analyzing source codes

-----------------------------------------------------------
 Nestia Error Report
-----------------------------------------------------------
libs\user\src\user.controller.ts - error on UserController.update()
  - implicit (unnamed) parameter type from "updateUserDto".

libs\user\src\user.controller.ts - error on UserController.delete()
  - implicit (unnamed) parameter type from "deleteUserDto".

@samchon samchon self-assigned this Nov 6, 2023
@samchon samchon added question Further information is requested duplicate This issue or pull request already exists invalid This doesn't seem right labels Nov 6, 2023
@Val0429
Copy link
Author

Val0429 commented Nov 6, 2023

Hi @samchon,

I've encountered same issues with the latest version of nestia, after fixing all the type-related error.
You can find my updated code here.

Here are the issues:

  1. The implicit (unnamed) parameter type from update and delete type should be incorrectly reported.
    They are already explicitly exported.
    I attempted to test this to address the type-related problem.
    Strangely, the error reported by @nestia/sdk disappears after I change the type definition from:
export type UpdateDto<T extends GeneralDelegateMeta> = Parameters<T["update"]>[0]

To:

export type UpdateDto<T extends GeneralDelegateMeta> = OmitDataFields<
    Parameters<T["update"]>[0],
    keyof IBaseEntity
>;
  1. After fixing the error message in the previous step, the SDK Library appears to be successfully generated.
    However, Issues 1, 2, 3, and 4 are still present.

@samchon
Copy link
Owner

samchon commented Nov 6, 2023

Your controller methods still do not have the explicit return type, and it is even same with your service providers.

@Val0429
Copy link
Author

Val0429 commented Nov 6, 2023

@samchon

Got it, thanks. I was trying to make the issue reproduction as simple as possible.

Since the Nestia SDK Generator does show message for "explicit parameter type", but not "explicit return type", developers of nestia may not know it's a requirement. (and it shows success without explicit return type). Shall there be error message for return type too?

I fixed the return type to simply be "string", in both controller and service. The issues are still valid.

@samchon
Copy link
Owner

samchon commented Nov 6, 2023

You're importing prisma auto generated TMP type that can't be exported to the SDK library. It's actually not explicit.

The name __type means that TypeScript compiler is saying that it does not have any explicit name.

In that case, you've to choose one option of below:

  1. Change the Prisma TMP type to explicit one
  2. Use clone mode of nestia.config.ts like below:
/*
 * File: nestia.config.ts
 * File Created: 2023-09-21 04:29:25
 * Author: Val Liu <[email protected]>
 *
 * -----
 * Last Modified: 2023-11-06 07:33:31
 * Modified By: Val Liu
 * -----
 */

import { INestiaConfig } from "@nestia/sdk";

export const NESTIA_CONFIG: INestiaConfig = {
    /**
     * Building `swagger.json` is also possible.
     *
     * If not specified, you can't build the `swagger.json`.
     */
    swagger: {
        /**
         * Output path of the `swagger.json`.
         *
         * If you've configured only directory, the file name would be the `swagger.json`.
         * Otherwise you've configured the full path with file name and extension, the
         * `swagger.json` file would be renamed to it.
         */
        output: "packages/swagger.json",

        servers: [
            {
                url: "http://localhost:3000",
                description: "Local Server",
            },
        ],
    },

    /**
     * List of files or directories containing the NestJS controller classes.
     */
    input: ["libs/**/*.controller.ts", "src/**/*.controller.ts"],

    /**
     * Output directory that SDK would be placed in.
     *
     * If not configured, you can't build the SDK library.
     */
    output: "src/api",

    /**
     * Target directory that SDK distribution files would be placed in.
     *
     * If you configure this property and runs `npx nestia sdk` command,
     * distribution environments for the SDK library would be generated.
     *
     * After the SDK library generation, move to the `distribute` directory,
     * and runs `npm publish` command, then you can share SDK library with
     * other client (frontend) developers.
     */
    distribute: "packages/api",

    /**
     * Allow simulation mode.
     *
     * If you configure this property to be `true`, the SDK library would be contain
     * simulation mode. In the simulation mode, the SDK library would not communicate
     * with the real backend server, but just returns random mock-up data
     * with requestion data validation.
     *
     * For reference, random mock-up data would be generated by `typia.random<T>()`
     * function.
     *
     * @default false
     */
    // simulate: true,

    /**
     * Whether to clone DTO structures or not.
     *
     * If being configured, all of DTOs used in the backend server would be cloned
     * into the `structures` directory, and the SDK library would be refer to the
     * cloned DTOs instead of the original.
     *
     * @default false
     */
    clone: true,
};
export default NESTIA_CONFIG;

@samchon
Copy link
Owner

samchon commented Nov 6, 2023

Also, I'll enhance the error message reporting following your advise.

@samchon samchon added enhancement New feature or request and removed invalid This doesn't seem right labels Nov 6, 2023
@Val0429
Copy link
Author

Val0429 commented Nov 6, 2023

Hi @samchon,

It's great to see that we have the clone feature, providing a lot of flexibility to developers!

However, there are still some issues with clone, and the updated repository can be found here (same place)

1. The distribution folder didn't generate any useful API

As the image shows, there is no functional folder with the generated APIs in the distribution folder. However, it does work in the /src/api directory.

image

2. Some generated type names are not valid

As shown in image1 and image2, the Input Type of src/api/functional/user/index.ts ends with ...1more..., which seems to be a bug.

image
image

3. Some generated namespace names are not valid

In /src/api/structures/idnumber.....ts, there are namespaces named 1more and 2more inside of it, which is not valid naming for namespaces.

image

@samchon
Copy link
Owner

samchon commented Nov 6, 2023

Then no way. Define the explicit type instead of using the Prisma generated TMP type.

@Val0429
Copy link
Author

Val0429 commented Nov 7, 2023

I used Prisma TMP type, for the purpose of "Single Source of Truth" (from the Prisma model).
Would you have better idea for it?

@samchon
Copy link
Owner

samchon commented Nov 7, 2023

As primsa has too long and deep structured name, no way right now.

How about suggest a spec that renaming DTO clone rule about this case?

@CarlosUtrilla
Copy link

Any updates? I would really like to be able to use the types of Prisma instead of the clone option

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants