Update built-in fields to new validate hook syntax (#9166)

Co-authored-by: Daniel Cousens <413395+dcousens@users.noreply.github.com>

.changeset/add-isnullable-multiselect.md

@@ -0,0 +1,5 @@
+---
+'@keystone-6/core': minor
+---
+
+Add `db.isNullable` support for multiselect field type, defaults to false

.changeset/fix-bigint-validation.md

@@ -0,0 +1,5 @@
+---
+'@keystone-6/core': patch
+---
+
+Fix bigInt field type to throw if `defaultValue: { kind: 'autoincrement' }` and `validation.isRequired` is set

.changeset/update-field-hooks.md

@@ -0,0 +1,5 @@
+---
+"@keystone-6/core": patch
+---
+
+Update built-in fields to use newer validate hook syntax

examples/framework-astro/src/keystone/schema.ts

@@ -46,11 +46,11 @@ export const lists = {
       title: text({ validation: { isRequired: true } }),
       // we use this field to arbitrarily restrict Posts to only be viewed on a particular browser (using Post.access.filter)
       browser: select({
+        validation: { isRequired: true },
         options: [
           { label: 'Chrome', value: 'chrome' },
           { label: 'Firefox', value: 'firefox' },
         ],
-        validation: { isRequired: true },
       }),
     },
   }),

examples/testing/example-test.ts

@@ -33,7 +33,7 @@ test('Check that trying to create user with no name (required field) fails', asy
       })
     },
     {
-      message: 'You provided invalid data for this operation.\n  - User.name: Name must not be empty',
+      message: 'You provided invalid data for this operation.\n  - User.name: value must not be empty',
     }
   )
 })

packages/core/src/fields/non-null-graphql.ts

@@ -1,40 +1,92 @@
-import { type BaseListTypeInfo, type CommonFieldConfig, type FieldData } from '../types'
+import {
+  type BaseListTypeInfo,
+  type FieldData,
+} from '../types'
+import {
+  type ValidateFieldHook
+} from '../types/config/hooks'
 
