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

multipleOf acts wrong starting with 7 decimal places #3486

Open
OlliL opened this issue May 11, 2024 · 1 comment
Open

multipleOf acts wrong starting with 7 decimal places #3486

OlliL opened this issue May 11, 2024 · 1 comment
Assignees

Comments

@OlliL
Copy link

OlliL commented May 11, 2024

If you want to enforce a max amount of decimal places you can use multipleOf(). If you want to enforce a max amount of 3 decimal places you can use multipleOf(0.001) which works perfectly fine.

  • 2.234 / 0.001 = 2234 ✅
  • 2.2345 / 0.001 = 2234.5 ‼️

So while this works up to 6 decimals (multipleOf(0.000001)) fine, it stops working with 7 decimals and above. To make it parse with 7 decimal places and above, the number to parse must contain exactly the amount of decimal places as specified in multpleOf.A small showcase:

  const number3 = 5.123;
  const number6 = 5.123456;
  const number7 = 5.1234567;
  const number8 = 5.12345678;
  let schema6 = z.number().multipleOf(0.000001);
  let schema7 = z.number().multipleOf(0.0000001);
  console.log(schema6.parse(number3));  // OK
  console.log(schema6.parse(number6));  // OK
  console.log(schema6.parse(number7));  // Error --> OK
  console.log(schema6.parse(number8));  // Error --> OK
  console.log(schema7.parse(number3));  // Error --> Bug
  console.log(schema7.parse(number6));  // Error --> Bug
  console.log(schema7.parse(number7));  // OK
  console.log(schema7.parse(number8));  // Error --> OK

The bug happens because of how JavaScript handles complex numbers. Zods internal function floatSafeRemainder tries to parse its parameter. While it works fine with 0.000001, it stops working fine with 7 and more decimals because then step becomes 1e-7 instead of 0.0000001 and the parsing logic stops working.

grafik

A possible fix which accounts exponential representation of complex numbers could be the following drop-in replacement logic for stepDecCount:

    let stepDecCount = (step.toString().split(".")[1] || "").length;
    if(stepDecCount === 0 && /\d?e-\d?/.test(step)) {
      stepDecCount = step.toString().match(/\d?e-(\d?)/)[1];
    }

Seems to work fine for me.....

@dellamora
Copy link

I would like to work on this. Can you assign me this issue ^^

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

No branches or pull requests

3 participants