Update built-in fields to new validate hook syntax

closes keystonejs#9165 - this updates the built-in fields using validateInput to use the new unified validation syntax, and handles merging with the old syntax to ensure user-land old syntax still works
acburdine · Jun 5, 2024 · ea10e02 · ea10e02
1 parent 1259e82
commit ea10e02
Show file tree

Hide file tree

Showing 15 changed files with 424 additions and 285 deletions.
diff --git a/.changeset/honest-moles-doubt.md b/.changeset/honest-moles-doubt.md
@@ -0,0 +1,5 @@
+---
+"@keystone-6/core": patch
+---
+
+Update built-in fields to use newer validate hook syntax
diff --git a/packages/core/src/fields/non-null-graphql.ts b/packages/core/src/fields/non-null-graphql.ts
@@ -13,13 +13,10 @@ export function getResolvedIsNullable (
   return true
 }
 
-export function resolveHasValidation ({
-  db,
-  validation
-}: {
-  db?: { isNullable?: boolean }
+export function resolveHasValidation (
+  db?: { isNullable?: boolean },
   validation?: unknown
-}) {
+) {
   if (db?.isNullable === false) return true
   if (validation !== undefined) return true
   return false

diff --git a/packages/core/src/fields/resolve-hooks.ts b/packages/core/src/fields/resolve-hooks.ts
@@ -0,0 +1,78 @@
+import type { FieldHooks, BaseListTypeInfo } from '../types'
+
+function splitValidateHooks <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 undefined 
+
+  if (typeof validate === 'function') {
+    return { create: validate, update: validate, delete: validate }
+  }
+
+  return validate
+}
+
+// force new syntax for built-in fields
+// also, we don't allow built-in hooks to specify resolveInput,
+// since they can do it via graphql resolvers
+export type InternalFieldHooks<ListTypeInfo extends BaseListTypeInfo> = 
+  Omit<FieldHooks<ListTypeInfo>, 'validateInput' | 'validateDelete' | 'resolveInput'>
+
+/**
+ * Utility function to convert deprecated field hook syntax to the new syntax
+ * Handles merging any built-in field hooks into the user-provided hooks
+ */
+export function mergeFieldHooks <ListTypeInfo extends BaseListTypeInfo> (
+  builtin?: InternalFieldHooks<ListTypeInfo>,
+  hooks: FieldHooks<ListTypeInfo> = {},
+): FieldHooks<ListTypeInfo> {
+  if (builtin === undefined) return hooks
+
+  const result: FieldHooks<ListTypeInfo> = {
+    resolveInput: hooks?.resolveInput,
+  }
+
+  if (hooks.beforeOperation || builtin.beforeOperation) {
+    result.beforeOperation = async (args) => {
+      await hooks.beforeOperation?.(args)
+      await builtin.beforeOperation?.(args)
+    }
+  }
+
+  if (hooks.afterOperation || builtin.afterOperation) {
+    result.afterOperation = async (args) => {
+      await hooks.afterOperation?.(args)
+      await builtin.afterOperation?.(args)
+    }
+  }
+
+  const builtinValidate = splitValidateHooks(builtin)
+  const fieldValidate = splitValidateHooks(hooks)
+
+  if (builtinValidate || fieldValidate) {
+    result.validate = {
+      create: async (args) => {
+        await builtinValidate?.create?.(args)
+        await fieldValidate?.create?.(args)
+      },
+      update: async (args) => {
+        await builtinValidate?.update?.(args)
+        await fieldValidate?.update?.(args)
+      },
+      delete: async (args) => {
+        await builtinValidate?.delete?.(args)
+        await fieldValidate?.delete?.(args)
+      },
+    }
+  }
+
+  return result
+}
diff --git a/packages/core/src/fields/types/bigInt/index.ts b/packages/core/src/fields/types/bigInt/index.ts
@@ -13,6 +13,7 @@ import {
   resolveHasValidation,
 } from '../../non-null-graphql'
 import { filters } from '../../filters'
+import { mergeFieldHooks, type InternalFieldHooks } from '../../resolve-hooks'
 
 export type BigIntFieldConfig<ListTypeInfo extends BaseListTypeInfo> =
   CommonFieldConfig<ListTypeInfo> & {
@@ -84,7 +85,37 @@ export function bigInt <ListTypeInfo extends BaseListTypeInfo> (
 
     const mode = isNullable === false ? 'required' : 'optional'
     const fieldLabel = config.label ?? humanize(meta.fieldKey)
-    const hasValidation = resolveHasValidation(config)
+    const hasValidation = resolveHasValidation(config.db, validation)
+
+    const hooks: InternalFieldHooks<ListTypeInfo> = {}
+    if (hasValidation) {
+      hooks.validate = ({ resolvedData, operation, addValidationError }) => {
+        if (operation === 'delete') return
+
+        const value = resolvedData[meta.fieldKey]
+
+        if (
+          (validation?.isRequired || isNullable === false) &&
+          (value === null ||
+            (operation === 'create' && value === undefined && !hasAutoIncDefault))
+        ) {
+          addValidationError(`${fieldLabel} is required`)
+        }
+        if (typeof value === 'number') {
+          if (validation?.min !== undefined && value < validation.min) {
+            addValidationError(
+              `${fieldLabel} must be greater than or equal to ${validation.min}`
+            )
+          }
+
+          if (validation?.max !== undefined && value > validation.max) {
+            addValidationError(
+              `${fieldLabel} must be less than or equal to ${validation.max}`
+            )
+          }
+        }
+      }
+    }
 
     return fieldType({
       kind: 'scalar',
@@ -102,35 +133,7 @@ export function bigInt <ListTypeInfo extends BaseListTypeInfo> (
       extendPrismaSchema: config.db?.extendPrismaSchema,
     })({
       ...config,
-      hooks: {
-        ...config.hooks,
-        validateInput: hasValidation ? async (args) => {
-          const value = args.resolvedData[meta.fieldKey]
-
-          if (
-            (validation?.isRequired || isNullable === false) &&
-            (value === null ||
-              (args.operation === 'create' && value === undefined && !hasAutoIncDefault))
-          ) {
-            args.addValidationError(`${fieldLabel} is required`)
-          }
-          if (typeof value === 'number') {
-            if (validation?.min !== undefined && value < validation.min) {
-              args.addValidationError(
-                `${fieldLabel} must be greater than or equal to ${validation.min}`
-              )
-            }
-
-            if (validation?.max !== undefined && value > validation.max) {
-              args.addValidationError(
-                `${fieldLabel} must be less than or equal to ${validation.max}`
-              )
-            }
-          }
-
-          await config.hooks?.validateInput?.(args)
-        } : config.hooks?.validateInput
-      },
+      hooks: mergeFieldHooks(hooks, config.hooks),
       input: {
         uniqueWhere:
           isIndexed === 'unique' ? { arg: graphql.arg({ type: graphql.BigInt }) } : undefined,

diff --git a/packages/core/src/fields/types/calendarDay/index.ts b/packages/core/src/fields/types/calendarDay/index.ts
@@ -7,9 +7,14 @@ import {
   orderDirectionEnum,
 } from '../../../types'
 import { graphql } from '../../..'
-import { assertReadIsNonNullAllowed, getResolvedIsNullable } from '../../non-null-graphql'
+import {
+  assertReadIsNonNullAllowed,
+  getResolvedIsNullable,
+  resolveHasValidation,
+} from '../../non-null-graphql'
 import { filters } from '../../filters'
 import { type CalendarDayFieldMeta } from './views'
+import { mergeFieldHooks, type InternalFieldHooks } from '../../resolve-hooks'
 
 export type CalendarDayFieldConfig<ListTypeInfo extends BaseListTypeInfo> =
   CommonFieldConfig<ListTypeInfo> & {
@@ -49,6 +54,7 @@ export const calendarDay =
     const mode = resolvedIsNullable === false ? 'required' : 'optional'
     const fieldLabel = config.label ?? humanize(meta.fieldKey)
     const usesNativeDateType = meta.provider === 'postgresql' || meta.provider === 'mysql'
+    const hasValidation = resolveHasValidation(config.db, validation)
 
     function resolveInput (value: string | null | undefined) {
       if (meta.provider === 'sqlite' || value == null) {
@@ -59,6 +65,18 @@ export const calendarDay =
 
     const commonResolveFilter = mode === 'optional' ? filters.resolveCommon : <T>(x: T) => x
 
+    const hooks: InternalFieldHooks<ListTypeInfo> = {}
+    if (hasValidation) {
+      hooks.validate = ({ resolvedData, addValidationError, operation }) => {
+        if (operation === 'delete') return
+
+        const value = resolvedData[meta.fieldKey]
+        if ((validation?.isRequired || resolvedIsNullable === false) && value === null) {
+          addValidationError(`${fieldLabel} is required`)
+        }
+      }
+    }
+
     return fieldType({
       kind: 'scalar',
       mode,
@@ -76,17 +94,7 @@ export const calendarDay =
       nativeType: usesNativeDateType ? 'Date' : undefined,
     })({
       ...config,
-      hooks: {
-        ...config.hooks,
-        async validateInput (args) {
-          const value = args.resolvedData[meta.fieldKey]
-          if ((validation?.isRequired || resolvedIsNullable === false) && value === null) {
-            args.addValidationError(`${fieldLabel} is required`)
-          }
-
-          await config.hooks?.validateInput?.(args)
-        },
-      },
+      hooks: mergeFieldHooks(hooks, config.hooks),
       input: {
         uniqueWhere:
           isIndexed === 'unique'

diff --git a/packages/core/src/fields/types/decimal/index.ts b/packages/core/src/fields/types/decimal/index.ts
@@ -9,9 +9,14 @@ import {
   type FieldData,
 } from '../../../types'
 import { graphql } from '../../..'
-import { assertReadIsNonNullAllowed, getResolvedIsNullable } from '../../non-null-graphql'
+import {
+  assertReadIsNonNullAllowed,
+  getResolvedIsNullable,
+  resolveHasValidation,
+} from '../../non-null-graphql'
 import { filters } from '../../filters'
 import { type DecimalFieldMeta } from './views'
+import { mergeFieldHooks, type InternalFieldHooks } from '../../resolve-hooks'
 
 export type DecimalFieldConfig<ListTypeInfo extends BaseListTypeInfo> =
   CommonFieldConfig<ListTypeInfo> & {
@@ -104,6 +109,7 @@ export const decimal =
         : parseDecimalValueOption(meta, defaultValue, 'defaultValue')
 
     const isNullable = getResolvedIsNullable(validation, config.db)
+    const hasValidation = resolveHasValidation(config.db, validation)
 
     assertReadIsNonNullAllowed(meta, config, isNullable)
 
@@ -120,29 +126,32 @@ export const decimal =
       map: config.db?.map,
       extendPrismaSchema: config.db?.extendPrismaSchema,
     } as const
-    return fieldType(dbField)({
-      ...config,
-      hooks: {
-        ...config.hooks,
-        async validateInput (args) {
-          const val: Decimal | null | undefined = args.resolvedData[meta.fieldKey]
 
-          if (val === null && (validation?.isRequired || isNullable === false)) {
-            args.addValidationError(`${fieldLabel} is required`)
+    const hooks: InternalFieldHooks<ListTypeInfo> = {}
+    if (hasValidation) {
+      hooks.validate = ({ resolvedData, addValidationError, operation }) => {
+        if (operation === 'delete') return
+
+        const val: Decimal | null | undefined = resolvedData[meta.fieldKey]
+
+        if (val === null && (validation?.isRequired || isNullable === false)) {
+          addValidationError(`${fieldLabel} is required`)
+        }
+        if (val != null) {
+          if (min !== undefined && val.lessThan(min)) {
+            addValidationError(`${fieldLabel} must be greater than or equal to ${min}`)
           }
-          if (val != null) {
-            if (min !== undefined && val.lessThan(min)) {
-              args.addValidationError(`${fieldLabel} must be greater than or equal to ${min}`)
-            }
 
-            if (max !== undefined && val.greaterThan(max)) {
-              args.addValidationError(`${fieldLabel} must be less than or equal to ${max}`)
-            }
+          if (max !== undefined && val.greaterThan(max)) {
+            addValidationError(`${fieldLabel} must be less than or equal to ${max}`)
           }
+        }
+      }
+    }
 
-          await config.hooks?.validateInput?.(args)
-        },
-      },
+    return fieldType(dbField)({
+      ...config,
+      hooks: mergeFieldHooks(hooks, config.hooks),
       input: {
         uniqueWhere:
           isIndexed === 'unique' ? { arg: graphql.arg({ type: graphql.Decimal }) } : undefined,

diff --git a/packages/core/src/fields/types/file/index.ts b/packages/core/src/fields/types/file/index.ts
@@ -7,6 +7,7 @@ import {
   fieldType,
 } from '../../../types'
 import { graphql } from '../../..'
+import { mergeFieldHooks, type InternalFieldHooks } from '../../resolve-hooks'
 
 export type FileFieldConfig<ListTypeInfo extends BaseListTypeInfo> =
   CommonFieldConfig<ListTypeInfo> & {
@@ -64,6 +65,27 @@ export function file <ListTypeInfo extends BaseListTypeInfo> (config: FileFieldC
       throw Error("isIndexed: 'unique' is not a supported option for field type file")
     }
 
+    const hooks: InternalFieldHooks<ListTypeInfo> = {}
+    if (!storage.preserve) {
+      hooks.beforeOperation = async function (args) {
+        if (args.operation === 'update' || args.operation === 'delete') {
+          const filenameKey = `${fieldKey}_filename`
+          const filename = args.item[filenameKey]
+
+          // This will occur on an update where a file already existed but has been
+          // changed, or on a delete, where there is no longer an item
+          if (
+            (args.operation === 'delete' ||
+              typeof args.resolvedData[fieldKey].filename === 'string' ||
+              args.resolvedData[fieldKey].filename === null) &&
+            typeof filename === 'string'
+          ) {
+            await args.context.files(config.storage).deleteAtSource(filename)
+          }
+        }
+      }
+    }
+
     return fieldType({
       kind: 'multi',
       extendPrismaSchema: config.db?.extendPrismaSchema,
@@ -73,29 +95,7 @@ export function file <ListTypeInfo extends BaseListTypeInfo> (config: FileFieldC
       },
     })({
       ...config,
-      hooks: storage.preserve
-        ? config.hooks
-        : {
-            ...config.hooks,
-            async beforeOperation (args) {
-              await config.hooks?.beforeOperation?.(args)
-              if (args.operation === 'update' || args.operation === 'delete') {
-                const filenameKey = `${fieldKey}_filename`
-                const filename = args.item[filenameKey]
-
-                // This will occur on an update where a file already existed but has been
-                // changed, or on a delete, where there is no longer an item
-                if (
-                  (args.operation === 'delete' ||
-                    typeof args.resolvedData[fieldKey].filename === 'string' ||
-                    args.resolvedData[fieldKey].filename === null) &&
-                  typeof filename === 'string'
-                ) {
-                  await args.context.files(config.storage).deleteAtSource(filename)
-                }
-              }
-            },
-          },
+      hooks: mergeFieldHooks(hooks, config.hooks),
       input: {
         create: {
           arg: inputArg,