Use required types internally for hooks (#8808)

keystonejs · Sep 18, 2023 · 3da69fe · 3da69fe
1 parent c9bcb45
commit 3da69fe
Show file tree

Hide file tree

Showing 8 changed files with 293 additions and 218 deletions.
diff --git a/packages/auth/src/types.ts b/packages/auth/src/types.ts
@@ -1,4 +1,4 @@
-import { BaseListTypeInfo, KeystoneContext } from '@keystone-6/core/types';
+import type { BaseListTypeInfo, KeystoneContext } from '@keystone-6/core/types';
 
 export type AuthGqlNames = {
   CreateInitialInput: string;

diff --git a/packages/core/src/lib/core/initialise-lists.ts b/packages/core/src/lib/core/initialise-lists.ts
@@ -159,9 +159,19 @@ function getIsEnabled(listsConfig: KeystoneConfig['lists']) {
   return isEnabled;
 }
 
+function defaultOperationHook() {}
 function defaultListHooksResolveInput({ resolvedData }: { resolvedData: any }) {
   return resolvedData;
 }
+function defaultFieldHooksResolveInput({
+  resolvedData,
+  fieldKey,
+}: {
+  resolvedData: any;
+  fieldKey: string;
+}) {
+  return resolvedData[fieldKey];
+}
 
 function parseListHooksResolveInput(f: ListHooks<BaseListTypeInfo>['resolveInput']) {
   if (typeof f === 'function') {
@@ -171,53 +181,29 @@ function parseListHooksResolveInput(f: ListHooks<BaseListTypeInfo>['resolveInput
     };
   }
 
-  const { create, update } = f ?? {};
-  return {
-    create: create ?? defaultListHooksResolveInput,
-    update: update ?? defaultListHooksResolveInput,
-  };
+  const { create = defaultListHooksResolveInput, update = defaultListHooksResolveInput } = f ?? {};
+  return { create, update };
 }
 
 function parseListHooks(hooks: ListHooks<BaseListTypeInfo>): ResolvedListHooks<BaseListTypeInfo> {
   return {
-    ...hooks,
     resolveInput: parseListHooksResolveInput(hooks.resolveInput),
+    validateInput: hooks.validateInput ?? defaultOperationHook,
+    validateDelete: hooks.validateDelete ?? defaultOperationHook,
+    beforeOperation: hooks.beforeOperation ?? defaultOperationHook,
+    afterOperation: hooks.afterOperation ?? defaultOperationHook,
   };
 }
 
-function defaultFieldHooksResolveInput({
-  resolvedData,
-  fieldKey,
-}: {
-  resolvedData: any;
-  fieldKey: string;
-}) {
-  return resolvedData[fieldKey];
-}
-
-function parseFieldHooksResolveInput(f: FieldHooks<BaseListTypeInfo>['resolveInput']) {
-  return f ?? defaultFieldHooksResolveInput;
-  // TODO: one day
-  //    if (typeof f === 'function') {
-  //      return {
-  //        create: f,
-  //        update: f,
-  //      };
-  //    }
-  //
-  //    const { create, update } = f ?? {};
-  //    return {
-  //      create: create ?? defaultFieldHooksResolveInput,
-  //      update: update ?? defaultFieldHooksResolveInput,
-  //    };
-}
-
 function parseFieldHooks(
   hooks: FieldHooks<BaseListTypeInfo>
 ): ResolvedFieldHooks<BaseListTypeInfo> {
   return {
-    ...hooks,
-    resolveInput: parseFieldHooksResolveInput(hooks.resolveInput),
+    resolveInput: hooks.resolveInput ?? defaultFieldHooksResolveInput,
+    validateInput: hooks.validateInput ?? defaultOperationHook,
+    validateDelete: hooks.validateDelete ?? defaultOperationHook,
+    beforeOperation: hooks.beforeOperation ?? defaultOperationHook,
+    afterOperation: hooks.afterOperation ?? defaultOperationHook,
   };
 }
 
@@ -780,8 +766,9 @@ export function initialiseLists(config: KeystoneConfig): Record<string, Initiali
         hasAnEnabledUpdateField = true;
       }
     }
-    // You can't have a graphQL type with no fields, so
-    // if they're all disabled, we have to disable the whole operation.
+
+    // you can't have empty GraphQL types
+    //   if empty, omit the type completely
     if (!hasAnEnabledCreateField) {
       list.graphql.isEnabled.create = false;
     }
@@ -790,12 +777,12 @@ export function initialiseLists(config: KeystoneConfig): Record<string, Initiali
     }
   }
 
-  // Error checking
+  // error checking
   for (const [listKey, { fields }] of Object.entries(intermediateLists)) {
     assertFieldsValid({ listKey, fields });
   }
 
-  // Fixup the GraphQL refs
+  // fixup the GraphQL refs
   for (const [listKey, intermediateList] of Object.entries(intermediateLists)) {
     listsRef[listKey] = {
       ...intermediateList,

diff --git a/packages/core/src/lib/core/mutations/create-update.ts b/packages/core/src/lib/core/mutations/create-update.ts
@@ -299,13 +299,10 @@ async function getResolvedData(
   resolvedData = Object.fromEntries(
     await Promise.all(
       Object.entries(list.fields).map(async ([fieldKey, field]) => {
-        if (field.hooks.resolveInput === undefined) return [fieldKey, resolvedData[fieldKey]];
-
-        const resolver = field.hooks.resolveInput;
         try {
           return [
             fieldKey,
-            await resolver({
+            await field.hooks.resolveInput({
               ...hookArgs,
               resolvedData,
               fieldKey,
@@ -380,7 +377,7 @@ async function resolveInputForCreateOrUpdate(
         // `hookArgs` based on the `operation` which will make `hookArgs.item`
         // be the right type for `originalItem` for the operation
         hookArgs.operation === 'create'
-          ? { ...hookArgs, item: updatedItem, originalItem: hookArgs.item }
+          ? { ...hookArgs, item: updatedItem, originalItem: undefined }
           : { ...hookArgs, item: updatedItem, originalItem: hookArgs.item }
       );
     },

diff --git a/packages/core/src/lib/core/mutations/hooks.ts b/packages/core/src/lib/core/mutations/hooks.ts
@@ -1,43 +1,28 @@
 import { extensionError } from '../graphql-errors';
+import type { InitialisedList } from '../initialise-lists';
 
 export async function runSideEffectOnlyHook<
-  HookName extends string,
-  List extends {
-    fields: Record<
-      string,
-      {
-        hooks: {
-          [Key in HookName]?: (args: { fieldKey: string } & Args) => Promise<void> | void;
-        };
-      }
-    >;
-    hooks: {
-      [Key in HookName]?: (args: any) => Promise<void> | void;
-    };
-    listKey: string;
-  },
-  Args extends Parameters<NonNullable<List['hooks'][HookName]>>[0]
->(list: List, hookName: HookName, args: Args) {
-  // Runs the before/after operation hooks
-
+  HookName extends 'beforeOperation' | 'afterOperation',
+  Args extends Parameters<NonNullable<InitialisedList['hooks'][HookName]>>[0]
+>(list: InitialisedList, hookName: HookName, args: Args) {
   let shouldRunFieldLevelHook: (fieldKey: string) => boolean;
   if (args.operation === 'delete') {
-    // Always run field hooks for delete operations
+    // always run field hooks for delete operations
     shouldRunFieldLevelHook = () => true;
   } else {
-    // Only run field hooks on if the field was specified in the
-    // original input for create and update operations.
+    // only run field hooks on if the field was specified in the
+    //   original input for create and update operations.
     const inputDataKeys = new Set(Object.keys(args.inputData));
     shouldRunFieldLevelHook = fieldKey => inputDataKeys.has(fieldKey);
   }
 
-  // Field hooks
+  // field hooks
   const fieldsErrors: { error: Error; tag: string }[] = [];
   await Promise.all(
     Object.entries(list.fields).map(async ([fieldKey, field]) => {
       if (shouldRunFieldLevelHook(fieldKey)) {
         try {
-          await field.hooks[hookName]?.({ fieldKey, ...args });
+          await field.hooks[hookName]({ fieldKey, ...args } as any); // TODO: FIXME any
         } catch (error: any) {
           fieldsErrors.push({ error, tag: `${list.listKey}.${fieldKey}.hooks.${hookName}` });
         }
@@ -49,9 +34,9 @@ export async function runSideEffectOnlyHook<
     throw extensionError(hookName, fieldsErrors);
   }
 
-  // List hooks
+  // list hooks
   try {
-    await list.hooks[hookName]?.(args);
+    await list.hooks[hookName](args as any); // TODO: FIXME any
   } catch (error: any) {
     throw extensionError(hookName, [{ error, tag: `${list.listKey}.hooks.${hookName}` }]);
   }

diff --git a/packages/core/src/lib/core/mutations/validation.ts b/packages/core/src/lib/core/mutations/validation.ts
@@ -22,7 +22,7 @@ export async function validateUpdateCreate({
       const addValidationError = (msg: string) =>
         messages.push(`${list.listKey}.${fieldKey}: ${msg}`);
       try {
-        await field.hooks.validateInput?.({ ...hookArgs, addValidationError, fieldKey });
+        await field.hooks.validateInput({ ...hookArgs, addValidationError, fieldKey });
       } catch (error: any) {
         fieldsErrors.push({ error, tag: `${list.listKey}.${fieldKey}.hooks.validateInput` });
       }
@@ -36,7 +36,7 @@ export async function validateUpdateCreate({
   // List validation hooks
   const addValidationError = (msg: string) => messages.push(`${list.listKey}: ${msg}`);
   try {
-    await list.hooks.validateInput?.({ ...hookArgs, addValidationError });
+    await list.hooks.validateInput({ ...hookArgs, addValidationError });
   } catch (error: any) {
     throw extensionError('validateInput', [{ error, tag: `${list.listKey}.hooks.validateInput` }]);
   }
@@ -62,7 +62,7 @@ export async function validateDelete({
       const addValidationError = (msg: string) =>
         messages.push(`${list.listKey}.${fieldKey}: ${msg}`);
       try {
-        await field.hooks.validateDelete?.({ ...hookArgs, addValidationError, fieldKey });
+        await field.hooks.validateDelete({ ...hookArgs, addValidationError, fieldKey });
       } catch (error: any) {
         fieldsErrors.push({ error, tag: `${list.listKey}.${fieldKey}.hooks.validateDelete` });
       }
@@ -74,7 +74,7 @@ export async function validateDelete({
   // List validation
   const addValidationError = (msg: string) => messages.push(`${list.listKey}: ${msg}`);
   try {
-    await list.hooks.validateDelete?.({ ...hookArgs, addValidationError });
+    await list.hooks.validateDelete({ ...hookArgs, addValidationError });
   } catch (error: any) {
     throw extensionError('validateDelete', [
       { error, tag: `${list.listKey}.hooks.validateDelete` },

diff --git a/packages/core/src/types/config/access-control.ts b/packages/core/src/types/config/access-control.ts
@@ -5,7 +5,7 @@ import type { BaseListTypeInfo } from '../type-info';
 export type BaseAccessArgs<ListTypeInfo extends BaseListTypeInfo> = {
   context: KeystoneContext<ListTypeInfo['all']>;
   session?: ListTypeInfo['all']['session'];
-  listKey: string;
+  listKey: ListTypeInfo['key'];
 };
 
 export type AccessOperation = 'create' | 'query' | 'update' | 'delete';

diff --git a/packages/core/src/types/config/fields.ts b/packages/core/src/types/config/fields.ts
@@ -13,8 +13,8 @@ export type BaseFields<ListTypeInfo extends BaseListTypeInfo> = {
 export type FilterOrderArgs<ListTypeInfo extends BaseListTypeInfo> = {
   context: KeystoneContext<ListTypeInfo['all']>;
   session?: ListTypeInfo['all']['session'];
-  listKey: string;
-  fieldKey: string;
+  listKey: ListTypeInfo['key'];
+  fieldKey: ListTypeInfo['fields'];
 };
 
 export type CommonFieldConfig<ListTypeInfo extends BaseListTypeInfo> = {