From d43390f2a4c5ebeb7b9b0f07e003816005efc761 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 26 Jul 2021 22:25:11 -0400 Subject: [PATCH] feat: improves conditional logic performance and edge cases --- .../forms/Form/buildStateFromSchema.ts | 13 ++------- .../components/forms/Form/fieldReducer.ts | 25 ++++++++++++++++- src/admin/components/forms/Form/index.tsx | 28 +++++++------------ src/admin/components/forms/Form/types.ts | 4 +-- .../forms/field-types/Array/Array.tsx | 2 -- .../forms/field-types/Blocks/Blocks.tsx | 2 -- .../forms/field-types/Checkbox/index.tsx | 2 -- .../forms/field-types/Code/Code.tsx | 2 -- .../forms/field-types/DateTime/index.tsx | 2 -- .../forms/field-types/Email/index.tsx | 2 -- .../forms/field-types/Number/index.tsx | 2 -- .../forms/field-types/RadioGroup/index.tsx | 2 -- .../forms/field-types/Relationship/index.tsx | 2 -- .../forms/field-types/RichText/RichText.tsx | 2 -- .../forms/field-types/Select/index.tsx | 2 -- .../forms/field-types/Text/index.tsx | 2 -- .../forms/field-types/Textarea/index.tsx | 2 -- .../forms/field-types/Upload/index.tsx | 2 -- .../components/forms/useFieldType/index.tsx | 4 +-- .../components/forms/useFieldType/types.ts | 3 +- .../components/forms/withCondition/index.tsx | 24 +++++++++------- src/fields/traverseFields.ts | 10 +++++++ src/fields/validationPromise.ts | 4 ++- 23 files changed, 67 insertions(+), 76 deletions(-) diff --git a/src/admin/components/forms/Form/buildStateFromSchema.ts b/src/admin/components/forms/Form/buildStateFromSchema.ts index b87980fb2d6..6b7ae209c7b 100644 --- a/src/admin/components/forms/Form/buildStateFromSchema.ts +++ b/src/admin/components/forms/Form/buildStateFromSchema.ts @@ -2,18 +2,12 @@ import ObjectID from 'bson-objectid'; import { Field as FieldSchema } from '../../../../fields/config/types'; import { Fields, Field, Data } from './types'; -const buildValidationPromise = async (fieldState: Field, field: FieldSchema, fullData: Data = {}, data: Data = {}) => { +const buildValidationPromise = async (fieldState: Field, field: FieldSchema) => { const validatedFieldState = fieldState; - let passesConditionalLogic = true; - - if (field?.admin?.condition) { - passesConditionalLogic = await field.admin.condition(fullData, data); - } - let validationResult: boolean | string = true; - if (passesConditionalLogic && typeof field.validate === 'function') { + if (typeof field.validate === 'function') { validationResult = await field.validate(fieldState.value, field); } @@ -37,10 +31,9 @@ const buildStateFromSchema = async (fieldSchema: FieldSchema[], fullData: Data = initialValue: value, valid: true, validate: field.validate, - condition: field?.admin?.condition, }; - validationPromises.push(buildValidationPromise(fieldState, field, fullData, data)); + validationPromises.push(buildValidationPromise(fieldState, field)); return fieldState; }; diff --git a/src/admin/components/forms/Form/fieldReducer.ts b/src/admin/components/forms/Form/fieldReducer.ts index 2d1939984e9..e42bacc8761 100644 --- a/src/admin/components/forms/Form/fieldReducer.ts +++ b/src/admin/components/forms/Form/fieldReducer.ts @@ -103,6 +103,29 @@ function fieldReducer(state: Fields, action): Fields { return newState; } + case 'MODIFY_CONDITION': { + const { path, result } = action; + + return Object.entries(state).reduce((newState, [key, val]) => { + if (key === path || key.indexOf(`${path}.`) === 0) { + return { + ...newState, + [key]: { + ...val, + passesCondition: result, + }, + }; + } + + return { + ...newState, + [key]: { + ...val, + }, + }; + }, {}); + } + default: { const newField = { value: action.value, @@ -113,7 +136,7 @@ function fieldReducer(state: Fields, action): Fields { initialValue: action.initialValue, stringify: action.stringify, validate: action.validate, - condition: action.condition, + passesCondition: action.passesCondition, }; return { diff --git a/src/admin/components/forms/Form/index.tsx b/src/admin/components/forms/Form/index.tsx index 591789e3c7e..b9de6331406 100644 --- a/src/admin/components/forms/Form/index.tsx +++ b/src/admin/components/forms/Form/index.tsx @@ -67,32 +67,24 @@ const Form: React.FC = (props) => { const validatedFieldState = {}; let isValid = true; - const data = contextRef.current.getData(); - const validationPromises = Object.entries(contextRef.current.fields).map(async ([path, field]) => { const validatedField = { ...field, valid: true, }; - const siblingData = contextRef.current.getSiblingData(path); - - let passesConditionalLogic = true; - - if (typeof field?.condition === 'function') { - passesConditionalLogic = await field.condition(data, siblingData); - } - - let validationResult: boolean | string = true; + if (field.passesCondition !== false) { + let validationResult: boolean | string = true; - if (passesConditionalLogic && typeof field.validate === 'function') { - validationResult = await field.validate(field.value); - } + if (typeof field.validate === 'function') { + validationResult = await field.validate(field.value); + } - if (typeof validationResult === 'string') { - validatedField.errorMessage = validationResult; - validatedField.valid = false; - isValid = false; + if (typeof validationResult === 'string') { + validatedField.errorMessage = validationResult; + validatedField.valid = false; + isValid = false; + } } validatedFieldState[path] = validatedField; diff --git a/src/admin/components/forms/Form/types.ts b/src/admin/components/forms/Form/types.ts index bc19f86af80..50c6d6de892 100644 --- a/src/admin/components/forms/Form/types.ts +++ b/src/admin/components/forms/Form/types.ts @@ -1,5 +1,3 @@ -import { Condition } from '../../../../fields/config/types'; - export type Field = { value: unknown initialValue: unknown @@ -9,7 +7,7 @@ export type Field = { disableFormData?: boolean ignoreWhileFlattening?: boolean stringify?: boolean - condition?: Condition + passesCondition?: boolean } export type Fields = { diff --git a/src/admin/components/forms/field-types/Array/Array.tsx b/src/admin/components/forms/field-types/Array/Array.tsx index e2423516810..a26677aa946 100644 --- a/src/admin/components/forms/field-types/Array/Array.tsx +++ b/src/admin/components/forms/field-types/Array/Array.tsx @@ -30,7 +30,6 @@ const ArrayFieldType: React.FC = (props) => { permissions, admin: { readOnly, - condition, }, } = props; @@ -69,7 +68,6 @@ const ArrayFieldType: React.FC = (props) => { validate: memoizedValidate, disableFormData, ignoreWhileFlattening: true, - condition, }); const addRow = useCallback(async (rowIndex) => { diff --git a/src/admin/components/forms/field-types/Blocks/Blocks.tsx b/src/admin/components/forms/field-types/Blocks/Blocks.tsx index f168c8d9045..30221b1b3fa 100644 --- a/src/admin/components/forms/field-types/Blocks/Blocks.tsx +++ b/src/admin/components/forms/field-types/Blocks/Blocks.tsx @@ -44,7 +44,6 @@ const Blocks: React.FC = (props) => { permissions, admin: { readOnly, - condition, }, } = props; @@ -79,7 +78,6 @@ const Blocks: React.FC = (props) => { validate: memoizedValidate, disableFormData, ignoreWhileFlattening: true, - condition, }); const addRow = useCallback(async (rowIndex, blockType) => { diff --git a/src/admin/components/forms/field-types/Checkbox/index.tsx b/src/admin/components/forms/field-types/Checkbox/index.tsx index c32f78c6b21..a9af7805e73 100644 --- a/src/admin/components/forms/field-types/Checkbox/index.tsx +++ b/src/admin/components/forms/field-types/Checkbox/index.tsx @@ -23,7 +23,6 @@ const Checkbox: React.FC = (props) => { readOnly, style, width, - condition, } = {}, } = props; @@ -43,7 +42,6 @@ const Checkbox: React.FC = (props) => { path, validate: memoizedValidate, disableFormData, - condition, }); return ( diff --git a/src/admin/components/forms/field-types/Code/Code.tsx b/src/admin/components/forms/field-types/Code/Code.tsx index 77f279cd2d3..72b30027df8 100644 --- a/src/admin/components/forms/field-types/Code/Code.tsx +++ b/src/admin/components/forms/field-types/Code/Code.tsx @@ -23,7 +23,6 @@ const Code: React.FC = (props) => { style, width, language, - condition, } = {}, label, minLength, @@ -54,7 +53,6 @@ const Code: React.FC = (props) => { path, validate: memoizedValidate, enableDebouncedValue: true, - condition, }); const classes = [ diff --git a/src/admin/components/forms/field-types/DateTime/index.tsx b/src/admin/components/forms/field-types/DateTime/index.tsx index ae37e7ae3fb..49fb0861b50 100644 --- a/src/admin/components/forms/field-types/DateTime/index.tsx +++ b/src/admin/components/forms/field-types/DateTime/index.tsx @@ -25,7 +25,6 @@ const DateTime: React.FC = (props) => { style, width, date, - condition, } = {}, } = props; @@ -44,7 +43,6 @@ const DateTime: React.FC = (props) => { } = useFieldType({ path, validate: memoizedValidate, - condition, }); const classes = [ diff --git a/src/admin/components/forms/field-types/Email/index.tsx b/src/admin/components/forms/field-types/Email/index.tsx index ddd3bb4ac9e..cd5e9a42be0 100644 --- a/src/admin/components/forms/field-types/Email/index.tsx +++ b/src/admin/components/forms/field-types/Email/index.tsx @@ -20,7 +20,6 @@ const Email: React.FC = (props) => { width, placeholder, autoComplete, - condition, } = {}, label, } = props; @@ -36,7 +35,6 @@ const Email: React.FC = (props) => { path, validate: memoizedValidate, enableDebouncedValue: true, - condition, }); const { diff --git a/src/admin/components/forms/field-types/Number/index.tsx b/src/admin/components/forms/field-types/Number/index.tsx index a84cae676bc..7c55f5622a0 100644 --- a/src/admin/components/forms/field-types/Number/index.tsx +++ b/src/admin/components/forms/field-types/Number/index.tsx @@ -23,7 +23,6 @@ const NumberField: React.FC = (props) => { width, step, placeholder, - condition, } = {}, } = props; @@ -43,7 +42,6 @@ const NumberField: React.FC = (props) => { path, validate: memoizedValidate, enableDebouncedValue: true, - condition, }); const handleChange = useCallback((e) => { diff --git a/src/admin/components/forms/field-types/RadioGroup/index.tsx b/src/admin/components/forms/field-types/RadioGroup/index.tsx index 553d49c14f6..e751b0cb740 100644 --- a/src/admin/components/forms/field-types/RadioGroup/index.tsx +++ b/src/admin/components/forms/field-types/RadioGroup/index.tsx @@ -25,7 +25,6 @@ const RadioGroup: React.FC = (props) => { layout = 'horizontal', style, width, - condition, } = {}, options, } = props; @@ -45,7 +44,6 @@ const RadioGroup: React.FC = (props) => { } = useFieldType({ path, validate: memoizedValidate, - condition, }); const classes = [ diff --git a/src/admin/components/forms/field-types/Relationship/index.tsx b/src/admin/components/forms/field-types/Relationship/index.tsx index 4fb5cc4fbc6..defa2253989 100644 --- a/src/admin/components/forms/field-types/Relationship/index.tsx +++ b/src/admin/components/forms/field-types/Relationship/index.tsx @@ -35,7 +35,6 @@ const Relationship: React.FC = (props) => { readOnly, style, width, - condition, } = {}, } = props; @@ -72,7 +71,6 @@ const Relationship: React.FC = (props) => { } = useFieldType({ path: path || name, validate: memoizedValidate, - condition, }); const addOptions = useCallback((data, relation) => { diff --git a/src/admin/components/forms/field-types/RichText/RichText.tsx b/src/admin/components/forms/field-types/RichText/RichText.tsx index cf939a8c6c6..3eb113db5a3 100644 --- a/src/admin/components/forms/field-types/RichText/RichText.tsx +++ b/src/admin/components/forms/field-types/RichText/RichText.tsx @@ -43,7 +43,6 @@ const RichText: React.FC = (props) => { style, width, placeholder, - condition, hideGutter, } = {}, } = props; @@ -105,7 +104,6 @@ const RichText: React.FC = (props) => { path, validate: memoizedValidate, stringify: true, - condition, }); const { diff --git a/src/admin/components/forms/field-types/Select/index.tsx b/src/admin/components/forms/field-types/Select/index.tsx index 9a5ae8e9141..210d2d9972c 100644 --- a/src/admin/components/forms/field-types/Select/index.tsx +++ b/src/admin/components/forms/field-types/Select/index.tsx @@ -36,7 +36,6 @@ const Select: React.FC = (props) => { readOnly, style, width, - condition, } = {}, } = props; @@ -57,7 +56,6 @@ const Select: React.FC = (props) => { } = useFieldType({ path, validate: memoizedValidate, - condition, }); const classes = [ diff --git a/src/admin/components/forms/field-types/Text/index.tsx b/src/admin/components/forms/field-types/Text/index.tsx index 792e95803ad..97c96cd9b71 100644 --- a/src/admin/components/forms/field-types/Text/index.tsx +++ b/src/admin/components/forms/field-types/Text/index.tsx @@ -20,7 +20,6 @@ const Text: React.FC = (props) => { readOnly, style, width, - condition, } = {}, } = props; @@ -29,7 +28,6 @@ const Text: React.FC = (props) => { const fieldType = useFieldType({ path, validate, - condition, enableDebouncedValue: true, }); diff --git a/src/admin/components/forms/field-types/Textarea/index.tsx b/src/admin/components/forms/field-types/Textarea/index.tsx index 22192690a5f..a3805b59467 100644 --- a/src/admin/components/forms/field-types/Textarea/index.tsx +++ b/src/admin/components/forms/field-types/Textarea/index.tsx @@ -20,7 +20,6 @@ const Textarea: React.FC = (props) => { width, placeholder, rows, - condition, } = {}, label, minLength, @@ -43,7 +42,6 @@ const Textarea: React.FC = (props) => { path, validate: memoizedValidate, enableDebouncedValue: true, - condition, }); const classes = [ diff --git a/src/admin/components/forms/field-types/Upload/index.tsx b/src/admin/components/forms/field-types/Upload/index.tsx index fc72c333ab9..16450047c18 100644 --- a/src/admin/components/forms/field-types/Upload/index.tsx +++ b/src/admin/components/forms/field-types/Upload/index.tsx @@ -30,7 +30,6 @@ const Upload: React.FC = (props) => { readOnly, style, width, - condition, } = {}, label, validate = upload, @@ -52,7 +51,6 @@ const Upload: React.FC = (props) => { const fieldType = useFieldType({ path, validate: memoizedValidate, - condition, }); const { diff --git a/src/admin/components/forms/useFieldType/index.tsx b/src/admin/components/forms/useFieldType/index.tsx index 9de075b9f38..f3ca2483783 100644 --- a/src/admin/components/forms/useFieldType/index.tsx +++ b/src/admin/components/forms/useFieldType/index.tsx @@ -13,7 +13,6 @@ const useFieldType = (options: Options): FieldType => { disableFormData, ignoreWhileFlattening, stringify, - condition, } = options; const formContext = useForm(); @@ -49,7 +48,6 @@ const useFieldType = (options: Options): FieldType => { ignoreWhileFlattening, initialValue, validate, - condition, value: valueToSend, valid: false, errorMessage: undefined, @@ -65,7 +63,7 @@ const useFieldType = (options: Options): FieldType => { } dispatchFields(fieldToDispatch); - }, [path, dispatchFields, validate, disableFormData, ignoreWhileFlattening, initialValue, stringify, condition]); + }, [path, dispatchFields, validate, disableFormData, ignoreWhileFlattening, initialValue, stringify]); // Method to return from `useFieldType`, used to // update internal field values from field component(s) diff --git a/src/admin/components/forms/useFieldType/types.ts b/src/admin/components/forms/useFieldType/types.ts index 164bc2ce949..cb0ca529b00 100644 --- a/src/admin/components/forms/useFieldType/types.ts +++ b/src/admin/components/forms/useFieldType/types.ts @@ -1,4 +1,4 @@ -import { Validate, Condition } from '../../../../fields/config/types'; +import { Validate } from '../../../../fields/config/types'; export type Options = { path: string @@ -7,7 +7,6 @@ export type Options = { disableFormData?: boolean ignoreWhileFlattening?: boolean stringify?: boolean - condition?: Condition } export type FieldType = { diff --git a/src/admin/components/forms/withCondition/index.tsx b/src/admin/components/forms/withCondition/index.tsx index 9236b667bd7..235c79cc600 100644 --- a/src/admin/components/forms/withCondition/index.tsx +++ b/src/admin/components/forms/withCondition/index.tsx @@ -34,20 +34,24 @@ const withCondition =

>(Field: React.Component const data = getData(); const siblingData = getSiblingData(path); - const passesCondition = condition ? condition(data, siblingData) : true; + const hasCondition = Boolean(condition); + const currentlyPassesCondition = hasCondition ? condition(data, siblingData) : true; + const field = getField(path); + const existingConditionPasses = field?.passesCondition; useEffect(() => { - if (!passesCondition) { - const field = getField(path); - dispatchFields({ - ...field, - path, - valid: true, - }); + if (hasCondition) { + if (!existingConditionPasses && currentlyPassesCondition) { + dispatchFields({ type: 'MODIFY_CONDITION', path, result: true }); + } + + if (!currentlyPassesCondition && (existingConditionPasses || typeof existingConditionPasses === 'undefined')) { + dispatchFields({ type: 'MODIFY_CONDITION', path, result: false }); + } } - }, [passesCondition, getField, dispatchFields, path]); + }, [currentlyPassesCondition, existingConditionPasses, dispatchFields, path, hasCondition]); - if (passesCondition) { + if (currentlyPassesCondition) { return ; } diff --git a/src/fields/traverseFields.ts b/src/fields/traverseFields.ts index 1c0ed533a24..50f082f57c2 100644 --- a/src/fields/traverseFields.ts +++ b/src/fields/traverseFields.ts @@ -34,6 +34,7 @@ type Arguments = { unflattenLocales: boolean unflattenLocaleActions: (() => void)[] docWithLocales?: Record + skipValidation?: boolean } const traverseFields = (args: Arguments): void => { @@ -64,6 +65,7 @@ const traverseFields = (args: Arguments): void => { unflattenLocaleActions, unflattenLocales, docWithLocales = {}, + skipValidation, } = args; fields.forEach((field) => { @@ -187,11 +189,14 @@ const traverseFields = (args: Arguments): void => { fullData, })); + const passesCondition = (field.admin?.condition && hook === 'beforeChange') ? field.admin.condition(fullData, data) : true; + if (fieldHasSubFields(field)) { if (field.name === undefined) { traverseFields({ ...args, fields: field.fields, + skipValidation: !passesCondition, }); } else if (fieldIsArrayType(field)) { if (Array.isArray(data[field.name])) { @@ -207,6 +212,7 @@ const traverseFields = (args: Arguments): void => { originalDoc: originalDoc?.[field.name]?.[i], docWithLocales: docWithLocales?.[field.name]?.[i], path: `${path}${field.name}.${i}.`, + skipValidation: !passesCondition, }); } } @@ -218,6 +224,7 @@ const traverseFields = (args: Arguments): void => { originalDoc: originalDoc[field.name], docWithLocales: docWithLocales?.[field.name], path: `${path}${field.name}.`, + skipValidation: !passesCondition, }); } } @@ -235,6 +242,7 @@ const traverseFields = (args: Arguments): void => { originalDoc: originalDoc?.[field.name]?.[i], docWithLocales: docWithLocales?.[field.name]?.[i], path: `${path}${field.name}.${i}.`, + skipValidation: !passesCondition, }); } }); @@ -267,6 +275,7 @@ const traverseFields = (args: Arguments): void => { existingData: { [field.name]: existingRowCount }, field, path, + skipValidation: skipValidation || !passesCondition, })); } else { validationPromises.push(() => validationPromise({ @@ -276,6 +285,7 @@ const traverseFields = (args: Arguments): void => { existingData: originalDoc, field, path, + skipValidation: skipValidation || !passesCondition, })); } } diff --git a/src/fields/validationPromise.ts b/src/fields/validationPromise.ts index 58644fd2347..c1fd4b5e97d 100644 --- a/src/fields/validationPromise.ts +++ b/src/fields/validationPromise.ts @@ -7,6 +7,7 @@ type Arguments = { errors: {message: string, field: string}[] newData: Record existingData: Record + skipValidation?: boolean } const validationPromise = async ({ @@ -16,8 +17,9 @@ const validationPromise = async ({ existingData, field, path, + skipValidation, }: Arguments): Promise => { - if (hook !== 'beforeChange') return true; + if (hook !== 'beforeChange' || !skipValidation) return true; const hasCondition = field.admin && field.admin.condition; const shouldValidate = field.validate && !hasCondition;