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

@QueryParam enum support #208

Closed
gempain opened this issue Jul 10, 2017 · 9 comments
Closed

@QueryParam enum support #208

gempain opened this issue Jul 10, 2017 · 9 comments
Labels
type: feature Issues related to new features.

Comments

@gempain
Copy link

gempain commented Jul 10, 2017

Hello,

@QueryParam('type') type: MyEnum will always set type to NaN. Otherwise, doing @QueryParam('type', {type: OperationType}) type: OperationType throws an error, since enums do not have constructors.

My current workaround is:

@UseBefore(RequestValidationUtils.isEnumMiddleware('type', MyEnum))
get(...)

and

import {Validator} from "class-validator";

export class RequestValidationUtils {

  public static isEnumMiddleware(queryParam, entity): Function {
    return (req, res, next): void => {
      const validator: Validator = new Validator();
      const queryParamValue = req.query[queryParam];
      if (!queryParamValue || validator.isEnum(req.query[queryParam], entity)) {
        next();
      } else {
        res.status(400).send('Bad request');
      }
    };
  }
}

Thanks for the amazing work !

@pleerock pleerock added community type: feature Issues related to new features. labels Jul 15, 2017
@pleerock
Copy link
Contributor

PRs are accepted for this feature

@MichalLytek
Copy link
Contributor

Can you show how the MyEnum type looks in your code? Is this normal TS enum? As far as I know there is a problem with reflection of this kind of types so class-transformer don't now how to map query param value to JS object or string or number.

@MichalLytek
Copy link
Contributor

@QueryParam('name') decorator is really hard in implementing features with validation and others - we won't develop new features for it, so in order to have validation on query params please take a look on #270 - the PR do the casting to type so you can declare a class with validation decorators 😉

@gempain
Copy link
Author

gempain commented Sep 7, 2017

Sorry for the delay ! My enum is a string enum:

enum MyEnum {
  A = <any>'a',
  B = <any>'b',
}

which produces a JS enum like so:

{
  'a': 'A',
  'A': 'a',
  'b': 'B',
  'B': 'b'
}

I made a PR (which was accepted) a few weeks ago that added support for string enums to the @Enum decorator. TypeScript 2.4 supports string enums, but I don't know if the underlying representation has changed.

To be reopened ?

@MichalLytek
Copy link
Contributor

MichalLytek commented Sep 7, 2017

To be reopened ?

Nope, TS 2.4 string enums works in runtime just like the old enums, nothing changed, just no need for <any> casting.

@gempain
Copy link
Author

gempain commented Sep 7, 2017

Right, but why close this issue since it is tagged as "new-feature" ? Will you still accept PRs ?

@MichalLytek
Copy link
Contributor

String enums have reflected types as String, so now it should be normalized to a string, not as a NaN (normal enums are numbers in runtime).

If you need validator on query param, you should use @QueryParams() decorator, declare a class for it and decorate with @IsEnum and other class-validator decorators.

@gempain
Copy link
Author

gempain commented Sep 8, 2017

Sweet, I didn't know there was an @QueryParams() feature (I see it now in the docs :) ). Thanks for the solution !

@github-actions
Copy link

This issue 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 Dec 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: feature Issues related to new features.
Development

No branches or pull requests

3 participants