Skip to content

Add new decorator for casting to Boolean #363

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

Closed
wants to merge 7 commits into from

Conversation

Goodluckhf
Copy link

No description provided.

@NoNameProvided
Copy link
Member

My opinion about this PR is the same as I wrote in #334. This is out of scope for class-transformer. In JSON we have a well-defined way of using boolean values: { propA: true, propB: false }.

If this feature is crucial for someone, this can be easily implemented with a @Transform decorator.

@NoNameProvided NoNameProvided added the flag: needs discussion Issues which needs discussion before implementation. label Jul 29, 2020
@Goodluckhf
Copy link
Author

My opinion about this PR is the same as I wrote in #334. This is out of scope for class-transformer. In JSON we have a well-defined way of using boolean values: { propA: true, propB: false }.

If this feature is crucial for someone, this can be easily implemented with a @Transform decorator.

Yes, that's why i implemented it as a separated decorator and didn't change default usage.
I need it in transforming config from env variables where everything is string (https://github.com/ukitgroup/nestjs-config). So I think it will be nice idea to add some wrappers for comfortable transformation out of the box like: @ToBoolean() , @ToInteger(), @ToNumber() and so on...

@NoNameProvided
Copy link
Member

So I think it will be a nice idea to add some wrappers for comfortable transformation out of the box like @ToBoolean() , @ToInteger(), @ToNumber() and so on...

You can achieve this currently by enabling implicit conversion.

I understand boolean is a special kind as in some scenarios boolean values received as strings in the true | false from (env variables, query params). We need to think this through, as last resort, we can add an extra flag which allows parsing of Boolean based on content but that is the last resort solution.

However, adding extra decorators I don't think we will do it encourages an (in my opinion) invalid workflow. class-validator should be able to infer what basic type you wanted to get there and if you need to use decorators them something is not handled correctly. (Again booleans are a somewhat special case, so I am not implying anything negative regarding your approach.)

@NoNameProvided
Copy link
Member

Closing in favor of #550.

@NoNameProvided NoNameProvided removed the flag: needs discussion Issues which needs discussion before implementation. label Jan 14, 2021
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants