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.

Sign up for GitHub

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

Jump to bottom

fix: IsNumber decorator fails on values ABS(x) < 0.000001 when using maxDecimalPlaces #1705

Open
igor-tatarnikov opened this issue Jul 25, 2022 · 3 comments · May be fixed by #2166
Open

fix: IsNumber decorator fails on values ABS(x) < 0.000001 when using maxDecimalPlaces #1705

igor-tatarnikov opened this issue Jul 25, 2022 · 3 comments · May be fixed by #2166
Labels
type: fix Issues describing a broken feature.

Comments

@igor-tatarnikov
Copy link

igor-tatarnikov commented Jul 25, 2022 •
edited

Hi team,

IsNumber decorator throws error, when a small value is analyzed (ABS(x) < 0.000001. It happens because such a small value has exponential format -->

> parseFloat('0.000001')
< 0.000001
> parseFloat('0.0000001')
< 1e-7

The following code fails, because value.toString().split('.')[1] is not available - there's no . in the string!

  if (options.maxDecimalPlaces !== undefined) {
    let decimalPlaces = 0;
    if (value % 1 !== 0) {
      decimalPlaces = value.toString().split('.')[1].length;
    }
    if (decimalPlaces > options.maxDecimalPlaces) {
      return false;
    }
  }

class-validator/src/decorator/typechecker/IsNumber.ts

Line 34 in 2ef8ff0

decimalPlaces = value.toString().split('.')[1].length;

@igor-tatarnikov
Copy link
Author

A fix proposal (I wasn't allowed to create a pull request for some reason, sorry)

  if (options.maxDecimalPlaces !== undefined) {
    let decimalPlaces = 0;
    if (value % 1 !== 0) {
      const stringValue = value.toString();

      if (stringValue.indexOf('.') > -1){
        decimalPlaces = stringValue.split('.')[1]?.length ?? 0;
      }
      else if (stringValue.indexOf('e-') > -1) {
        decimalPlaces = parseInt(stringValue.split('e-')[1] ?? '0');
      }
    }
    if (decimalPlaces > options.maxDecimalPlaces) {
      return false;
    }
  }

@igor-tatarnikov igor-tatarnikov changed the title IsNumber decorator fails on values ABS(x) < 0.0000001 IsNumber decorator fails on values ABS(x) < 0.000001 Jul 25, 2022
@Clashsoft
Copy link
Contributor

All the indexOf and split stuff seems very arcane for number manipulation. Is there no better, numeric way to get the decimal places?

@V3RON
Copy link

V3RON commented Nov 16, 2022

@Clashsoft I'm afraid, there is no reliable, numeric way to determine how many digits are present in the fractional part. Theoretically, we should be able to simply multiply the value by 10 until we get an integer, but as we all know, float operations may end up with an unexpected result and it won't work in some cases.

@NoNameProvided NoNameProvided changed the title IsNumber decorator fails on values ABS(x) < 0.000001 fix: IsNumber decorator fails on values ABS(x) < 0.000001 when using maxDecimalPlaces Nov 17, 2022
@NoNameProvided NoNameProvided added the type: fix Issues describing a broken feature. label Nov 17, 2022
@backnight backnight linked a pull request Jul 16, 2023 that will close this issue
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type: fix Issues describing a broken feature.
Milestone
No milestone
4 participants
@Clashsoft @NoNameProvided @igor-tatarnikov @V3RON