Skip to content
This repository has been archived by the owner on Apr 25, 2019. It is now read-only.

Number validated fields report forbidden values #120

Closed
kpdecker opened this issue Sep 10, 2015 · 8 comments
Closed

Number validated fields report forbidden values #120

kpdecker opened this issue Sep 10, 2015 · 8 comments
Labels
Milestone

Comments

@kpdecker
Copy link
Contributor

Screen shot under chrome using the default styles:

image

Validated with:

Joi.number().required().description('Lorem ipsum')

Two concerns here:

  1. Showing forbidden values of Infinity and -Infinity feels like noise since they were not explicitly defined.
  2. The visual styling here makes it confusing what the data hierarchy is. Ideally this would match the styling of the type field so help indicate that this is within the same set of metadata.
@Marsup
Copy link
Contributor

Marsup commented Sep 10, 2015

  1. I agree but technically Infinity are numbers, Joi explicitly denies them, so I don't see another way, what do you propose ?
  2. Can you check with current master ? I changed the presentation a bit hoping it would help.

Currently the build fails because handlebars@3 seems to skip keys with undefined values inside objects when using #each, since you maintain handlebars would you know why ?

@Marsup Marsup added the request label Sep 10, 2015
@Marsup Marsup self-assigned this Sep 10, 2015
@kpdecker
Copy link
Contributor Author

  1. I'm not sure, I'm still coming up to speed on the echosystem. Perhaps filtering for known cases that are defaults in joi? While strictly true, most people don't care and outputting this hurts readability.
  2. Will do.

Handlebars 3 should output keys with undefined values from each. Under 4 this changed as it causes breakages downstream if you allow that... Although perhaps it's better to fix the root issue by continuing to iterate over undefined values and forcing a fake context when executing helpers.
handlebars-lang/handlebars.js#1093
handlebars-lang/handlebars.js#1065

Could you chime in on the open issue on handlebars pointing to the template that is failing and an example of the data that it is failing with?

@Marsup
Copy link
Contributor

Marsup commented Sep 10, 2015

My bad that was on 4 you're right.

@Marsup
Copy link
Contributor

Marsup commented Sep 10, 2015

Removing defaults might be ok here but on strings for example, if you don't know empty strings are denied it's giving you false assumptions.

@Marsup
Copy link
Contributor

Marsup commented Sep 10, 2015

Well I have nothing to add to those issues, null or undefined gives the same behavior so I'm in handlebars-lang/handlebars.js#1093 case.

@kpdecker
Copy link
Contributor Author

Posting the data that is failing will help me understand and document the use case. Seemingly minor changes like this cause problems in Handlebars a lot and being able to point to reasons for the decisions is beneficial for future reference as well as helping me to decide which of the not completely pleasant options are best :)

@Marsup Marsup closed this as completed in 0bb0dbe Sep 13, 2015
@Marsup
Copy link
Contributor

Marsup commented Sep 13, 2015

Thought about it a bit, Infinity cannot even be stringified, so it makes no sense for an API to tell that.
Did you give a try to the small change I made for design see if it fits your needs ?

@Marsup Marsup added this to the 7.1.0 milestone Sep 13, 2015
@Marsup
Copy link
Contributor

Marsup commented Sep 13, 2015

Published the new version, you can upgrade or try yourself on the live demo.

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants