TCMiranda / joi-extract-type

Provides native type extraction from Joi schemas for Typescript
MIT License
151 stars 27 forks source link

possibly undefined value whereas `.default` is defined #17

Closed nicgirault closed 4 years ago

nicgirault commented 5 years ago

Hello!

Thanks so much for this project.

I think I catch an unexpected behavior:

My object definition is:

Joi.object({
  page: Joi.number().default(0),
}).required()

Then when I extract type I get this type:

Map<{
    page?: number | undefined;
} & {}>

As far as I know, page can't be undefined after joi validation. Am I right?

TCMiranda commented 5 years ago

Thanks! Yeah you are right! This makes sense.

I'll work to make te extracted type like:


{
    page?: number;
}
```It needs to be an optional property in order to allow an empty object `{ }`

Em sex, 14 de jun de 2019 às 01:59, Nicolas Girault <
notifications@github.com> escreveu:

> Hello!
>
> Thanks so much for this project.
>
> I think I catch an unexpected behavior:
>
> My object definition is:
>
> Joi.object({
>   page: Joi.number().default(0),
> }).required()
>
> Then when I extract type I get this type:
>
> Map<{
>     page?: number | undefined;
> } & {}>
>
> As far as I know, page can't be undefined after joi validation. Am I right?
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/TCMiranda/joi-extract-type/issues/17?email_source=notifications&email_token=ACM4J77VEEOJSZ652P2TTETP2MQT3A5CNFSM4HYEQCUKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GZO6AKA>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ACM4J77DM6IE2IGCL7JCSOLP2MQT3ANCNFSM4HYEQCUA>
> .
>

-- 
Tiago Miranda
In <https://linkedin.com/in/tiagosemoh>
nicgirault commented 5 years ago

I don't understand why the extracted type would be:

{
  page: number
}

Why would we allow an empty object {}?

TCMiranda commented 5 years ago

Because Joi.validate({ }, schema) would be accepted since the property has a .default(0)

Em sex, 14 de jun de 2019 às 12:14, Nicolas Girault < notifications@github.com> escreveu:

I don't understand why the extracted type would be:

{ page: number }

Why would we allow an empty object {}?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/TCMiranda/joi-extract-type/issues/17?email_source=notifications&email_token=ACM4J73YEFBHFCR7Y2EIA5DP2OYXFA5CNFSM4HYEQCUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXC2OY#issuecomment-502148411, or mute the thread https://github.com/notifications/unsubscribe-auth/ACM4J7YKF6D6RLS22LOUFODP2OYXFANCNFSM4HYEQCUA .

-- Tiago Miranda In https://linkedin.com/in/tiagosemoh

nicgirault commented 5 years ago

And don't we want Joi.validate({ }, schema) to be accepted?

TCMiranda commented 5 years ago

We do. That's why I suggested the extracted type to contain an optional property:

{
    page?: number;
}

😅

Em seg, 17 de jun de 2019 às 09:38, Nicolas Girault < notifications@github.com> escreveu:

And don't we want Joi.validate({ }, schema) to be accepted?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/TCMiranda/joi-extract-type/issues/17?email_source=notifications&email_token=ACM4J74PSLLMKN3J5CEEZSTP26ATXA5CNFSM4HYEQCUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODX3BABQ#issuecomment-502665222, or mute the thread https://github.com/notifications/unsubscribe-auth/ACM4J77TPOZUNFMX56KK53TP26ATXANCNFSM4HYEQCUA .

-- Tiago Miranda In https://linkedin.com/in/tiagosemoh

mrozekma commented 4 years ago

I think the confusion here is around whether the extracted type is supposed to model the object going into Joi.validate() or the object that comes out when validation is successful. For example:

const schema = Joi.object({
  page: Joi.number().default(0),
}).required();
const { value } = schema.validate({});

The object being passed into validate can be empty, but the one that comes out can't -- in this case it would be { page: 0 }. I would expect the extracted type to model the validated object, so:

{
    page: number;
}
NicolasDuran commented 4 years ago

@TCMiranda I think @mrozekma is right. I hope this issue can be addresses, as .default(xx) is a very common pattern

TCMiranda commented 4 years ago

Thanks everybody, I made it required. Just published 15.0.2, could you see if it fits?

mantljosh commented 3 years ago

@TCMiranda did this ever land? I'm still seeing the following:

const enabledConfigSchema = Joi.object({ enabled: Joi.boolean().default(true) })
export type EnabledConfig = Joi.extractType<typeof enabledConfigSchema>
// enabled can still be undefined, even though it has a default
pke commented 1 year ago

@TCMiranda this does indeed still not work in 15.0.8. Was the patch ever released to npm? I can't find any tags on this repo nor releases.