-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Corrected defaulting & non-required empty property handling #542
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ import { | |
| retrieveSchema, | ||
| toIdSchema, | ||
| getDefaultRegistry, | ||
| cleanUpNonRequiredArray, | ||
| } from "../../utils"; | ||
|
|
||
| function ArrayFieldTitle({ TitleField, idSchema, title, required }) { | ||
|
|
@@ -202,14 +203,19 @@ class ArrayField extends Component { | |
|
|
||
| onAddClick = event => { | ||
| event.preventDefault(); | ||
| const { schema, registry, formData } = this.props; | ||
| const { schema, registry, required } = this.props; | ||
| const { definitions } = registry; | ||
| let formData = this.props.formData; | ||
| let itemSchema = schema.items; | ||
| if (isFixedItems(schema) && allowAdditionalItems(schema)) { | ||
| itemSchema = schema.additionalItems; | ||
| } | ||
|
|
||
| this.props.onChange( | ||
| [...formData, getDefaultFormState(itemSchema, undefined, definitions)], | ||
| [ | ||
| ...formData, | ||
| getDefaultFormState(itemSchema, undefined, definitions, required), | ||
| ], | ||
| { validate: false } | ||
| ); | ||
| }; | ||
|
|
@@ -219,9 +225,18 @@ class ArrayField extends Component { | |
| if (event) { | ||
| event.preventDefault(); | ||
| } | ||
| const { formData, onChange } = this.props; | ||
| const { onChange, required } = this.props; | ||
| let formData = this.props.formData; | ||
|
|
||
| // if field isn't required and doesn't contain any entries | ||
| // remove the whole property from formData to guarantee correct validation | ||
| if (!required) { | ||
| formData = cleanUpNonRequiredArray(formData); | ||
| } | ||
| // refs #195: revalidate to ensure properly reindexing errors | ||
| onChange(formData.filter((_, i) => i !== index), { validate: true }); | ||
|
|
||
| formData = formData ? formData.filter((_, i) => i !== index) : formData; | ||
| onChange(formData, { validate: true }); | ||
| }; | ||
| }; | ||
|
|
||
|
|
@@ -250,12 +265,13 @@ class ArrayField extends Component { | |
| onChangeForIndex = index => { | ||
| return value => { | ||
| const { formData, onChange } = this.props; | ||
| const newFormData = formData.map((item, i) => { | ||
| let newFormData = formData.map((item, i) => { | ||
|
Contributor
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. Nit: Does this still need to be |
||
| // We need to treat undefined items as nulls to have validation. | ||
| // See https://github.com/tdegrunt/jsonschema/issues/206 | ||
| const jsonValue = typeof value === "undefined" ? null : value; | ||
| return index === i ? jsonValue : item; | ||
| }); | ||
|
|
||
| onChange(newFormData, { validate: false }); | ||
| }; | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -103,7 +103,12 @@ export function getWidget(schema, widget, registeredWidgets = {}) { | |
| throw new Error(`No widget "${widget}" for type "${type}"`); | ||
| } | ||
|
|
||
| function computeDefaults(schema, parentDefaults, definitions = {}) { | ||
| function computeDefaults( | ||
| schema, | ||
| parentDefaults, | ||
| definitions = {}, | ||
| required = false | ||
| ) { | ||
| // Compute the defaults recursively: give highest priority to deepest nodes. | ||
| let defaults = parentDefaults; | ||
| if (isObject(defaults) && isObject(schema.default)) { | ||
|
|
@@ -116,10 +121,10 @@ function computeDefaults(schema, parentDefaults, definitions = {}) { | |
| } else if ("$ref" in schema) { | ||
| // Use referenced schema defaults for this node. | ||
| const refSchema = findSchemaDefinition(schema.$ref, definitions); | ||
| return computeDefaults(refSchema, defaults, definitions); | ||
| return computeDefaults(refSchema, defaults, definitions, required); | ||
| } else if (isFixedItems(schema)) { | ||
| defaults = schema.items.map(itemSchema => | ||
| computeDefaults(itemSchema, undefined, definitions)); | ||
| computeDefaults(itemSchema, undefined, definitions, required)); | ||
| } | ||
| // Not defaults defined for this node, fallback to generic typed ones. | ||
| if (typeof defaults === "undefined") { | ||
|
|
@@ -133,32 +138,76 @@ function computeDefaults(schema, parentDefaults, definitions = {}) { | |
| (acc, key) => { | ||
| // Compute the defaults for this node, with the parent defaults we might | ||
| // have from a previous run: defaults[key]. | ||
| acc[key] = computeDefaults( | ||
| schema.properties[key], | ||
| (defaults || {})[key], | ||
| definitions | ||
| ); | ||
| if (acc) { | ||
| acc[key] = computeDefaults( | ||
| schema.properties[key], | ||
| (defaults || {})[key], | ||
| definitions, | ||
| schema.required && schema.required.indexOf(key) > -1 | ||
| ); | ||
|
|
||
| if (!required) { | ||
| acc = cleanUpNonRequiredObject(acc); | ||
| } | ||
| } | ||
| return acc; | ||
| }, | ||
| {} | ||
| ); | ||
|
|
||
| case "array": | ||
| if (schema.minItems) { | ||
| if (required && schema.minItems) { | ||
| return new Array(schema.minItems).fill( | ||
| computeDefaults(schema.items, defaults, definitions) | ||
| computeDefaults(schema.items, defaults, definitions, required) | ||
|
Contributor
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. Should |
||
| ); | ||
| } | ||
| } | ||
| return defaults; | ||
| } | ||
|
|
||
| export function getDefaultFormState(_schema, formData, definitions = {}) { | ||
| export function cleanUpNonRequiredObject(values) { | ||
| const cleanValues = []; | ||
|
|
||
| for (let key in values) { | ||
| if (values.hasOwnProperty(key) && typeof values[key] !== "undefined") { | ||
| cleanValues.push(values[key]); | ||
| } | ||
| } | ||
|
|
||
| if (!cleanValues.length) { | ||
| values = undefined; | ||
| } | ||
|
|
||
| return values; | ||
| } | ||
|
|
||
| export function cleanUpNonRequiredArray(values) { | ||
| values = values.filter(item => { | ||
| return item !== null && item !== undefined; | ||
|
Contributor
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. Why do we need to filter out undefined values here? |
||
| }); | ||
|
|
||
| values = values.length ? values : undefined; | ||
|
|
||
| return values; | ||
| } | ||
|
|
||
| export function getDefaultFormState( | ||
| _schema, | ||
| formData, | ||
| definitions = {}, | ||
| required = true | ||
| ) { | ||
| if (!isObject(_schema)) { | ||
| throw new Error("Invalid schema: " + _schema); | ||
| } | ||
|
|
||
| const schema = retrieveSchema(_schema, definitions); | ||
| const defaults = computeDefaults(schema, _schema.default, definitions); | ||
| const defaults = computeDefaults( | ||
| schema, | ||
| _schema.default, | ||
| definitions, | ||
| required | ||
| ); | ||
| if (typeof formData === "undefined") { | ||
| // No form data? Use schema defaults. | ||
| return defaults; | ||
|
|
@@ -197,6 +246,8 @@ export function isObject(thing) { | |
|
|
||
| export function mergeObjects(obj1, obj2, concatArrays = false) { | ||
| // Recursively merge deeply nested objects. | ||
| obj1 = obj1 || {}; | ||
| obj2 = obj2 || {}; | ||
|
Contributor
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. What is this change about? |
||
| var acc = Object.assign({}, obj1); // Prevent mutation of source object. | ||
| return Object.keys(obj2).reduce( | ||
| (acc, key) => { | ||
|
|
||
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.
Shouldn't we be doing this
filterbefore theif (!required)check?