Parse int pipe improvement

[oschweitzer]

TLDR; This PR improves the ParseIntPipe validation.

Motivation

Take the following code base:

// sample.controller.ts
//...
  @Get(':id')
  async getById(
    @Param('id', new ParseIntPipe()) id: number,
  ) {
      console.log(id);
  }
//...

If a user sends the following HTTP request GET http://localhost:3000/users/1abc, the current parseIntPipe will validate the parameter. I think it's not the expected behaviour.

Proposal

This PR will improve the parseIntPipe by using the rxjs function isNumeric to be sure that the input string is only composed of numbers.
rxjs being already a dependency of nestjs, this PR doesn't add a new dependency.

[coveralls]

Coverage increased (+0.006%) to 94.559% when pulling e089a47 on oschweitzer:parseInt-pipe-improvement into 1812567 on nestjs:master.

[kamilmysliwiec]

Hi @oschweitzer, thanks for PR!
However, I don't think it's a good idea to reuse functions from internal API of other libraries.

[oschweitzer]

Sure you're right, I propose to change the condition to:

    if ('string' === typeof value && !isNaN(parseFloat(value)) && isFinite(value as any)) {
     return parseInt(value, 10);
    }

And therefore to remove the rxjs import, is it ok for you ?

[kamilmysliwiec]

@oschweitzer sounds good!

[kamilmysliwiec]

Great job @oschweitzer! Thanks 🙂

[amitport]

this will map the following to the same value:
111
<spaces>111<spaces> (how do you inline space with quoted code in GFM?)
111.43
111.234
please consider the implementation in #370

[lock]

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