Repository: mozilla-services/react-jsonschema-form
Pull request #490 (2017-02-25 07:17:30)

Centralized handling shouldComponentUpdate into SchemaField
### Reasons for making this change

When one property is updated, the rest of the properties will be unnecessarily rendered.
This will cause a significant performance issue in a large object schema such as the demo of 'Large' in the playground.

### Checklist

* [ ] **I'm updating documentation**
  - [ ] I've checked the rendering of the Markdown text I've added
  - [ ] If I'm adding a new section, I've updated the Table of Content
* [x] **I'm adding or updating code**
  - [x] I've added and/or updated tests
  - [ ] I've updated docs if needed
* [ ] **I'm adding a new feature**
  - [ ] I've updated the playground with an example use of the feature


n1k0: Thank you :+1: 

Patch:
diff --git a/src/components/fields/ArrayField.js b/src/components/fields/ArrayField.js
index 5344b2f80..4dff3f486 100644
--- a/src/components/fields/ArrayField.js
+++ b/src/components/fields/ArrayField.js
@@ -11,7 +11,6 @@ import {
   optionsList,
   retrieveSchema,
   toIdSchema,
-  shouldRender,
   getDefaultRegistry
 } from "../../utils";
 
@@ -159,10 +158,6 @@ class ArrayField extends Component {
     autofocus: false,
   };
 
