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

Add @Ignore decorator to exclude from sdk #694

Closed
wants to merge 3 commits into from

Conversation

ninthsun91
Copy link
Contributor

I was trying to add @Ignore decorator to exclude certain controllers and methods when using module include in nestia.config.ts.

But I couldn't pass the test. It fails in npm install --force.

Test Error Message

-----------------------------------------
TEST
-----------------------------------------
npm cache clean --force
npm install --force
Error: Command failed: npm install --force
    at checkExecSyncError (node:child_process:890:11)
    at Object.execSync (node:child_process:962:15)
    at /Users/me/Projects/nestia/deploy/publish.js:11:12
    at /Users/me/Projects/nestia/deploy/publish.js:56:23
    at /Users/me/Projects/nestia/deploy/publish.js:100:24
    at /Users/me/Projects/nestia/test/executable/start.js:89:27
    at /Users/me/Projects/nestia/test/executable/start.js:82:15
    at main (/Users/me/Projects/nestia/test/executable/start.js:87:42)
    at Object.<anonymous> (/Users/me/Projects/nestia/test/executable/start.js:121:1) {
  status: 1,
  signal: null,
  output: [ null, null, null ],
  pid: 89843,
  stdout: null,
  stderr: null
}

related issue

@ninthsun91
Copy link
Contributor Author

I found a way to debug the issues. I'll try to resolve script errors – npm install --force and npx nestia swagger.

@ninthsun91
Copy link
Contributor Author

First, npm install error in local machine was due to typescript version conflict. /packages/sdk/package.json and /test/package.json has typescript version ^5.3.0-beta, whereas typia and some other packages has peer dependencies restrict to typescript version under 5.3.0.

So, I changed the versions in /packages/sdk/package.json and /test/package.json to ^5.2.2 as other packages. However, I won't commit this change because this wasn't an issue in CI, and because I can't ensure compatibility.

npm install --force
npm WARN using --force Recommended protections disabled.
npm WARN ERESOLVE overriding peer dependency
npm WARN While resolving: typia@5.2.6
npm WARN Found: typescript@5.3.2
npm WARN node_modules/typescript
npm WARN   dev typescript@"^5.3.0" from the root project
npm WARN   5 more (@nestia/fetcher, eslint-plugin-deprecation, ts-node, ...)
npm WARN 
npm WARN Could not resolve dependency:
npm WARN peer typescript@">=4.8.0 <5.3.0" from typia@5.2.6
npm WARN node_modules/typia
npm WARN   typia@"^5.2.6" from the root project
npm WARN 
npm WARN Conflicting peer dependency: typescript@5.2.2
npm WARN node_modules/typescript
npm WARN   peer typescript@">=4.8.0 <5.3.0" from typia@5.2.6
npm WARN   node_modules/typia
npm WARN     typia@"^5.2.6" from the root project

@ninthsun91
Copy link
Contributor Author

For the npx nestia swagger error, I was getting below error saying @nestia/sdk is not installed, although the path was correctly indicating the tarball file.

@nestia/sdk has not been installed. Run "npx nestia setup" before.

However, it turned out nothing was actually wrong. I don't know why, but it seems something was broken at the first time I opened the project in WebStorm. The second project which I cloned in VSCode is working fine even after cherry-picking the commits.

I'll overwrite with new commits after running the full test. Also, I'm sorry I might have bothered you last night.

@samchon samchon self-requested a review November 25, 2023 02:20
@samchon samchon self-assigned this Nov 25, 2023
@samchon samchon added enhancement New feature or request good first issue Good for newcomers labels Nov 25, 2023
@samchon samchon added this to In progress in V2 Major Update via automation Nov 25, 2023
Copy link
Owner

@samchon samchon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://nestia.io/docs/sdk/swagger/#special-tags

At now, nestia does not export to swagger documents when @internal comment being used. In the sdk side, the sdk will make the matched function for the @internal type, but it would not be exported to the d.ts file, so that user of the sdk cannot identify it.

If what you want is even not being analyzed in the analyzer side, it would better to using the @ignore comment tag for the consistency. You can accomplish it by start editing from the below line.

https://github.com/samchon/nestia/blob/master/packages/sdk/src/analyses/ControllerAnalyzer.ts#L172

Also, don't forget to adding a test program. You can add a new test program by running npm run generate ignore command under the test project, and test only the ignore feature by executing the npm start -- --only ignore command.

@samchon
Copy link
Owner

samchon commented Nov 25, 2023

About the TS 5.3, I am waiting for this update, so that holding it for a while.

The reason why force install is, there had been break change on compiler API side, so that I need to reflect it.

nonara/ts-patch#134

@ninthsun91
Copy link
Contributor Author

If what you want is even not being analyzed in the analyzer side, it would better to using the @ignore comment tag for the consistency. You can accomplish it by start editing from the below line.

yup, I wish it is not analyzed.

This is because, some features which nestia doesn't support yet, generates broken code in the generated sdk source code. Because of these broken codes, I cannot run the deploy script unless I remove them manually. I try as much as possible to use JSON formatted response while using nestia, but there are cases I use rxjs or streams.

I opened this issue quite abruptly without any request or your permission in advance, but I think I can work on this issue as long as you are okay with it. Also, you may close this PR, since I might not get this done right away. I can reopen it when it's ready for your review.

@samchon
Copy link
Owner

samchon commented Nov 25, 2023

Got it, will develop a @ignore comment tag.

@samchon samchon closed this Nov 25, 2023
V2 Major Update automation moved this from In progress to Done Nov 25, 2023
@ninthsun91 ninthsun91 deleted the feat/deco-exclude branch November 25, 2023 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants