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

Standard Approach NestJS Fix #52

Closed
wants to merge 3 commits into from
Closed

Standard Approach NestJS Fix #52

wants to merge 3 commits into from

Conversation

imabp
Copy link

@imabp imabp commented Apr 14, 2021

Description

The following changes in the code enable the automatic NestJS serialization for JSON which was turned off, while using Library Specific @Res

This PR also adds a workflow command npm run lint:fix to fix the lints.

Related Issue
Fixes #51

Type of Change:

  • Code

Code/Quality Assurance Only

  • Bug fix (non-breaking change which fixes an issue)

Additional Info (OPTIONAL)

NestJS Standard Approach is mentioned over here
image

Checklist

  • My code follows the code style of this project.
  • My changes generate no new warnings

@imabp
Copy link
Author

imabp commented Apr 14, 2021

Honestly, Linters are crazy, fixing them up.

@imabp
Copy link
Author

imabp commented Apr 14, 2021

Really I would admire, if linting check is not done at least on server for now.
What are your views @Abhishek-kumar09 :(
This is something making me feel not good

@Abhishek-kumar09
Copy link
Contributor

Did you tried running npm run lint:fix

@imabp
Copy link
Author

imabp commented Apr 14, 2021

Can you add this to your GitHub Actions ?
Let me create an issue for the same

@imabp
Copy link
Author

imabp commented Apr 14, 2021

@Abhishek-kumar09 I added few a command to let Workflow run, the npm run lint:fix
Let me know if you feel good about it.

@Abhishek-kumar09
Copy link
Contributor

I see, we are doing this to serialize the returned object.
It is being said in the documentation that

Nest detects when the handler is using either @res() or @Next(), indicating you have chosen the library-
specific option. If both approaches are used at the same time, the Standard approach is automatically disabled

Infact, we don't need library specific response here, I believe.
Instead we can use standard approach to return simple json object and the nest will handle everything automatically and it is recommended approach also.
image

@imabp
Copy link
Author

imabp commented Apr 16, 2021

Yes and thats why to maintain standard approach
passthrough:true is added

@Abhishek-kumar09
Copy link
Contributor

Abhishek-kumar09 commented Apr 16, 2021

I meant removing @Res library fully to follow the standard approach and infact returning the json responses from nest.

@imabp
Copy link
Author

imabp commented Apr 16, 2021

We do need @Res for future cases. So cant remove it. @Abhishek-kumar09

@Abhishek-kumar09
Copy link
Contributor

We do need @Res for future cases. So cant remove it.

Not really, When we are using @Res we are just using or forcing to use express library response behaviour.

When we are returning the object using standard approach, we can omit the use of @Res.
I don't think if there will be some specific use case for @Res in the future.

Here is the example of removal of @Res and picking standard response controller provided by nestjs 2e30df8#r49621473.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Library Specific @Res disables the Standard Approach for POST Assignment Route
3 participants