hapijs / joi

The most powerful data validation library for JS
Other
20.95k stars 1.51k forks source link

[types] added "dontChangeOptional" boolean to avoid changing optional props in SchemaMap #2907

Open maxgalbu opened 1 year ago

maxgalbu commented 1 year ago

I have a type which has some parameters that come from GET (and requires validation), others are filled later using request headers and protocol and other stuff (which don't require validation).

When trying to use the type when defining the schema, the fact that either all the properties are made optional (PartialSchemaMap), or are mandatory (StrictSchemaMap), makes defining schema types impossible when some parameters doesn't need validation: https://github.com/hapijs/joi/blob/8fc72733bdd6b1743810a361e739bd43f2f1f0c4/lib/index.d.ts#L793-L801

Example:

type AppParams = {
    destination: string
    destination_latitude: number
    destination_longitude: number
    destination_country: string
    destination_id: string
    secure?: boolean
    host?: boolean
}

//This alerts that "secure" and "host" are missing
const schema = Joi.object<AppParams, true>({
    destination: Joi.string(),
    destination_latitude: Joi.number(),
    destination_longitude: Joi.number(),
    destination_country: Joi.string(),
    destination_id: Joi.string(),
})

const validatedParams = schema.validate(req.query);
validatedParams.secure = req.secure;
validatedParams.host = req.headers.host;

This pull request would allow this:

type AppParams = {
    destination: string
    destination_latitude: number
    destination_longitude: number
    destination_country: string
    destination_id: string
    secure?: boolean
    host?: boolean
}

//This doesn't throw error
const schema = Joi.object<AppParams, true, true>({
    destination: Joi.string(),
    destination_latitude: Joi.number(),
    destination_longitude: Joi.number(),
    destination_country: Joi.string(),
    destination_id: Joi.string(),
})

const validatedParams = schema.validate(req.query);
validatedParams.secure = req.secure;
validatedParams.host = req.headers.host;
maxgalbu commented 1 year ago

I've tried to be as backward compatible as possible. Probably a better solution could be to have isStrict changed to accept string|bool (true for strict, and can also accept "strict", "strict-unchaged", "partial", "partial-unchanged")

Marsup commented 1 year ago

Hi, and sorry for the super late reply. I'm not entirely sure I understand your problem. isStrict is here to ensure that you have a schema for all the keys that you declare having.

So it's quite normal that in your example, the missing secure and host would throw an error. It should resemble something like this:

const schema = Joi.object<AppParams, true, true>({
    destination: Joi.string().required(),
    destination_latitude: Joi.number().required(),
    destination_longitude: Joi.number().required(),
    destination_country: Joi.string().required(),
    destination_id: Joi.string().required(),
    secure: Joi.boolean(),
    host: Joi.string()
})

The required fields should be required, and the optional ones should be present but without the required property.

If you don't care for some properties, then you should do something like Omit<AppParams, 'host' | 'secure'> on your type before passing it to joi's generic.

maxgalbu commented 1 year ago

The fact is that the typecheck should be strict but it `should not overwrite the optional fields... secure and host are* optional

It should be like this, without the -?

type StrictSchemaMap<TSchema = any> =  {
    [key in keyof TSchema]: ObjectPropertiesSchema<TSchema[key]>
};
Marsup commented 1 year ago

Optional doesn't mean they can be entirely absent from your schema. If it's part of the type, it should as well be part of the schema, otherwise you should omit them.