From 6728977b0085ef37c310b4d40d076a2622664ba7 Mon Sep 17 00:00:00 2001 From: Ashwin Ramaswami Date: Tue, 30 Jul 2019 11:23:06 -0700 Subject: [PATCH] Fix dependency defaults for uncontrolled components (#1371) * fix: don't return keys with undefined values in computeDefaults * fix: make sure getDefaultFormState keeps default data equal to 0 * fix: make sure existing formData equal to null is overwritten by defaults * fix: undo "don't return keys with undefined values" * fix: fix overwriting of data with defaults for all noneValues * fix: re-add fix of removing undefined values from computeDefaults * fix: include form data with undefined values when validateFormData is called * fix: render dependency defaults for uncontrolled components --- src/components/Form.js | 11 +++++++--- src/utils.js | 48 +++++++++++++++++++++++++++++++++++------- src/validate.js | 6 +++++- test/Form_test.js | 24 +++++++++++++++++++++ test/utils_test.js | 48 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 125 insertions(+), 12 deletions(-) diff --git a/src/components/Form.js b/src/components/Form.js index 9201bcd62f..4eacafc8d4 100644 --- a/src/components/Form.js +++ b/src/components/Form.js @@ -13,6 +13,7 @@ import { getDefaultRegistry, deepEquals, toPathSchema, + isObject, } from "../utils"; import validateFormData, { toErrorList } from "../validate"; @@ -30,7 +31,7 @@ export default class Form extends Component { constructor(props) { super(props); - this.state = this.getStateFromProps(props); + this.state = this.getStateFromProps(props, props.formData); if ( this.props.onChange && !deepEquals(this.state.formData, this.props.formData) @@ -41,7 +42,7 @@ export default class Form extends Component { } componentWillReceiveProps(nextProps) { - const nextState = this.getStateFromProps(nextProps); + const nextState = this.getStateFromProps(nextProps, nextProps.formData); if ( !deepEquals(nextState.formData, nextProps.formData) && !deepEquals(nextState.formData, this.state.formData) && @@ -52,7 +53,7 @@ export default class Form extends Component { this.setState(nextState); } - getStateFromProps(props, inputFormData = props.formData) { + getStateFromProps(props, inputFormData) { const state = this.state || {}; const schema = "schema" in props ? props.schema : this.props.schema; const uiSchema = "uiSchema" in props ? props.uiSchema : this.props.uiSchema; @@ -171,6 +172,10 @@ export default class Form extends Component { }; onChange = (formData, newErrorSchema) => { + if (isObject(formData) || Array.isArray(formData)) { + const newState = this.getStateFromProps(this.props, formData); + formData = newState.formData; + } const mustValidate = !this.props.noValidate && this.props.liveValidate; let state = { formData }; let newFormData = formData; diff --git a/src/utils.js b/src/utils.js index 2099c0412e..44ef93834b 100644 --- a/src/utils.js +++ b/src/utils.js @@ -148,7 +148,8 @@ function computeDefaults( schema, parentDefaults, definitions, - rawFormData = {} + rawFormData = {}, + includeUndefinedValues = false ) { const formData = isObject(rawFormData) ? rawFormData : {}; // Compute the defaults recursively: give highest priority to deepest nodes. @@ -163,13 +164,31 @@ function computeDefaults( } else if ("$ref" in schema) { // Use referenced schema defaults for this node. const refSchema = findSchemaDefinition(schema.$ref, definitions); - return computeDefaults(refSchema, defaults, definitions, formData); + return computeDefaults( + refSchema, + defaults, + definitions, + formData, + includeUndefinedValues + ); } else if ("dependencies" in schema) { const resolvedSchema = resolveDependencies(schema, definitions, formData); - return computeDefaults(resolvedSchema, defaults, definitions, formData); + return computeDefaults( + resolvedSchema, + defaults, + definitions, + formData, + includeUndefinedValues + ); } else if (isFixedItems(schema)) { defaults = schema.items.map(itemSchema => - computeDefaults(itemSchema, undefined, definitions, formData) + computeDefaults( + itemSchema, + undefined, + definitions, + formData, + includeUndefinedValues + ) ); } else if ("oneOf" in schema) { schema = @@ -190,12 +209,16 @@ function computeDefaults( return Object.keys(schema.properties || {}).reduce((acc, key) => { // Compute the defaults for this node, with the parent defaults we might // have from a previous run: defaults[key]. - acc[key] = computeDefaults( + let computedDefault = computeDefaults( schema.properties[key], (defaults || {})[key], definitions, - (formData || {})[key] + (formData || {})[key], + includeUndefinedValues ); + if (includeUndefinedValues || computedDefault !== undefined) { + acc[key] = computedDefault; + } return acc; }, {}); @@ -225,7 +248,12 @@ function computeDefaults( return defaults; } -export function getDefaultFormState(_schema, formData, definitions = {}) { +export function getDefaultFormState( + _schema, + formData, + definitions = {}, + includeUndefinedValues = false +) { if (!isObject(_schema)) { throw new Error("Invalid schema: " + _schema); } @@ -234,7 +262,8 @@ export function getDefaultFormState(_schema, formData, definitions = {}) { schema, _schema.default, definitions, - formData + formData, + includeUndefinedValues ); if (typeof formData === "undefined") { // No form data? Use schema defaults. @@ -244,6 +273,9 @@ export function getDefaultFormState(_schema, formData, definitions = {}) { // Override schema defaults with form data. return mergeObjects(defaults, formData); } + if (formData === 0) { + return formData; + } return formData || defaults; } diff --git a/src/validate.js b/src/validate.js index 8454e93e90..e70aebfee1 100644 --- a/src/validate.js +++ b/src/validate.js @@ -1,7 +1,7 @@ import toPath from "lodash/toPath"; import Ajv from "ajv"; let ajv = createAjvInstance(); -import { deepEquals } from "./utils"; +import { deepEquals, getDefaultFormState } from "./utils"; let formerCustomFormats = null; let formerMetaSchema = null; @@ -172,6 +172,10 @@ export default function validateFormData( additionalMetaSchemas = [], customFormats = {} ) { + // Include form data with undefined values, which is required for validation. + const { definitions } = schema; + formData = getDefaultFormState(schema, formData, definitions, true); + const newMetaSchemas = !deepEquals(formerMetaSchema, additionalMetaSchemas); const newFormats = !deepEquals(formerCustomFormats, customFormats); diff --git a/test/Form_test.js b/test/Form_test.js index 1060274b1a..7084d553a6 100644 --- a/test/Form_test.js +++ b/test/Form_test.js @@ -2469,4 +2469,28 @@ describe("Form", () => { sinon.assert.notCalled(outerOnSubmit); }); }); + + describe("Dependency defaults", () => { + it("should show dependency defaults for uncontrolled components", () => { + const schema = { + type: "object", + properties: { + firstName: { type: "string" }, + }, + dependencies: { + firstName: { + properties: { + lastName: { type: "string", default: "Norris" }, + }, + }, + }, + }; + const { node } = createFormComponent({ schema }); + + Simulate.change(node.querySelector("#root_firstName"), { + target: { value: "Chuck" }, + }); + expect(node.querySelector("#root_lastName").value).eql("Norris"); + }); + }); }); diff --git a/test/utils_test.js b/test/utils_test.js index 56ed93ad03..d9210af1d8 100644 --- a/test/utils_test.js +++ b/test/utils_test.js @@ -35,6 +35,33 @@ describe("utils", () => { }) ).to.eql("foo"); }); + + it("should keep existing form data that is equal to 0", () => { + expect( + getDefaultFormState( + { + type: "number", + default: 1, + }, + 0 + ) + ).to.eql(0); + }); + + const noneValues = [null, undefined, NaN]; + noneValues.forEach(noneValue => { + it("should overwrite existing form data that is equal to a none value", () => { + expect( + getDefaultFormState( + { + type: "number", + default: 1, + }, + noneValue + ) + ).to.eql(1); + }); + }); }); describe("nested default", () => { @@ -723,6 +750,27 @@ describe("utils", () => { }); }); }); + + describe("with schema keys not defined in the formData", () => { + it("shouldn't add in undefined keys to formData", () => { + const schema = { + type: "object", + properties: { + foo: { type: "string" }, + bar: { type: "string" }, + }, + }; + const formData = { + foo: "foo", + baz: "baz", + }; + const result = { + foo: "foo", + baz: "baz", + }; + expect(getDefaultFormState(schema, formData)).to.eql(result); + }); + }); }); describe("asNumber()", () => {