-export function getResolvedIsNullable (
+export function resolveDbNullable (
   validation: undefined | { isRequired?: boolean },
   db: undefined | { isNullable?: boolean }
 ): boolean {
-  if (db?.isNullable === false) {
-    return false
-  }
+  if (db?.isNullable === false) return false
   if (db?.isNullable === undefined && validation?.isRequired) {
     return false
   }
   return true
 }
 
-export function resolveHasValidation ({
-  db,
-  validation
-}: {
-  db?: { isNullable?: boolean }
-  validation?: unknown
-}) {
-  if (db?.isNullable === false) return true
-  if (validation !== undefined) return true
-  return false
+export function makeValidateHook <ListTypeInfo extends BaseListTypeInfo> (
+  meta: FieldData,
+  config: {
+    label?: string,
+    db?: {
+      isNullable?: boolean
+    },
+    graphql?: {
+      isNonNull?: {
+        read?: boolean
+      }
+    },
+    validation?: {
+      isRequired?: boolean
+      [key: string]: unknown
+    },
+  },
+  f?: ValidateFieldHook<ListTypeInfo, 'create' | 'update' | 'delete', ListTypeInfo['fields']>
+) {
+  const dbNullable = resolveDbNullable(config.validation, config.db)
+  const mode = dbNullable ? ('optional' as const) : ('required' as const)
+  const valueRequired = config.validation?.isRequired || !dbNullable
+
+  assertReadIsNonNullAllowed(meta, config, dbNullable)
+  const addValidation = config.db?.isNullable === false || config.validation?.isRequired
+  if (addValidation) {
+    const validate = async function (args) {
+      const { operation, addValidationError, resolvedData } = args
+
+      if (valueRequired) {
+        const value = resolvedData?.[meta.fieldKey]
+        if (
+            (operation === 'create' && value === undefined)
+        || ((operation === 'create' || operation === 'update') && (value === null))
+        ) {
+          addValidationError(`missing value`)
+        }
+      }
+
+      await f?.(args)
+    } satisfies ValidateFieldHook<ListTypeInfo, 'create' | 'update' | 'delete', ListTypeInfo['fields']>
+
+    return {
+      mode,
+      validate,
+    }
+  }
+
+  return {
+    mode,
+    validate: f
+  }
 }
 
 export function assertReadIsNonNullAllowed<ListTypeInfo extends BaseListTypeInfo> (
   meta: FieldData,
-  config: CommonFieldConfig<ListTypeInfo>,
-  resolvedIsNullable: boolean
+  config: {
+    graphql?: {
+      isNonNull?: {
+        read?: boolean
+      }
+    }
+  },
+  dbNullable: boolean
 ) {
-  if (!resolvedIsNullable) return
+  if (!dbNullable) return
   if (!config.graphql?.isNonNull?.read) return
 
   throw new Error(
-    `The field at ${meta.listKey}.${meta.fieldKey} sets graphql.isNonNull.read: true, but not validation.isRequired: true, or db.isNullable: false\n` +
+    `${meta.listKey}.${meta.fieldKey} sets graphql.isNonNull.read: true, but not validation.isRequired: true (or db.isNullable: false)\n` +
       `Set validation.isRequired: true, or db.isNullable: false, or graphql.isNonNull.read: false`
   )
 }

packages/core/src/fields/resolve-hooks.ts

@@ -0,0 +1,71 @@
+import {
+  type BaseListTypeInfo,
+  type FieldHooks,
+  type MaybePromise
+} from '../types'
+
+// force new syntax for built-in fields
+//   and block hooks from using resolveInput, they should use GraphQL resolvers
+export type InternalFieldHooks<ListTypeInfo extends BaseListTypeInfo> =
+  Omit<FieldHooks<ListTypeInfo>, 'validateInput' | 'validateDelete' | 'resolveInput'>
+
+/** @deprecated, TODO: remove in breaking change */
+function resolveValidateHooks <ListTypeInfo extends BaseListTypeInfo> ({
+  validate,
+  validateInput,
+  validateDelete
+}: FieldHooks<ListTypeInfo>): Exclude<FieldHooks<ListTypeInfo>["validate"], Function> | undefined {
+  if (validateInput || validateDelete) {
+    return {
+      create: validateInput,
+      update: validateInput,
+      delete: validateDelete,
+    }
+  }
+
+  if (!validate) return
+  if (typeof validate === 'function') {
+    return {
+      create: validate,
+      update: validate,
+      delete: validate
+    }
+  }
+
+  return validate
+}
+
+function merge <
+  R,
+  A extends (r: R) => MaybePromise<void>,
+  B extends (r: R) => MaybePromise<void>
+> (a?: A, b?: B) {
+  if (!a && !b) return undefined
+  return async (args: R) => {
+    await a?.(args)
+    await b?.(args)
+  }
+}
+
+export function mergeFieldHooks <ListTypeInfo extends BaseListTypeInfo> (
+  builtin?: InternalFieldHooks<ListTypeInfo>,
+  hooks?: FieldHooks<ListTypeInfo>,
+) {
+  if (hooks === undefined) return builtin
+  if (builtin === undefined) return hooks
+
+  const builtinValidate = resolveValidateHooks(builtin)
+  const hooksValidate = resolveValidateHooks(hooks)
+  return {
+    ...hooks,
+    // WARNING: beforeOperation is _after_ a user beforeOperation hook, TODO: this is align with user expectations about when "operations" happen
+    //   our *Operation hooks are built-in, and should happen nearest to the database
+    beforeOperation: merge(hooks.beforeOperation, builtin.beforeOperation),
+    afterOperation: merge(builtin.afterOperation, hooks.afterOperation),
+    validate: (builtinValidate || hooksValidate) ? {
+      create: merge(builtinValidate?.create, hooksValidate?.create),
+      update: merge(builtinValidate?.update, hooksValidate?.update),
+      delete: merge(builtinValidate?.delete, hooksValidate?.delete)
+    } : undefined,
+  } satisfies FieldHooks<ListTypeInfo>
+}