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

Javascript number representation #1362

Closed
marshallford opened this issue Nov 17, 2017 · 6 comments
Closed

Javascript number representation #1362

marshallford opened this issue Nov 17, 2017 · 6 comments
Assignees
Labels
support Questions, discussions, and general support

Comments

@marshallford
Copy link

Context

  • node version: 8.6
  • joi version: 13.0.2
  • environment (node, browser): Node
  • used with (hapi, standalone, ...): standalone
  • any other relevant information:

What are you trying to achieve or the steps to reproduce ?

The primary reason that I am opening this issue is to start a discussion about number representation. See this issue from the node postgres lib for a good summary. Basically, if an integer schema is given to Joi will/should it be valid if it is a string type because it has to be a string? How does the convert option effect the outcome? Will Joi convert the string to a Javascript number type and cause lost precision?

Thanks!

const schema = {
  bigserial: Joi.number.integer(),
}
@WesTyler
Copy link
Contributor

WesTyler commented Nov 17, 2017

Will Joi convert the string to a Javascript number type

Yes. Joi.number().integer().validate('2'); // { error: null, value: 2

and cause lost precision

No, it's an integer. As long as your integer is smaller than Number.MAX_SAFE_INTEGER // 9007199254740991

You can prevent the convert option from allowing strings to be considered valid and converting to numbers though:

Joi.number().integer().validate('2', { convert: false }); // ValidationError: "value" must be a number

or

Joi.number().integer().options({convert: false}).validate('2'); // ValidationError: "value" must be a number

@WesTyler WesTyler added the support Questions, discussions, and general support label Nov 17, 2017
@marshallford
Copy link
Author

Thanks for the clarification however my main concern still stands. Lets say I have an integer that is over 9007199254740991 represented as a string like the issue I linked recommends. Shouldn't Joi be able to handle this? Quite a few database datatypes allow numeric values over the Javascript max safe value.

@WesTyler
Copy link
Contributor

Shouldn't Joi be able to handle this?

I honestly think no, since JS itself does not.

@Marsup
Copy link
Collaborator

Marsup commented Nov 19, 2017

I agree that it's unreasonable to accept numbers that we know could be wrong. I have no intention of embedding any high precision math libraries to deal with numbers as I consider it to be a pretty marginal use case, but you're free to make your own number extension doing that.

@Marsup Marsup self-assigned this Nov 19, 2017
@marshallford
Copy link
Author

I hear you in regards to Joi shouldn't handle something that Javascript itself doesn't handle, however I don't agree it is a marginal use case. From my view, Joi validates data, and for a lot of folks their data comes from outside the NodeJS/Javascript world. Data can come from other programming languages, from databases, legacy applications, etc. As a particular example, the primary keys of databases tend to rely on large numbers for uniqueness. In fact, Joi's intro mentions Facebook as a hypothetical use-case for Joi, surely they have the databases with high precision numbers?

At the end of day I understand wanting to keep the project scope focused, I'm just a little bummed. Thanks for all the work you do.

@Marsup
Copy link
Collaborator

Marsup commented Aug 13, 2018

I'm going to close this one in favor of #1531 which gives us a practical path forward, without having to use a new library for that. From a quick scan of npm, I didn't see any joi extension dealing with those numbers, I'm not sure if there's no interest for that or if people are just dealing with it in another way.

@Marsup Marsup closed this as completed Aug 13, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
support Questions, discussions, and general support
Projects
None yet
Development

No branches or pull requests

4 participants