Skip to content

Remove setState in ObjectField and ArrayField #480

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.

Sign up for GitHub

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

Merged
n1k0 merged 4 commits into from
Feb 22, 2017
Merged

Remove setState in ObjectField and ArrayField #480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8575a6f
Remove setState in ObjectField and ArrayField
knilink Feb 20, 2017
b8f07d5
Update performance test
knilink Feb 21, 2017
fb63caf
Fixed failing lint
knilink Feb 21, 2017
de766c2
Clean up.
knilink Feb 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 30 additions & 52 deletions src/components/fields/ArrayField.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ import {
retrieveSchema,
toIdSchema,
shouldRender,
getDefaultRegistry,
setState
getDefaultRegistry
} from "../../utils";
Copy link
Copy Markdown
Collaborator

@n1k0 n1k0 Feb 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should check that this is still needed elsewhere. If not, we should remove that helper function, which is now obsolete.


function ArrayFieldTitle({TitleField, idSchema, title, required}) {
Expand Down Expand Up @@ -151,6 +150,7 @@ function DefaultNormalArrayFieldTemplate(props) {
class ArrayField extends Component {
static defaultProps = {
uiSchema: {},
formData: [],
idSchema: {},
registry: getDefaultRegistry(),
required: false,
Expand All @@ -159,23 +159,6 @@ class ArrayField extends Component {
autofocus: false,
};

constructor(props) {
super(props);
this.state = this.getStateFromProps(props);
}

componentWillReceiveProps(nextProps) {
this.setState(this.getStateFromProps(nextProps));
}

getStateFromProps(props) {
const formData = Array.isArray(props.formData) ? props.formData : null;
const {definitions} = this.props.registry;
return {
items: getDefaultFormState(props.schema, formData, definitions) || []
};
}

shouldComponentUpdate(nextProps, nextState) {
return shouldRender(this, nextProps, nextState);
}
Expand All @@ -189,36 +172,29 @@ class ArrayField extends Component {
return itemsSchema.type === "string" && itemsSchema.minLength > 0;
}

asyncSetState(state, options={validate: false}) {
setState(this, state, () => {
this.props.onChange(this.state.items, options);
});
}

onAddClick = (event) => {
event.preventDefault();
const {items} = this.state;
const {schema, registry} = this.props;
const {schema, registry, formData} = this.props;
const {definitions} = registry;
let itemSchema = schema.items;
if (isFixedItems(schema) && allowAdditionalItems(schema)) {
itemSchema = schema.additionalItems;
}
this.asyncSetState({
items: items.concat([
getDefaultFormState(itemSchema, undefined, definitions)
])
});
this.props.onChange([
...formData,
getDefaultFormState(itemSchema, undefined, definitions)
Copy link
Copy Markdown
Collaborator

@n1k0 n1k0 Feb 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be worth renaming utils.getDefaultFormState which now sounds a little confusing. How about something like initDefaults?

Copy link
Copy Markdown
Contributor Author

@knilink knilink Feb 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether it is appropriate to do it at the moment because getDefaultFormState is used by the root Form component as well.

], {validate: false});
};

onDropIndexClick = (index) => {
return (event) => {
if (event) {
event.preventDefault();
}
this.asyncSetState({
items: this.state.items.filter((_, i) => i !== index)
}, {validate: true}); // refs #195
this.props.onChange(
this.props.formData.filter((_, i) => i !== index),
{validate: true} // refs #195
);
};
};

Expand All @@ -228,33 +204,35 @@ class ArrayField extends Component {
event.preventDefault();
event.target.blur();
}
const {items} = this.state;
this.asyncSetState({
items: items.map((item, i) => {
const items = this.props.formData;
Copy link
Copy Markdown
Collaborator

@n1k0 n1k0 Feb 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this can alternatively be written as const {formData: items} = this.props;, which I personally like a lot. But here we could probably just use formData as it makes sense that they're a list of items in an ArrayField.

this.props.onChange(
items.map((item, i) => {
if (i === newIndex) {
return items[index];
} else if (i === index) {
return items[newIndex];
} else {
return item;
}
})
}, {validate: true});
}),
{validate: true}
);
};
};

onChangeForIndex = (index) => {
return (value) => {
this.asyncSetState({
items: this.state.items.map((item, i) => {
this.props.onChange(
this.props.formData.map((item, i) => {
return index === i ? value : item;
})
});
}),
{validate: false}
);
};
};

onSelectChange = (value) => {
this.asyncSetState({items: value});
this.props.onChange(value, {validate: false});
};

render() {
Expand All @@ -275,6 +253,7 @@ class ArrayField extends Component {
const {
schema,
uiSchema,
formData,
errorSchema,
idSchema,
name,
Expand All @@ -287,26 +266,25 @@ class ArrayField extends Component {
onBlur
} = this.props;
const title = (schema.title === undefined) ? name : schema.title;
const {items = []} = this.state;
const {ArrayFieldTemplate, definitions, fields} = registry;
const {TitleField, DescriptionField} = fields;
const itemsSchema = retrieveSchema(schema.items, definitions);
const {addable=true} = getUiOptions(uiSchema);

const arrayProps = {
canAdd: addable,
items: items.map((item, index) => {
items: formData.map((item, index) => {
const itemErrorSchema = errorSchema ? errorSchema[index] : undefined;
const itemIdPrefix = idSchema.$id + "_" + index;
const itemIdSchema = toIdSchema(itemsSchema, itemIdPrefix, definitions);
return this.renderArrayFieldItem({
index,
canMoveUp: index > 0,
canMoveDown: index < items.length - 1,
canMoveDown: index < formData.length - 1,
itemSchema: itemsSchema,
itemIdSchema,
itemErrorSchema,
itemData: items[index],
itemData: formData[index],
itemUiSchema: uiSchema.items,
autofocus: autofocus && index === 0,
onBlur
Expand All @@ -332,7 +310,7 @@ class ArrayField extends Component {

renderMultiSelect() {
const {schema, idSchema, uiSchema, disabled, readonly, autofocus, onBlur} = this.props;
const {items} = this.state;
const items = this.props.formData;
const {widgets, definitions, formContext} = this.props.registry;
const itemsSchema = retrieveSchema(schema.items, definitions);
const enumOptions = optionsList(itemsSchema);
Expand All @@ -357,7 +335,7 @@ class ArrayField extends Component {
renderFiles() {
const {schema, uiSchema, idSchema, name, disabled, readonly, autofocus, onBlur} = this.props;
const title = schema.title || name;
const {items} = this.state;
const items = this.props.formData;
const {widgets, formContext} = this.props.registry;
const {widget="files", ...options} = getUiOptions(uiSchema);
const Widget = getWidget(schema, widget, widgets);
Expand Down Expand Up @@ -393,7 +371,7 @@ class ArrayField extends Component {
onBlur
} = this.props;
const title = schema.title || name;
let {items} = this.state;
let items = this.props.formData;
const {ArrayFieldTemplate, definitions, fields} = registry;
const {TitleField} = fields;
const itemSchemas = schema.items.map(item =>
Expand Down
58 changes: 6 additions & 52 deletions src/components/fields/ObjectField.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,35 +1,16 @@
import React, {Component, PropTypes} from "react";

import {deepEquals} from "../../utils";


import {
getDefaultFormState,
orderProperties,
retrieveSchema,
shouldRender,
getDefaultRegistry,
setState
getDefaultRegistry
} from "../../utils";


function objectKeysHaveChanged(formData, state) {
// for performance, first check for lengths
const newKeys = Object.keys(formData);
const oldKeys = Object.keys(state);
if (newKeys.length < oldKeys.length) {
return true;
}
// deep check on sorted keys
if (!deepEquals(newKeys.sort(), oldKeys.sort())) {
return true;
}
return false;
}

class ObjectField extends Component {
static defaultProps = {
uiSchema: {},
formData: {},
errorSchema: {},
idSchema: {},
registry: getDefaultRegistry(),
Expand All @@ -38,29 +19,6 @@ class ObjectField extends Component {
readonly: false,
}

constructor(props) {
super(props);
this.state = this.getStateFromProps(props);
}

componentWillReceiveProps(nextProps) {
const state = this.getStateFromProps(nextProps);
const {formData} = nextProps;
if (formData && objectKeysHaveChanged(formData, this.state)) {
// We *need* to replace state entirely here has we have received formData
// holding different keys (so with some removed).
this.state = state;
this.forceUpdate();
} else {
this.setState(state);
}
}

getStateFromProps(props) {
const {schema, formData, registry} = props;
return getDefaultFormState(schema, formData, registry.definitions) || {};
}

shouldComponentUpdate(nextProps, nextState) {
return shouldRender(this, nextProps, nextState);
}
Expand All @@ -71,21 +29,17 @@ class ObjectField extends Component {
schema.required.indexOf(name) !== -1;
}

asyncSetState(state, options={validate: false}) {
setState(this, state, () => {
this.props.onChange(this.state, options);
});
}

onPropertyChange = (name) => {
return (value, options) => {
this.asyncSetState({[name]: value}, options);
const newFormData = {...this.props.formData, [name]: value};
this.props.onChange(newFormData, options);
};
};

render() {
const {
uiSchema,
formData,
errorSchema,
idSchema,
name,
Expand Down Expand Up @@ -135,7 +89,7 @@ class ObjectField extends Component {
uiSchema={uiSchema[name]}
errorSchema={errorSchema[name]}
idSchema={idSchema[name]}
formData={this.state[name]}
formData={formData[name]}
onChange={this.onPropertyChange(name)}
onBlur={onBlur}
registry={this.props.registry}
Expand Down
Loading