feat: do not wrap string validations #162
Conversation
mallachari
force-pushed
the
feat/string-validations-no-wrap
branch
from
18cceea to
833151a
Compare
| const validationFormatters: Record<string, (value: unknown) => ValidationFormat | null> = { | ||
| enum: createValidationsFormatter('Allowed'), | ||
| examples: createValidationsFormatter('Example'), | ||
| enum: createValidationsFormatter('Allowed', { nowrap: true }), |
There was a problem hiding this comment.
Hmm, and how will one recognize such an enum?
enum: [1, "1"]
I don't think it's a good decision to remove quotes. These are strings, so let them be.
There was a problem hiding this comment.
We can't in such a case. That's why I added it in the first place.
@mnaumanali94 is it really needed to remove those quotes? We're losing a type then.
There was a problem hiding this comment.
Hmm, and how will one recognize such an enum?
enum: [1, "1"]
This is a terrible enum! If someone is mixing stringified numbers and numberified numbers in an enum, their API is beyond our help. 
There was a problem hiding this comment.
I know it is, but it's just to show the example.
The below cases are valid tho:
enum: ["1", "2", "3"]
enum: [1, 2, 3]
If we render them the same way, how will one be able to tell the difference?
There was a problem hiding this comment.
@P0lip are enums the only potential problem? Or can you think about more examples where this change would cause confusion? 🤔
What if we made the enums an exception here? @mnaumanali94 is a strong proponent of that change. 🤷
There was a problem hiding this comment.
It depends on the scenario.
Consider the below:
{
"enum": ["0", 1, "2"]
}or
{
"type": ["string", "number"],
"enum": ["0", 1, "2"]
}One won't be able to tell the difference once we apply the change.
There was a problem hiding this comment.
Hmm. Have you ever seen an enum like this though? 🤦🏼
There was a problem hiding this comment.
Should
type: string,
enum: [asc, desc]
In our docs, these values should appear as "asc", "desc" is what you're suggesting?
There was a problem hiding this comment.
The first example is quite common actually.
There was a problem hiding this comment.
I'm thinking this is not a common use case. The concern is primarily folks thinking "" are part of the value which would be wrong. Lets get this in, and see if there's a reaction from our consumers. If so, we can use colours to denote string vs integer values.
|
🎉 This PR is included in version 4.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
7 participants
Addresses: https://github.com/stoplightio/platform-internal/issues/7377
Quotes were added to distinguish the type of validation.
This PR makes
enum,examplesanddefaultnot show quotes regardless of type.