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

Discussion for a new feature - Partial object #81

Open
woubuc opened this issue Nov 8, 2024 · 7 comments
Open

Discussion for a new feature - Partial object #81

woubuc opened this issue Nov 8, 2024 · 7 comments

Comments

@woubuc
Copy link

woubuc commented Nov 8, 2024

Basically the Partial<T> modifier in Typescript.

I use PATCH for a lot of update routes, where the client can send only the field(s) that changed instead of sending the entire object each time. So instead of doing this:

vine.object({
  name:        vine.string().optional(),
  description: vine.string().optional(),
  imageUrl:    vine.string().url().optional(),
  // etc.optional()
})

I would love to be able to do this:

vine.object({
  name:        vine.string(),
  description: vine.string(),
  imageUrl:    vine.string().url(),
  // etc
}).partial()

Which makes all fields in the object optional (but not the object itself). I think this would make the schema a lot easier to read and communicates the intent better.

A downside I can see is that it may be more difficult to pick up at a glance that this is a partial object - especially if it's a complex object with lots of nested properties so the .partial() modifier is pushed way down. But the inferred type would still make this obvious right away when using it so I don't think that's necessarily a problem.

Would love to hear y'all's thoughts on this.

@thetutlage
Copy link
Contributor

Will it make all the nested properties as optional too?

@woubuc
Copy link
Author

woubuc commented Nov 13, 2024

I'd say no, only the properties of the object itself would be made optional.

There may be room for a 'deep partial' variant but I don't see as many uses for that one personally.

@KeentGG
Copy link

KeentGG commented Dec 26, 2024

+1 my current workaround is (since i dont like re-typing same fields for POST body) to map all fields from createUser with .optional() and removing the password field.

const createUserSchema = vine.object({
  email: vine.string().optional(),
  userName: vine.string().optional(),
  displayName: vine.string(),
  password: vine.string().optional(),
  type: vine.enum(UserType),
})
const { password, ...omitPassUser } = createUserSchema.getProperties()
const optionalOmitPassUser = Object.fromEntries(
  Object.entries(omitPassUser).map(([key, value]) => [key, value.optional()]),
)
const updateUserSchema = vine.object({
  ...optionalOmitPassUser,
  params: vine.object({ id: vine.string() }),
})

this works for me but i admit it's very ugly. While at it, is there a more elegant or proper way to omit a certain field from an object?

@KeentGG
Copy link

KeentGG commented Dec 28, 2024

A downside I can see is that it may be more difficult to pick up at a glance that this is a partial object - especially if it's a complex object with lots of nested properties so the .partial() modifier is pushed way down. But the inferred type would still make this obvious right away when using it so I don't think that's necessarily a problem.

Would love to hear y'all's thoughts on this.

Can do like vine.partial(vine.object({...}, {deep: false}))?

@zakariamehbi
Copy link

+1

@zakariamehbi
Copy link

hey,

i hacked my way around adonisjs & vinejs to put my validation rules in the model.
my function used in the parent base model below is inherited by my other models.

public static getVineSchema(): VineObject<{}, {}, {}, {}> {
    let finalVineSchema = vine.object({})
    const columnsDefinitions = Array.from(this.$columnsDefinitions.values())
    const blacklistedColumns: string[] = ['id', 'created_at', 'updated_at', 'deleted_at']

    for (const columnDefinition of Object.values(columnsDefinitions)) {
        const { columnName, meta } = columnDefinition as ModelColumnOptions

        if (blacklistedColumns.includes(columnName)) {
            continue
        }

        const columnValidations: IColumnDecorator['meta']['validations'] = meta?.validations || []
        let columnType: IColumnDecorator['meta']['type'] = meta?.type
        let columnChildrenType: IColumnDecorator['meta']['children_type'] = meta?.children_type

        if (!meta || !meta?.type) {
            throw new GenericException({
                message: `Object meta is missing or incorrect for column ${column?.name}`,
                status: 400,
                code: 'base/invalid-payload',
            })
        }

        try {
            let shouldEscape = true
            let columnVineSchema: any

            if (columnType === 'array') {
                if (!columnChildrenType) {
                    throw new Error('Subtype is required for array type')
                }

                // @ts-ignore
                columnVineSchema = vine.array(vine[columnChildrenType]({})).compact()
            } else if (columnType === 'object') {
                columnVineSchema = vine.object({}).allowUnknownProperties()
            } else {
                // @ts-ignore
                columnVineSchema = vine[columnType]()
            }

            if (columnValidations && columnValidations.length) {
                for (const v of columnValidations) {
                    if (v?.name === 'noEscape') {
                        shouldEscape = false
                    } else if (v?.name) {
                        // @ts-ignore
                        columnVineSchema[v?.name](v?.options || {})
                    }
                }

                if (shouldEscape && typeof columnVineSchema.escape === 'function') {
                    columnVineSchema.escape()
                }

                if (shouldEscape && typeof columnVineSchema.trim === 'function') {
                    columnVineSchema.trim()
                }
            }

            finalVineSchema = vine.object({
                ...finalVineSchema.getProperties(),
                [columnName]: columnVineSchema.clone(),
            })
        } catch (error) {
            console.error(error)
            console.error('debugging', { columnDefinition, columnName, meta })
        }
    }

    return finalVineSchema
}
export interface IColumnDecorator {
  serializeAs?: null
  meta: {
    searchable: boolean
    type: 'string' | 'boolean' | 'number' | 'date' | 'enum' | 'object' | 'array'
    children_type?: 'string' | 'boolean' | 'number' | 'date' | 'enum' | 'object' | 'array'
    validations: {
      name:
        | 'email'
        | 'url'
        | 'uuid'
        | 'mobile'
        | 'trim'
        | 'normalizeEmail'
        | 'normalizeUrl'
        | 'escape'
        | 'toUpperCase'
        | 'toCamelCase'
        | 'toLowerCase'
        | 'optional'
        | 'nullable'
        | 'noEscape'
      options?: object
    }[]
  }
}
@column({
    meta: {
        searchable: true,
        type: 'string',
        validations: [{ name: 'uuid' }],
    },
})
declare app_uid: string

@column({
    meta: { searchable: true, type: 'string', validations: [{ name: 'noEscape' }] },
    } as IColumnDecorator)
declare timezone: string

usually i write small validations into a separate file and then use them in the controllers (request.validateUsing()).
but when i need to check the full object i use my getVineSchema() function.
for post/put requests i would just add a parameter to the getVineSchema() function partial boolean and apply optional() & nullable() to all the properties.

hope this helps. tell me if there is an easier way to do model-based validations.

@Fabricio-191
Copy link

+1

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

No branches or pull requests

5 participants