joi icon indicating copy to clipboard operation
joi copied to clipboard

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

Open maxgalbu opened this issue 2 years ago • 4 comments

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 avatar Feb 15 '23 13:02 maxgalbu

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")

maxgalbu avatar Feb 15 '23 13:02 maxgalbu

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.

Marsup avatar Aug 27 '23 16:08 Marsup

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]>
};

maxgalbu avatar Sep 28 '23 10:09 maxgalbu

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.

Marsup avatar Sep 28 '23 16:09 Marsup