Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maxgalbu
Copy link

@maxgalbu maxgalbu commented Feb 15, 2023

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:

joi/lib/index.d.ts

Lines 793 to 801 in 8fc7273

type PartialSchemaMap<TSchema = any> = {
[key in keyof TSchema]?: SchemaLike | SchemaLike[];
}
type StrictSchemaMap<TSchema = any> = {
[key in keyof TSchema]-?: ObjectPropertiesSchema<TSchema[key]>
};
type SchemaMap<TSchema = any, isStrict = false> = isStrict extends true ? StrictSchemaMap<TSchema> : PartialSchemaMap<TSchema>

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
Copy link
Author

maxgalbu commented Feb 15, 2023

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
Copy link
Collaborator

Marsup commented Aug 27, 2023

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
Copy link
Author

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
Copy link
Collaborator

Marsup commented Sep 28, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants