cloudflare / chanfana

OpenAPI 3 and 3.1 schema generator and validator for Hono, itty-router and more!
https://chanfana.pages.dev
MIT License
288 stars 38 forks source link

Improve Enumeration constructor types #28

Closed gimenete closed 1 year ago

gimenete commented 1 year ago

I spent some time trying to understand why something as simple as this wasn't working:

new Enumeration({
  description: "",
  values: ["enabled", "disabled", "custom"],
  required: false,
}),

Then, digging into the code and checking the docs I saw that values needs to be Record<string, any>. But then, why TypeScript wasn't complaining?

It turns out that an array of strings satisfies Record<string, any>. Yeah: https://www.typescriptlang.org/play?#code/MYewdgzgLgBADgQwE4ILYQFwwEoFNRIAmAPNEgJZgDmANDAmAJ4B8MAvDANoDkC3AugChBoSCAA2uAHTiQVABSIU6AJRA

So, It'd be better to use Record<string, unknown> instead, which gives a compiler error if you try to pass an array of strings.

G4brym commented 1 year ago

Hey, maybe this field could accept both Record<string, any> or Array, because there are some project that don't require the string transformation that it currently applies. I suggest that in that class constructor checking if the values is an array, and if true, then just convert it to an object. This way, it would require no changes in other class's

gimenete commented 1 year ago

@G4brym that's a good idea. I've just updated the PR do do that 👍

G4brym commented 1 year ago

Looks good @gimenete Thanks for contributing this, I will make a new release in the next days to include this

gimenete commented 1 year ago

Awesome. Thanks!!