-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Bug: Issue with formData not updating when dependencies change #4388
Merged
+2,142
−1,519
Merged
Changes from 17 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
2e04b8f
Fixing issue with formData value not changing when dependencies are u…
abdalla-rko 24db3a3
refactoring tests and added test to test non-valid formData
abdalla-rko 3e16130
update changeLog
abdalla-rko be667a2
Merge remote-tracking branch 'origin/main' into valid-formdata
abdalla-rko 9e8bd14
added test for getValidFormData method
abdalla-rko 8b6cfce
fixed failing issue because of type change
abdalla-rko 9f00d5b
Merge branch 'main' into valid-formdata
heath-freenome c8de92f
changes based on feedback
abdalla-rko 62e3385
changes based on feedback
abdalla-rko b522bc3
Merge remote-tracking branch 'origin/main' into valid-formdata
abdalla-rko c153ac4
Merge remote-tracking branch 'origin/main' into valid-formdata
abdalla-rko 3571dc9
fixed test coverage
abdalla-rko cecbe97
Override the formData with the const if the constAsDefaults is set to…
abdalla-rko 0ffe426
Fixed issue with validator-ajv6 ignoring oneOf in dependencies becaus…
abdalla-rko 64afea6
improvement based on feedback
abdalla-rko 43949c9
Fixed failing tests
abdalla-rko 761a882
removed unnecessary code
abdalla-rko 6817fa0
Merge branch 'main' into valid-formdata
abdalla-rko 895e480
improvement based on feedback.
abdalla-rko File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ import get from 'lodash/get'; | |
|
||
import isObject from './isObject'; | ||
import { GenericObjectType } from '../src'; | ||
import isNil from 'lodash/isNil'; | ||
|
||
/** Merges the `defaults` object of type `T` into the `formData` of type `T` | ||
* | ||
|
@@ -19,47 +20,72 @@ import { GenericObjectType } from '../src'; | |
* @param [formData] - The form data into which the defaults will be merged | ||
* @param [mergeExtraArrayDefaults=false] - If true, any additional default array entries are appended onto the formData | ||
* @param [defaultSupercedesUndefined=false] - If true, an explicit undefined value will be overwritten by the default value | ||
* @param [overrideFormDataWithDefaults=false] - If true, the default value will overwrite the form data value. If the value | ||
* doesn't exist in the default, we take it from formData and in the case where the value is set to undefined in formData. | ||
* This is useful when we have already merged formData with defaults and want to add an additional field from formData | ||
* that does not exist in defaults. | ||
* @returns - The resulting merged form data with defaults | ||
*/ | ||
export default function mergeDefaultsWithFormData<T = any>( | ||
defaults?: T, | ||
formData?: T, | ||
mergeExtraArrayDefaults = false, | ||
defaultSupercedesUndefined = false | ||
defaultSupercedesUndefined = false, | ||
overrideFormDataWithDefaults = false | ||
): T | undefined { | ||
if (Array.isArray(formData)) { | ||
const defaultsArray = Array.isArray(defaults) ? defaults : []; | ||
const mapped = formData.map((value, idx) => { | ||
if (defaultsArray[idx]) { | ||
|
||
// If overrideFormDataWithDefaults is true, we want to override the formData with the defaults | ||
const overrideArray = overrideFormDataWithDefaults ? defaultsArray : formData; | ||
const overrideOppositeArray = overrideFormDataWithDefaults ? formData : defaultsArray; | ||
|
||
const mapped = overrideArray.map((value, idx) => { | ||
if (overrideOppositeArray[idx]) { | ||
return mergeDefaultsWithFormData<any>( | ||
defaultsArray[idx], | ||
value, | ||
formData[idx], | ||
mergeExtraArrayDefaults, | ||
defaultSupercedesUndefined | ||
defaultSupercedesUndefined, | ||
overrideFormDataWithDefaults | ||
); | ||
} | ||
return value; | ||
}); | ||
|
||
// Merge any extra defaults when mergeExtraArrayDefaults is true | ||
if (mergeExtraArrayDefaults && mapped.length < defaultsArray.length) { | ||
mapped.push(...defaultsArray.slice(mapped.length)); | ||
// Or when overrideFormDataWithDefaults is true and the default array is shorter than the formData array | ||
if ((mergeExtraArrayDefaults || overrideFormDataWithDefaults) && mapped.length < overrideOppositeArray.length) { | ||
mapped.push(...overrideOppositeArray.slice(mapped.length)); | ||
} | ||
return mapped as unknown as T; | ||
} | ||
if (isObject(formData)) { | ||
const acc: { [key in keyof T]: any } = Object.assign({}, defaults); // Prevent mutation of source object. | ||
return Object.keys(formData as GenericObjectType).reduce((acc, key) => { | ||
const keyValue = get(formData, key); | ||
const keyExistsInDefaults = isObject(defaults) && key in (defaults as GenericObjectType); | ||
const keyExistsInFormData = key in (formData as GenericObjectType); | ||
acc[key as keyof T] = mergeDefaultsWithFormData<T>( | ||
defaults ? get(defaults, key) : {}, | ||
get(formData, key), | ||
keyValue, | ||
mergeExtraArrayDefaults, | ||
defaultSupercedesUndefined | ||
defaultSupercedesUndefined, | ||
// overrideFormDataWithDefaults can be true only when the key value exists in defaults | ||
// Or if the key value doesn't exist in formData | ||
overrideFormDataWithDefaults && (keyExistsInDefaults || !keyExistsInFormData) | ||
); | ||
return acc; | ||
}, acc); | ||
} | ||
if (defaultSupercedesUndefined && formData === undefined) { | ||
if ( | ||
defaultSupercedesUndefined && | ||
((!isNil(defaults) && isNil(formData)) || (typeof formData === 'number' && isNaN(formData))) | ||
) { | ||
return defaults; | ||
} else if (overrideFormDataWithDefaults && isNil(formData)) { | ||
// If the overrideFormDataWithDefaults flag is true and formData is set to undefined or null return formData | ||
return formData; | ||
} | ||
return formData; | ||
return overrideFormDataWithDefaults ? defaults : formData; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a way to optimize this logic so that we have:
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -30,8 +30,12 @@ import { | |||||||||||
ValidatorType, | ||||||||||||
} from '../types'; | ||||||||||||
import isMultiSelect from './isMultiSelect'; | ||||||||||||
import isSelect from './isSelect'; | ||||||||||||
import retrieveSchema, { resolveDependencies } from './retrieveSchema'; | ||||||||||||
import isConstant from '../isConstant'; | ||||||||||||
import { JSONSchema7Object } from 'json-schema'; | ||||||||||||
import isEqual from 'lodash/isEqual'; | ||||||||||||
import optionsList from '../optionsList'; | ||||||||||||
|
||||||||||||
const PRIMITIVE_TYPES = ['string', 'number', 'integer', 'boolean', 'null']; | ||||||||||||
|
||||||||||||
|
@@ -169,6 +173,10 @@ interface ComputeDefaultsProps<T = any, S extends StrictRJSFSchema = RJSFSchema> | |||||||||||
experimental_customMergeAllOf?: Experimental_CustomMergeAllOf<S>; | ||||||||||||
/** Optional flag, if true, indicates this schema was required in the parent schema. */ | ||||||||||||
required?: boolean; | ||||||||||||
/** Optional flag, if true, It will merge defaults into formData. | ||||||||||||
* The formData should take precedence unless it's not valid. This is useful when for example the value from formData does not exist in the schema 'enum' property, in such cases we take the value from the defaults because the value from the formData is not valid. | ||||||||||||
*/ | ||||||||||||
shouldMergeDefaultsIntoFormData?: boolean; | ||||||||||||
} | ||||||||||||
|
||||||||||||
/** Computes the defaults for the current `schema` given the `rawFormData` and `parentDefaults` if any. This drills into | ||||||||||||
|
@@ -193,6 +201,7 @@ export function computeDefaults<T = any, S extends StrictRJSFSchema = RJSFSchema | |||||||||||
experimental_defaultFormStateBehavior = undefined, | ||||||||||||
experimental_customMergeAllOf = undefined, | ||||||||||||
required, | ||||||||||||
shouldMergeDefaultsIntoFormData = false, | ||||||||||||
} = computeDefaultsProps; | ||||||||||||
const formData: T = (isObject(rawFormData) ? rawFormData : {}) as T; | ||||||||||||
const schema: S = isObject(rawSchema) ? rawSchema : ({} as S); | ||||||||||||
|
@@ -245,6 +254,7 @@ export function computeDefaults<T = any, S extends StrictRJSFSchema = RJSFSchema | |||||||||||
parentDefaults: Array.isArray(parentDefaults) ? parentDefaults[idx] : undefined, | ||||||||||||
rawFormData: formData as T, | ||||||||||||
required, | ||||||||||||
shouldMergeDefaultsIntoFormData, | ||||||||||||
}) | ||||||||||||
) as T[]; | ||||||||||||
} else if (ONE_OF_KEY in schema) { | ||||||||||||
|
@@ -266,7 +276,7 @@ export function computeDefaults<T = any, S extends StrictRJSFSchema = RJSFSchema | |||||||||||
getClosestMatchingOption<T, S, F>( | ||||||||||||
validator, | ||||||||||||
rootSchema, | ||||||||||||
isEmpty(formData) ? undefined : formData, | ||||||||||||
rawFormData, | ||||||||||||
oneOf as S[], | ||||||||||||
0, | ||||||||||||
discriminator, | ||||||||||||
|
@@ -284,7 +294,7 @@ export function computeDefaults<T = any, S extends StrictRJSFSchema = RJSFSchema | |||||||||||
getClosestMatchingOption<T, S, F>( | ||||||||||||
validator, | ||||||||||||
rootSchema, | ||||||||||||
isEmpty(formData) ? undefined : formData, | ||||||||||||
rawFormData, | ||||||||||||
anyOf as S[], | ||||||||||||
0, | ||||||||||||
discriminator, | ||||||||||||
|
@@ -304,6 +314,7 @@ export function computeDefaults<T = any, S extends StrictRJSFSchema = RJSFSchema | |||||||||||
parentDefaults: defaults as T | undefined, | ||||||||||||
rawFormData: formData as T, | ||||||||||||
required, | ||||||||||||
shouldMergeDefaultsIntoFormData, | ||||||||||||
}); | ||||||||||||
} | ||||||||||||
|
||||||||||||
|
@@ -314,7 +325,67 @@ export function computeDefaults<T = any, S extends StrictRJSFSchema = RJSFSchema | |||||||||||
|
||||||||||||
const defaultBasedOnSchemaType = getDefaultBasedOnSchemaType(validator, schema, computeDefaultsProps, defaults); | ||||||||||||
|
||||||||||||
return defaultBasedOnSchemaType ?? defaults; | ||||||||||||
let defaultsWithFormData = defaultBasedOnSchemaType ?? defaults; | ||||||||||||
// if shouldMergeDefaultsIntoFormData is true, then merge the defaults into the formData. | ||||||||||||
if (shouldMergeDefaultsIntoFormData) { | ||||||||||||
const { arrayMinItems = {} } = experimental_defaultFormStateBehavior || {}; | ||||||||||||
const { mergeExtraDefaults } = arrayMinItems; | ||||||||||||
|
||||||||||||
const matchingFormData = ensureFormDataMatchingSchema( | ||||||||||||
validator, | ||||||||||||
schema, | ||||||||||||
rootSchema, | ||||||||||||
rawFormData, | ||||||||||||
experimental_defaultFormStateBehavior | ||||||||||||
); | ||||||||||||
if (!isObject(rawFormData)) { | ||||||||||||
defaultsWithFormData = mergeDefaultsWithFormData<T>( | ||||||||||||
defaultsWithFormData as T, | ||||||||||||
matchingFormData as T, | ||||||||||||
mergeExtraDefaults, | ||||||||||||
true | ||||||||||||
) as T; | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
return defaultsWithFormData; | ||||||||||||
} | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* Ensure that the formData matches the given schema. If it's not matching in the case of a selectField, we change it to match the schema. | ||||||||||||
* @param validator - an implementation of the `ValidatorType` interface that will be used when necessary | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
* @param schema - The schema for which the formData state is desired | ||||||||||||
* @param rootSchema - The root schema, used to primarily to look up `$ref`s | ||||||||||||
* @param formData - The current formData | ||||||||||||
* @param experimental_defaultFormStateBehavior - Optional configuration object, if provided, allows users to override default form state behavior | ||||||||||||
* @returns - valid formData that matches schema | ||||||||||||
*/ | ||||||||||||
export function ensureFormDataMatchingSchema< | ||||||||||||
T = any, | ||||||||||||
S extends StrictRJSFSchema = RJSFSchema, | ||||||||||||
F extends FormContextType = any | ||||||||||||
>( | ||||||||||||
validator: ValidatorType<T, S, F>, | ||||||||||||
schema: S, | ||||||||||||
rootSchema: S, | ||||||||||||
formData: T | undefined, | ||||||||||||
experimental_defaultFormStateBehavior?: Experimental_DefaultFormStateBehavior | ||||||||||||
): T | T[] | undefined { | ||||||||||||
const isSelectField = !isConstant(schema) && isSelect(validator, schema, rootSchema); | ||||||||||||
let validFormData: T | T[] | undefined = formData; | ||||||||||||
if (isSelectField) { | ||||||||||||
const getOptionsList = optionsList(schema); | ||||||||||||
const isValid = getOptionsList?.some((option) => isEqual(option.value, formData)); | ||||||||||||
validFormData = isValid ? formData : undefined; | ||||||||||||
} | ||||||||||||
|
||||||||||||
// Override the formData with the const if the constAsDefaults is set to always | ||||||||||||
const constTakesPrecedence = schema[CONST_KEY] && experimental_defaultFormStateBehavior?.constAsDefaults === 'always'; | ||||||||||||
if (constTakesPrecedence) { | ||||||||||||
validFormData = schema.const as any; | ||||||||||||
abdalla-rko marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
} | ||||||||||||
|
||||||||||||
return validFormData; | ||||||||||||
} | ||||||||||||
|
||||||||||||
/** Computes the default value for objects. | ||||||||||||
|
@@ -336,6 +407,7 @@ export function getObjectDefaults<T = any, S extends StrictRJSFSchema = RJSFSche | |||||||||||
experimental_defaultFormStateBehavior = undefined, | ||||||||||||
experimental_customMergeAllOf = undefined, | ||||||||||||
required, | ||||||||||||
shouldMergeDefaultsIntoFormData, | ||||||||||||
}: ComputeDefaultsProps<T, S> = {}, | ||||||||||||
defaults?: T | T[] | undefined | ||||||||||||
): T { | ||||||||||||
|
@@ -369,6 +441,7 @@ export function getObjectDefaults<T = any, S extends StrictRJSFSchema = RJSFSche | |||||||||||
parentDefaults: get(defaults, [key]), | ||||||||||||
rawFormData: get(formData, [key]), | ||||||||||||
required: retrievedSchema.required?.includes(key), | ||||||||||||
shouldMergeDefaultsIntoFormData, | ||||||||||||
}); | ||||||||||||
maybeAddDefaultToObject<T>( | ||||||||||||
acc, | ||||||||||||
|
@@ -413,6 +486,7 @@ export function getObjectDefaults<T = any, S extends StrictRJSFSchema = RJSFSche | |||||||||||
parentDefaults: get(defaults, [key]), | ||||||||||||
rawFormData: get(formData, [key]), | ||||||||||||
required: retrievedSchema.required?.includes(key), | ||||||||||||
shouldMergeDefaultsIntoFormData, | ||||||||||||
}); | ||||||||||||
// Since these are additional properties we don't need to add the `experimental_defaultFormStateBehavior` prop | ||||||||||||
maybeAddDefaultToObject<T>( | ||||||||||||
|
@@ -447,6 +521,7 @@ export function getArrayDefaults<T = any, S extends StrictRJSFSchema = RJSFSchem | |||||||||||
experimental_defaultFormStateBehavior = undefined, | ||||||||||||
experimental_customMergeAllOf = undefined, | ||||||||||||
required, | ||||||||||||
shouldMergeDefaultsIntoFormData, | ||||||||||||
}: ComputeDefaultsProps<T, S> = {}, | ||||||||||||
defaults?: T | T[] | undefined | ||||||||||||
): T | T[] | undefined { | ||||||||||||
|
@@ -474,6 +549,7 @@ export function getArrayDefaults<T = any, S extends StrictRJSFSchema = RJSFSchem | |||||||||||
experimental_customMergeAllOf, | ||||||||||||
parentDefaults: item, | ||||||||||||
required, | ||||||||||||
shouldMergeDefaultsIntoFormData, | ||||||||||||
}); | ||||||||||||
}) as T[]; | ||||||||||||
} | ||||||||||||
|
@@ -493,6 +569,7 @@ export function getArrayDefaults<T = any, S extends StrictRJSFSchema = RJSFSchem | |||||||||||
rawFormData: item, | ||||||||||||
parentDefaults: get(defaults, [idx]), | ||||||||||||
required, | ||||||||||||
shouldMergeDefaultsIntoFormData, | ||||||||||||
}); | ||||||||||||
}) as T[]; | ||||||||||||
|
||||||||||||
|
@@ -541,6 +618,7 @@ export function getArrayDefaults<T = any, S extends StrictRJSFSchema = RJSFSchem | |||||||||||
experimental_defaultFormStateBehavior, | ||||||||||||
experimental_customMergeAllOf, | ||||||||||||
required, | ||||||||||||
shouldMergeDefaultsIntoFormData, | ||||||||||||
}) | ||||||||||||
) as T[]; | ||||||||||||
// then fill up the rest with either the item default or empty, up to minItems | ||||||||||||
|
@@ -607,26 +685,33 @@ export default function getDefaultFormState< | |||||||||||
throw new Error('Invalid schema: ' + theSchema); | ||||||||||||
} | ||||||||||||
const schema = retrieveSchema<T, S, F>(validator, theSchema, rootSchema, formData, experimental_customMergeAllOf); | ||||||||||||
|
||||||||||||
// Get the computed defaults with 'shouldMergeDefaultsIntoFormData' set to true to merge defaults into formData. | ||||||||||||
// This is done when for example the value from formData does not exist in the schema 'enum' property, in such | ||||||||||||
// cases we take the value from the defaults because the value from the formData is not valid. | ||||||||||||
const defaults = computeDefaults<T, S, F>(validator, schema, { | ||||||||||||
rootSchema, | ||||||||||||
includeUndefinedValues, | ||||||||||||
experimental_defaultFormStateBehavior, | ||||||||||||
experimental_customMergeAllOf, | ||||||||||||
rawFormData: formData, | ||||||||||||
shouldMergeDefaultsIntoFormData: true, | ||||||||||||
}); | ||||||||||||
|
||||||||||||
if (formData === undefined || formData === null || (typeof formData === 'number' && isNaN(formData))) { | ||||||||||||
// No form data? Use schema defaults. | ||||||||||||
return defaults; | ||||||||||||
} | ||||||||||||
const { mergeDefaultsIntoFormData, arrayMinItems = {} } = experimental_defaultFormStateBehavior || {}; | ||||||||||||
const { mergeExtraDefaults } = arrayMinItems; | ||||||||||||
const defaultSupercedesUndefined = mergeDefaultsIntoFormData === 'useDefaultIfFormDataUndefined'; | ||||||||||||
if (isObject(formData)) { | ||||||||||||
return mergeDefaultsWithFormData<T>(defaults as T, formData, mergeExtraDefaults, defaultSupercedesUndefined); | ||||||||||||
} | ||||||||||||
if (Array.isArray(formData)) { | ||||||||||||
return mergeDefaultsWithFormData<T[]>(defaults as T[], formData, mergeExtraDefaults, defaultSupercedesUndefined); | ||||||||||||
// If the formData is an object or an array, add additional properties from formData and override formData with | ||||||||||||
// defaults since the defaults are already merged with formData. | ||||||||||||
if (isObject(formData) || Array.isArray(formData)) { | ||||||||||||
const { mergeDefaultsIntoFormData } = experimental_defaultFormStateBehavior || {}; | ||||||||||||
const defaultSupercedesUndefined = mergeDefaultsIntoFormData === 'useDefaultIfFormDataUndefined'; | ||||||||||||
const result = mergeDefaultsWithFormData<T>( | ||||||||||||
defaults as T, | ||||||||||||
abdalla-rko marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
formData, | ||||||||||||
true, // set to true to add any additional default array entries. | ||||||||||||
defaultSupercedesUndefined, | ||||||||||||
true // set to true to override formData with defaults if they exist. | ||||||||||||
); | ||||||||||||
return result; | ||||||||||||
} | ||||||||||||
return formData; | ||||||||||||
|
||||||||||||
return defaults; | ||||||||||||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be
5.23.3
since there isn't a new feature?