From 2cefad87dfcc38728f5164f9462f62c925a7c646 Mon Sep 17 00:00:00 2001 From: Artem Kuzko Date: Tue, 21 Mar 2017 00:12:10 +0200 Subject: [PATCH] performance optimization and support improvements - refactor form attributes update routines to get rid of `cloneDeep` calls: deep cloning is unefficient since it potentially most of cloned data should not be cloned. instead, implement `update` and `updated` helper functions that carefully clone only path-related collections. - refactor code to depend only on 'lodash.get' and 'lodash.set' packages - extend form from `Component` if `PureComponent` is unavailable. this allows to support older versions of react - corresponding package.json updates - minor update README update --- .eslintrc | 2 +- README.md | 3 ++- package.json | 7 ++++--- src/Form.jsx | 37 +++++++++++++++---------------------- src/utils.js | 40 ++++++++++++++++++++++++++++++++-------- test/utils.test.js | 36 ++++++++++++++++++++++++++++++++---- 6 files changed, 86 insertions(+), 39 deletions(-) diff --git a/.eslintrc b/.eslintrc index 1d4d6e1..4389bc6 100644 --- a/.eslintrc +++ b/.eslintrc @@ -31,6 +31,6 @@ "react/jsx-equals-spacing": [1, "never"], "react/jsx-indent-props": [1, 2], "react/jsx-indent": [1, 2], - "react/jsx-no-duplicate-props": 2, + "react/jsx-no-duplicate-props": 2 } } diff --git a/README.md b/README.md index e1e7360..60db5a7 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,8 @@ Details on how to run it locally are at the end of README. Most of forms developers deal with are quite complicated and encapsulate vast amount of validation and rendering logic. After some basic setup described in the [Wiki](https://github.com/akuzko/react-form-base/wiki) your form may -look like following: +look like following (please note that `$render` is a helper method and it is +possible to use classic `render` method with a slightly more verbose result): ```js class UserForm extends Form { diff --git a/package.json b/package.json index 5da9780..09cf4f6 100644 --- a/package.json +++ b/package.json @@ -50,10 +50,11 @@ "rimraf": "^2.5.4" }, "dependencies": { - "lodash": "^4.16.4" + "lodash.get": "^4.4.2", + "lodash.set": "^4.3.2" }, "peerDependencies": { - "react": ">= 15.0.0", - "react-dom": ">= 15.0.0" + "react": "^0.14.8 || >=15.0.0", + "react-dom": "^0.14.8 || >=15.0.0" } } diff --git a/src/Form.jsx b/src/Form.jsx index 914cae6..21be145 100644 --- a/src/Form.jsx +++ b/src/Form.jsx @@ -1,13 +1,9 @@ -import React, { PropTypes, PureComponent } from 'react'; +import { PropTypes, PureComponent, Component } from 'react'; -import { nameToPath, buildFormValidator } from './utils'; -import isPlainObject from 'lodash/isPlainObject'; -import cloneDeep from 'lodash/cloneDeep'; -import get from 'lodash/get'; -import set from 'lodash/set'; -import noop from 'lodash/noop'; +import { update, buildFormValidator, noop } from './utils'; +import get from 'lodash.get'; -export default class Form extends PureComponent { +export default class Form extends (PureComponent || Component) { static propTypes = { attrs: PropTypes.object.isRequired, onChange: PropTypes.func, @@ -76,11 +72,11 @@ export default class Form extends PureComponent { get(name) { if (name === undefined) return this.props.attrs; - return get(this.props.attrs, nameToPath(name)); + return get(this.props.attrs, name.split('.')); } set(name, value) { - if (isPlainObject(name)) return this._setObject(name); + if (name && (typeof name === 'object') && (name.constructor === Object)) return this._setObject(name); return this._setAttr(name, value); } @@ -88,7 +84,7 @@ export default class Form extends PureComponent { _setObject(obj) { return this._set((attrs, errors) => { for (const name in obj) { - set(attrs, nameToPath(name), obj[name]); + update(attrs, name, obj[name], false); this._updateErrors(errors, name, obj[name]); } }); @@ -96,7 +92,7 @@ export default class Form extends PureComponent { _setAttr(name, value) { return this._set((attrs, errors) => { - set(attrs, nameToPath(name), value); + update(attrs, name, value, false); this._updateErrors(errors, name, value); }); } @@ -113,13 +109,12 @@ export default class Form extends PureComponent { _set(updater) { const { attrs, onChange } = this.props; - const newAttrs = cloneDeep(attrs); - const newErrors = { ...this.getErrors() }; + const nextAttrs = { ...attrs }; + const nextErrors = { ...this.getErrors() }; + updater(nextAttrs, nextErrors); - updater(newAttrs, newErrors); - - this._nextErrors = newErrors; - onChange(newAttrs); + this._nextErrors = nextErrors; + onChange(nextAttrs); } _shouldClearError(name) { @@ -165,10 +160,8 @@ export default class Form extends PureComponent { merge(name, value) { const current = this.get(name) || {}; - const attrs = cloneDeep(this.props.attrs); - set(attrs, name, { ...current, ...value }); - this.props.onChange(attrs); + return this.set(name, { ...current, ...value }); } push(name, value) { @@ -181,7 +174,7 @@ export default class Form extends PureComponent { const ary = this.get(name); return this._set((attrs, errors) => { - set(attrs, nameToPath(name), [...ary.slice(0, i), ...ary.slice(i + 1)]); + update(attrs, name, [...ary.slice(0, i), ...ary.slice(i + 1)]); this._updateErrors(errors, `${name}.${i}`, null); }); } diff --git a/src/utils.js b/src/utils.js index 926d53e..611a9d1 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,5 +1,6 @@ -import isArray from 'lodash/isArray'; -import isPlainObject from 'lodash/isPlainObject'; +import set from 'lodash.set'; + +export function noop(){} export function bindState(component, key = 'form') { return { @@ -8,8 +9,31 @@ export function bindState(component, key = 'form') { }; } -export function nameToPath(name) { - return name.replace(/\.(\d+)(\.)?/g, (_match, i, dot) => `[${i}]` + (dot ? '.' : '')); +export function update(obj, name, value) { + _update(obj, name, value); + + return obj; +} + +export function updated(obj, name, value) { + const current = { ...obj }; + + return update(current, name, value); +} + +function _update(current, name, value) { + const match = name.match(/^([\w\d]+)\.?(.+)?$/); + const { 1: key, 2: rest } = match; + + if (current[key] === undefined) { + return set(current, name.split('.'), value); + } + if (!rest) { + return current[key] = value; + } + + current[key] = Array.isArray(current[key]) ? [...current[key]] : { ...current[key] }; + _update(current[key], rest, value); } function wildcard(name) { @@ -32,10 +56,7 @@ export function buildFormValidator(form) { return null; function callValidator(validator) { - if (isPlainObject(validator)) { - return callObjectValidator(validator); - } - if (isArray(validator)) { + if (Array.isArray(validator)) { return callArrayValidator(validator); } if (typeof validator === 'string') { @@ -44,6 +65,9 @@ export function buildFormValidator(form) { if (typeof validator === 'function') { return validator.call(form, value); } + if (validator && (typeof validator === 'object')) { + return callObjectValidator(validator); + } throw new Error(`unable to use '${validator}' as validator function`); } diff --git a/test/utils.test.js b/test/utils.test.js index 02eecd7..16047c0 100644 --- a/test/utils.test.js +++ b/test/utils.test.js @@ -1,12 +1,40 @@ import React, { Component } from 'react'; import Form from '../src/Form'; -import { nameToPath, bindState } from '../src/utils'; +import { bindState, updated } from '../src/utils'; import { shallow } from 'enzyme'; import expect from 'expect'; -describe('nameToPath', function() { - it('generates a lodash path based on passed name', function() { - expect(nameToPath('foo.1.bar.baz')).toEqual('foo[1].bar.baz'); +describe('updated', function() { + it('carefully sets deeply nested item: deeply nested array', function() { + const obj = { foo: { bar: { baz: [1, 2, 3] } }, bak: { big: 1 } }; + const upd = updated(obj, 'foo.bar.baz.1', 4); + + expect(obj === upd).toBe(false, 'obj should not be updated in place'); + expect(obj.foo === upd.foo).toBe(false, 'obj.foo should not be updated in place'); + expect(obj.foo.bar === upd.foo.bar).toBe(false, 'obj.foo.bar should not be updated in place'); + expect(obj.foo.bar.baz === upd.foo.bar.baz).toBe(false, 'obj.foo.bar.baz should not be updated in place'); + expect(obj.bak === upd.bak).toBe(true, 'obj.bak should not be cloned'); + expect(upd.foo.bar.baz).toMatch([1, 4, 3], 'value under desired name should be updated'); + }); + + it('carefully sets deeply nested item: deeply nested object', function() { + const obj = { foo: { bar: [{ baz: 'baz1' }, { baz: 'baz2' }] }, bak: { big: 1 } }; + const upd = updated(obj, 'foo.bar.1.baz', 'baz3'); + + expect(obj === upd).toBe(false, 'obj should not be updated in place'); + expect(obj.foo === upd.foo).toBe(false, 'obj.foo should not be updated in place'); + expect(obj.foo.bar === upd.foo.bar).toBe(false, 'obj.foo.bar should not be updated in place'); + expect(obj.foo.bar[0] === upd.foo.bar[0]).toBe(true, 'obj.foo.bar items should not be cloned'); + expect(obj.bak === upd.bak).toBe(true, 'obj.bak should not be cloned'); + expect(upd.foo.bar[1]).toMatch({ baz: 'baz3' }, 'value under desired name should be updated'); + }); + + it('carefully sets deeply nested item, path collections are not defined', function() { + const obj = { bak: { big: 1 } }; + const upd = updated(obj, 'foo.bar.baz.1', 4); + + expect(obj.bak === upd.bak).toBe(true, 'obj.bak should not be cloned'); + expect(upd.foo.bar.baz).toMatch([undefined, 4], 'value under desired name should be updated'); }); });