-  shouldComponentUpdate(nextProps, nextState) {
-    return shouldRender(this, nextProps, nextState);
-  }
-
   get itemTitle() {
     const {schema} = this.props;
     return schema.items.title || schema.items.description || "Item";
diff --git a/src/components/fields/ObjectField.js b/src/components/fields/ObjectField.js
index 97bb12966..4b048c806 100644
--- a/src/components/fields/ObjectField.js
+++ b/src/components/fields/ObjectField.js
@@ -3,7 +3,6 @@ import React, {Component, PropTypes} from "react";
 import {
   orderProperties,
   retrieveSchema,
-  shouldRender,
   getDefaultRegistry
 } from "../../utils";
 
@@ -19,10 +18,6 @@ class ObjectField extends Component {
     readonly: false,
   }
 
-  shouldComponentUpdate(nextProps, nextState) {
-    return shouldRender(this, nextProps, nextState);
-  }
-
   isRequired(name) {
     const schema = this.props.schema;
     return Array.isArray(schema.required) &&
diff --git a/src/components/fields/SchemaField.js b/src/components/fields/SchemaField.js
index 409885611..47e6422a8 100644
--- a/src/components/fields/SchemaField.js
+++ b/src/components/fields/SchemaField.js
@@ -4,7 +4,8 @@ import {
   isMultiSelect,
   retrieveSchema,
   getDefaultRegistry,
-  isFilesArray
+  isFilesArray,
+  deepEquals
 } from "../../utils";
 import UnsupportedField from "./UnsupportedField";
 
@@ -128,7 +129,7 @@ DefaultTemplate.defaultProps = {
   displayLabel: true,
 };
 
-function SchemaField(props) {
+function SchemaFieldRender(props) {
   const {uiSchema, errorSchema, idSchema, name, required, registry} = props;
   const {definitions, fields, formContext, FieldTemplate = DefaultTemplate} = registry;
   const schema = retrieveSchema(props.schema, definitions);
@@ -211,6 +212,21 @@ function SchemaField(props) {
   return <FieldTemplate {...fieldProps}>{field}</FieldTemplate>;
 }
 
+class SchemaField extends React.Component {
+  shouldComponentUpdate(nextProps, nextState) {
+    // if schemas are equal idSchemas will be equal as well,
+    // so it is not necessary to compare
+    return !deepEquals(
+      {...this.props, idSchema: undefined},
+      {...nextProps,  idSchema: undefined}
+    );
+  }
+
+  render() {
+    return SchemaFieldRender(this.props);
+  }
+}
+
 SchemaField.defaultProps = {
   uiSchema: {},
   errorSchema: {},


Test patch:
diff --git a/test/performance_test.js b/test/performance_test.js
index bb4f5d827..505d8ba9c 100644
--- a/test/performance_test.js
+++ b/test/performance_test.js
@@ -1,9 +1,8 @@
 import sinon from "sinon";
 import React from "react";
-
+import {scryRenderedComponentsWithType} from "react-addons-test-utils";
 import {getDefaultRegistry} from "../src/utils";
-import ArrayField from "../src/components/fields/ArrayField";
-import ObjectField from "../src/components/fields/ObjectField";
+import SchemaField from "../src/components/fields/SchemaField";
 import {
   createComponent,
   createFormComponent,
@@ -47,51 +46,61 @@ describe("Rendering performance optimizations", () => {
 
       sinon.assert.notCalled(comp.render);
     });
-  });
 
-  describe("ArrayField", () => {
-    const onChange = () => {};
-    const onBlur = () => {};
-    const schema = {type: "array", items: {type: "string"}};
-    const uiSchema = {};
-    const registry = getDefaultRegistry();
+    it("should only render changed object properties", () => {
+      const schema = {
+        type: "object",
+        properties: {
+          const: {type: "string"},
+          var: {type: "string"}
+        }
+      };
 
-    it("should not render if next props are equivalent", () => {
-      const props = {
-        registry,
+      const {comp} = createFormComponent({
         schema,
-        uiSchema,
-        onChange,
-        onBlur
-      };
+        formData: {const: "0", var: "0"}
+      });
 
-      const {comp} = createComponent(ArrayField, props);
-      sandbox.stub(comp, "render").returns(<div/>);
+      const fields = scryRenderedComponentsWithType(comp, SchemaField)
+        .reduce( (fields, fieldComp) => {
+          sandbox.stub(fieldComp, "render").returns(<div/>);
+          fields[fieldComp.props.idSchema.$id] = fieldComp;
+          return fields;
+        });
 
-      setProps(comp, props);
+      setProps(comp, {schema, formData: {const: "0", var: "1"}});
 
-      sinon.assert.notCalled(comp.render);
+      sinon.assert.notCalled(fields.root_const.render);
+      sinon.assert.calledOnce(fields.root_var.render);
     });
 
-    it("should not render if next formData are equivalent", () => {
-      const props = {
-        registry,
-        schema,
-        formData: ["a", "b"],
-        onChange,
-        onBlur
+    it("should only render changed array items", () => {
+      const schema = {
+        type: "array",
+        items: {type: "string"}
       };
 
-      const {comp} = createComponent(ArrayField, props);
-      sandbox.stub(comp, "render").returns(<div/>);
+      const {comp} = createFormComponent({
+        schema,
+        formData: ["const", "var0"]
+      });
 
-      setProps(comp, {...props, formData: ["a", "b"]});
+      const fields = scryRenderedComponentsWithType(comp, SchemaField)
+        .reduce( (fields, fieldComp) => {
+          sandbox.stub(fieldComp, "render").returns(<div/>);
+          fields[fieldComp.props.idSchema.$id] = fieldComp;
+          return fields;
+        });
+
+      setProps(comp, {schema, formData: ["const", "var1"]});
+
+      sinon.assert.notCalled(fields.root_0.render);
+      sinon.assert.calledOnce(fields.root_1.render);
 
-      sinon.assert.notCalled(comp.render);
     });
   });
 
-  describe("ObjectField", () => {
+  describe("SchemaField", () => {
     const onChange = () => {};
     const onBlur = () => {};
     const registry = getDefaultRegistry();
@@ -114,7 +123,7 @@ describe("Rendering performance optimizations", () => {
         onBlur
       };
 
-      const {comp} = createComponent(ObjectField, props);
+      const {comp} = createComponent(SchemaField, props);
       sandbox.stub(comp, "render").returns(<div/>);
 
       setProps(comp, props);
@@ -132,7 +141,7 @@ describe("Rendering performance optimizations", () => {
         onBlur
       };
 
-      const {comp} = createComponent(ObjectField, props);
+      const {comp} = createComponent(SchemaField, props);
       sandbox.stub(comp, "render").returns(<div/>);
 
       setProps(comp, {...props, formData: {foo: "blah"}});