Fixed field-level validation bug (#32)

* Fixed field-level validation bug

* Improved travis config

Author: erikras

.travis.yml

@@ -11,8 +11,6 @@ script:
   - npm start validate
 after_success:
   - npx codecov
-  - npm install --global semantic-release
-  # - semantic-release pre && npm publish && semantic-release post
 branches:
   only:
     - master

package-lock.json

@@ -25,46 +25,48 @@
   },
   "homepage": "https://github.com/final-form/react-final-form-arrays#readme",
   "devDependencies": {
-    "@types/react": "^16.0.38",
-    "babel-eslint": "^8.2.2",
-    "babel-jest": "^22.4.1",
+    "@types/react": "^16.3.16",
+    "babel-eslint": "^8.2.3",
+    "babel-jest": "^23.0.1",
     "babel-plugin-external-helpers": "^6.22.0",
     "babel-plugin-transform-flow-strip-types": "^6.22.0",
-    "babel-preset-env": "^1.6.1",
+    "babel-preset-env": "^1.7.0",
     "babel-preset-react": "^6.24.1",
     "babel-preset-stage-2": "^6.24.1",
-    "bundlesize": "^0.16.0",
+    "bundlesize": "^0.17.0",
     "doctoc": "^1.3.1",
-    "eslint": "^4.18.1",
+    "eslint": "^4.19.1",
     "eslint-config-react-app": "^2.1.0",
-    "eslint-plugin-babel": "^4.1.2",
-    "eslint-plugin-flowtype": "^2.46.1",
-    "eslint-plugin-import": "^2.9.0",
+    "eslint-plugin-babel": "^5.1.0",
+    "eslint-plugin-flowtype": "^2.49.3",
+    "eslint-plugin-import": "^2.12.0",
     "eslint-plugin-jsx-a11y": "^6.0.2",
-    "eslint-plugin-react": "^7.7.0",
-    "final-form": "^4.2.1",
+    "eslint-plugin-react": "^7.9.1",
+    "final-form": "^4.7.3",
     "final-form-arrays": "^1.0.4",
-    "flow-bin": "^0.66.0",
+    "flow-bin": "^0.73.0",
+    "glow": "^1.2.2",
     "husky": "^0.14.3",
-    "jest": "^22.4.2",
-    "lint-staged": "^7.0.0",
-    "nps": "^5.7.1",
+    "jest": "^23.1.0",
+    "jest-watch-typeahead": "^0.1.0",
+    "lint-staged": "^7.1.3",
+    "nps": "^5.9.0",
     "nps-utils": "^1.5.0",
-    "prettier": "^1.10.2",
+    "prettier": "^1.12.0",
     "prettier-eslint-cli": "^4.7.1",
-    "prop-types": "^15.6.0",
+    "prop-types": "^15.6.1",
     "raf": "^3.4.0",
-    "react": "^16.1.0",
-    "react-dom": "^16.1.0",
-    "react-final-form": "^3.1.0",
-    "rollup": "^0.56.2",
-    "rollup-plugin-babel": "^3.0.2",
-    "rollup-plugin-commonjs": "^8.3.0",
+    "react": "^16.4.0",
+    "react-dom": "^16.4.0",
+    "react-final-form": "^3.5.1",
+    "rollup": "^0.60.0",
+    "rollup-plugin-babel": "^3.0.4",
+    "rollup-plugin-commonjs": "^9.1.3",
     "rollup-plugin-flow": "^1.1.1",
-    "rollup-plugin-node-resolve": "^3.0.3",
+    "rollup-plugin-node-resolve": "^3.3.0",
     "rollup-plugin-replace": "^2.0.0",
-    "rollup-plugin-uglify": "^3.0.0",
-    "typescript": "^2.7.2"
+    "rollup-plugin-uglify": "^4.0.0",
+    "typescript": "^2.9.1"
   },
   "peerDependencies": {
     "final-form": ">=4.0.0",
@@ -73,7 +75,10 @@
     "react": "^15.3.0 || ^16.0.0-0"
   },
   "jest": {
-    "setupFiles": ["raf/polyfill"]
+    "watchPlugins": [
+      "jest-watch-typeahead/filename",
+      "jest-watch-typeahead/testname"
+    ]
   },
   "lint-staged": {
     "*.{js*,ts*,json,md,css}": ["prettier --write", "git add"]
@@ -91,5 +96,9 @@
       "path": "dist/react-final-form-arrays.cjs.js",
       "threshold": "3kB"
     }
-  ]
+  ],
+  "dependencies": {
+    "npm-check": "^5.7.1",
+    "react-lifecycles-compat": "^3.0.4"
+  }
 }

package.json

@@ -2,7 +2,7 @@ import resolve from 'rollup-plugin-node-resolve'
 import babel from 'rollup-plugin-babel'
 import flow from 'rollup-plugin-flow'
 import commonjs from 'rollup-plugin-commonjs'
-import uglify from 'rollup-plugin-uglify'
+import { uglify } from 'rollup-plugin-uglify'
 import replace from 'rollup-plugin-replace'
 
 const minify = process.env.MINIFY
@@ -34,7 +34,6 @@ if (es) {
 
 // eslint-disable-next-line no-nested-ternary
 export default {
-  name: 'react-final-form-arrays',
   input: 'src/index.js',
   output: Object.assign(
     {

rollup.config.js

@@ -1,26 +1,37 @@
 // @flow
 import * as React from 'react'
-import warning from './warning'
+import { polyfill } from 'react-lifecycles-compat'
 import PropTypes from 'prop-types'
-import { fieldSubscriptionItems, version as ffVersion } from 'final-form'
+import {
+  fieldSubscriptionItems,
+  version as ffVersion,
+  ARRAY_ERROR
+} from 'final-form'
+import { version as rffVersion } from 'react-final-form'
 import type { ReactContext } from 'react-final-form'
 import diffSubscription from './diffSubscription'
-import type { FieldSubscription, FieldState } from 'final-form'
+import type { FieldSubscription, FieldState, FieldValidator } from 'final-form'
 import type { Mutators } from 'final-form-arrays'
 import type { FieldArrayProps as Props } from './types'
 import renderComponent from './renderComponent'
-import { version } from './index'
+export const version = '1.0.4'
+
+const versions = {
+  'final-form': ffVersion,
+  'react-final-form': rffVersion,
+  'react-final-form-arrays': version
+}
 
 const all: FieldSubscription = fieldSubscriptionItems.reduce((result, key) => {
   result[key] = true
   return result
 }, {})
 
 type State = {
-  state: FieldState
+  state: ?FieldState
 }
 
-export default class FieldArray extends React.PureComponent<Props, State> {
+class FieldArray extends React.PureComponent<Props, State> {
   context: ReactContext
   props: Props
   state: State
@@ -33,10 +44,12 @@ export default class FieldArray extends React.PureComponent<Props, State> {
   constructor(props: Props, context: ReactContext) {
     super(props, context)
     let initialState
-    warning(
-      context.reactFinalForm,
-      'FieldArray must be used inside of a ReactFinalForm component'
-    )
+    // istanbul ignore next
+    if (process.env.NODE_ENV !== 'production' && !context.reactFinalForm) {
+      console.error(
+        'Warning: FieldArray must be used inside of a ReactFinalForm component'
+      )
+    }
     const { reactFinalForm } = this.context
     if (reactFinalForm) {
       // avoid error, warning will alert developer to their mistake
@@ -48,7 +61,7 @@ export default class FieldArray extends React.PureComponent<Props, State> {
         }
       })
     }
-    this.state = { state: initialState || {} }
+    this.state = { state: initialState }
     this.bindMutators(props)
     this.mounted = false
   }
@@ -62,20 +75,36 @@ export default class FieldArray extends React.PureComponent<Props, State> {
       listener,
       subscription ? { ...subscription, length: true } : all,
       {
-        getValidator: () => this.props.validate
+        getValidator: () => this.validate
       }
     )
   }
 
+  validate: FieldValidator = (...args) => {
+    const { validate } = this.props
+    if (!validate) return undefined
+    const error = validate(...args)
+    if (!error || Array.isArray(error)) {
+      return error
+    } else {
+      const arrayError = []
+      // gross, but we have to set a string key on the array
+      ;((arrayError: any): Object)[ARRAY_ERROR] = error
+      return arrayError
+    }
+  }
+
   bindMutators = ({ name }: Props) => {
     const { reactFinalForm } = this.context
     if (reactFinalForm) {
       const { mutators } = reactFinalForm
       const hasMutators = !!(mutators && mutators.push && mutators.pop)
-      warning(
-        hasMutators,
-        'Array mutators not found. You need to provide the mutators from final-form-arrays to your form'
-      )
+      // istanbul ignore next
+      if (process.env.NODE_ENV !== 'production' && !hasMutators) {
+        console.error(
+          'Warning: Array mutators not found. You need to provide the mutators from final-form-arrays to your form'
+        )
+      }
       if (hasMutators) {
         this.mutators = Object.keys(mutators).reduce((result, key) => {
           result[key] = (...args) => mutators[key](name, ...args)
@@ -95,23 +124,27 @@ export default class FieldArray extends React.PureComponent<Props, State> {
 
   forEach = (iterator: (name: string, index: number) => void): void => {
     const { name } = this.props
-    const { length } = this.state.state
+    // required || for Flow, but results in uncovered line in Jest/Istanbul
+    // istanbul ignore next
+    const length = this.state.state ? this.state.state.length || 0 : 0
     for (let i = 0; i < length; i++) {
       iterator(`${name}[${i}]`, i)
     }
   }
 
   map = (iterator: (name: string, index: number) => any): any[] => {
     const { name } = this.props
-    const { length } = this.state.state
+    // required || for Flow, but results in uncovered line in Jest/Istanbul
+    // istanbul ignore next
+    const length = this.state.state ? this.state.state.length || 0 : 0
     const results: any[] = []
     for (let i = 0; i < length; i++) {
       results.push(iterator(`${name}[${i}]`, i))
     }
     return results
   }
 
-  componentWillReceiveProps(nextProps: Props) {
+  UNSAFE_componentWillReceiveProps(nextProps: Props) {
     const { name, subscription } = nextProps
     if (
       this.props.name !== name ||
@@ -158,7 +191,8 @@ export default class FieldArray extends React.PureComponent<Props, State> {
       valid,
       visited,
       ...fieldStateFunctions
-    } = this.state.state
+    } =
+      this.state.state || {}
     const meta = {
       active,
       dirty,
@@ -188,7 +222,8 @@ export default class FieldArray extends React.PureComponent<Props, State> {
           ...fieldState
         },
         meta,
-        ...rest
+        ...rest,
+        __versions: versions
       },
       `FieldArray(${name})`
     )
@@ -198,3 +233,7 @@ export default class FieldArray extends React.PureComponent<Props, State> {
 FieldArray.contextTypes = {
   reactFinalForm: PropTypes.object
 }
+
+polyfill(FieldArray)
+
+export default FieldArray

src/FieldArray.js

@@ -9,27 +9,48 @@ const sleep = ms => new Promise(resolve => setTimeout(resolve, ms))
 
 describe('FieldArray', () => {
   it('should warn error if not used inside a form', () => {
+    const spy = jest.spyOn(global.console, 'error').mockImplementation(() => {})
     TestUtils.renderIntoDocument(
       <FieldArray name="foo" render={() => <div />} />
     )
+    expect(spy).toHaveBeenCalled()
+    expect(spy).toHaveBeenCalledTimes(1)
+    expect(spy).toHaveBeenCalledWith(
+      'Warning: FieldArray must be used inside of a ReactFinalForm component'
+    )
+    spy.mockRestore()
   })
 
   it('should warn if no render strategy is provided', () => {
+    const spy = jest.spyOn(global.console, 'error').mockImplementation(() => {})
     TestUtils.renderIntoDocument(
       <Form
         onSubmit={onSubmitMock}
         mutators={arrayMutators}
         render={() => <FieldArray name="foo" />}
       />
     )
+    expect(spy).toHaveBeenCalled()
+    expect(spy).toHaveBeenCalledTimes(1)
+    expect(spy).toHaveBeenCalledWith(
+      'Warning: Must specify either a render prop, a render function as children, or a component prop to FieldArray(foo)'
+    )
+    spy.mockRestore()
   })
 
   it('should warn if no array mutators provided', () => {
+    const spy = jest.spyOn(global.console, 'error').mockImplementation(() => {})
     TestUtils.renderIntoDocument(
       <Form onSubmit={onSubmitMock}>
         {() => <FieldArray name="foo" render={() => <div />} />}
       </Form>
     )
+    expect(spy).toHaveBeenCalled()
+    expect(spy).toHaveBeenCalledTimes(1)
+    expect(spy).toHaveBeenCalledWith(
+      'Warning: Array mutators not found. You need to provide the mutators from final-form-arrays to your form'
+    )
+    spy.mockRestore()
   })
 
   it('should render with a render component', () => {
@@ -53,6 +74,7 @@ describe('FieldArray', () => {
           <Form
             onSubmit={onSubmitMock}
             mutators={arrayMutators}
+            subscription={{}}
             initialValues={{ dogs: ['Odie'], cats: ['Garfield'] }}
           >
             {() => (
@@ -129,7 +151,7 @@ describe('FieldArray', () => {
       <Form onSubmit={onSubmitMock} render={render} />
     )
     expect(render).toHaveBeenCalled()
-    expect(render).toHaveBeenCalledTimes(2)
+    expect(render).toHaveBeenCalledTimes(1)
     expect(renderArray).toHaveBeenCalled()
     expect(renderArray).toHaveBeenCalledTimes(1)
   })
@@ -152,13 +174,13 @@ describe('FieldArray', () => {
       />
     )
     expect(render).toHaveBeenCalled()
-    expect(render).toHaveBeenCalledTimes(2)
+    expect(render).toHaveBeenCalledTimes(1)
     expect(renderArray).toHaveBeenCalled()
-    expect(renderArray).toHaveBeenCalledTimes(2)
-    expect(renderArray.mock.calls[1][0].meta.dirty).not.toBeUndefined()
-    expect(renderArray.mock.calls[1][0].meta.dirty).toBe(false)
-    expect(renderArray.mock.calls[1][0].fields.length).not.toBeUndefined()
-    expect(renderArray.mock.calls[1][0].fields.length).toBe(2)
+    expect(renderArray).toHaveBeenCalledTimes(1)
+    expect(renderArray.mock.calls[0][0].meta.dirty).not.toBeUndefined()
+    expect(renderArray.mock.calls[0][0].meta.dirty).toBe(false)
+    expect(renderArray.mock.calls[0][0].fields.length).not.toBeUndefined()
+    expect(renderArray.mock.calls[0][0].fields.length).toBe(2)
   })
 
   it('should unsubscribe on unmount', () => {
@@ -213,7 +235,7 @@ describe('FieldArray', () => {
       />
     )
     expect(render).toHaveBeenCalled()
-    expect(render).toHaveBeenCalledTimes(2)
+    expect(render).toHaveBeenCalledTimes(1)
     expect(renderArray).toHaveBeenCalled()
     expect(renderArray).toHaveBeenCalledTimes(1)
     expect(renderArray.mock.calls[0][0].meta.valid).toBe(true)
@@ -246,7 +268,7 @@ describe('FieldArray', () => {
       />
     )
     expect(render).toHaveBeenCalled()
-    expect(render).toHaveBeenCalledTimes(2)
+    expect(render).toHaveBeenCalledTimes(1)
     expect(renderArray).toHaveBeenCalled()
     expect(renderArray).toHaveBeenCalledTimes(1)
 
@@ -278,7 +300,7 @@ describe('FieldArray', () => {
       />
     )
     expect(render).toHaveBeenCalled()
-    expect(render).toHaveBeenCalledTimes(2)
+    expect(render).toHaveBeenCalledTimes(1)
     expect(renderArray).toHaveBeenCalled()
     expect(renderArray).toHaveBeenCalledTimes(1)
 
@@ -299,9 +321,10 @@ describe('FieldArray', () => {
       <Form
         onSubmit={onSubmitMock}
         mutators={arrayMutators}
+        subscription={{}}
         render={() => (
           <form>
-            <FieldArray name="foo">
+            <FieldArray name="foo" subscription={{}}>
               {({ fields }) => (
                 <div>
                   {fields.map(name => (
@@ -336,10 +359,10 @@ describe('FieldArray', () => {
     TestUtils.Simulate.click(button)
     await sleep(2)
 
-    // notice it doesn't NEED to be called for foo[0] because that field hasn't changed!
-    expect(renderInput).toHaveBeenCalledTimes(3)
-    expect(renderInput.mock.calls[2][0].input.name).toBe('foo[1]')
-    expect(renderInput.mock.calls[2][0].input.value).toBe('')
+    // it must rerender foo[0] because the whole array is rerendered due to the change of length
+    expect(renderInput).toHaveBeenCalledTimes(4)
+    expect(renderInput.mock.calls[3][0].input.name).toBe('foo[1]')
+    expect(renderInput.mock.calls[3][0].input.value).toBe('')
   })
 
   it('should allow Fields to be rendered for complex objects', async () => {
@@ -349,9 +372,10 @@ describe('FieldArray', () => {
       <Form
         onSubmit={onSubmitMock}
         mutators={arrayMutators}
+        subscription={{}}
         render={() => (
           <form>
-            <FieldArray name="foo">
+            <FieldArray name="foo" subscription={{}}>
               {({ fields }) => (
                 <div>
                   {fields.map(name => (
@@ -408,17 +432,17 @@ describe('FieldArray', () => {
     TestUtils.Simulate.click(button)
     await sleep(2)
 
-    // notice it doesn't NEED to be called for foo[0] because that field hasn't changed!
-    expect(renderFirstNameInput).toHaveBeenCalledTimes(3)
-    expect(renderFirstNameInput.mock.calls[2][0].input.name).toBe(
+    // it must rerender foo[0] inputs because the whole array is rerendered due to the change of length
+    expect(renderFirstNameInput).toHaveBeenCalledTimes(4)
+    expect(renderFirstNameInput.mock.calls[3][0].input.name).toBe(
       'foo[1].firstName'
     )
-    expect(renderFirstNameInput.mock.calls[2][0].input.value).toBe('')
-    expect(renderLastNameInput).toHaveBeenCalledTimes(2)
-    expect(renderLastNameInput.mock.calls[1][0].input.name).toBe(
+    expect(renderFirstNameInput.mock.calls[3][0].input.value).toBe('')
+    expect(renderLastNameInput).toHaveBeenCalledTimes(3)
+    expect(renderLastNameInput.mock.calls[2][0].input.name).toBe(
       'foo[1].lastName'
     )
-    expect(renderLastNameInput.mock.calls[1][0].input.value).toBe('')
+    expect(renderLastNameInput.mock.calls[2][0].input.value).toBe('')
   })
 
   it('should not warn if updating state after unmounting', async () => {

src/FieldArray.test.js

@@ -1,4 +1,2 @@
 // @flow
-export { default as FieldArray } from './FieldArray'
-
-export const version = '1.0.3'
+export { default as FieldArray, version } from './FieldArray'

src/index.js

@@ -1,6 +1,5 @@
 // @flow
 import * as React from 'react'
-import warning from './warning'
 import type { RenderableProps } from './types'
 
 // shared logic between components that use either render prop,
@@ -16,14 +15,14 @@ export default function renderComponent<T>(
   if (render) {
     return render({ ...rest, children }) // inject children back in
   }
+  // istanbul ignore next
   if (typeof children !== 'function') {
-    warning(
-      false,
-      `Must specify either a render prop, a render function as children, or a component prop to ${
-        name
-      }`
-    )
-    return null // warning will alert developer to their mistake
+    if (process.env.NODE_ENV !== 'production') {
+      console.error(
+        `Warning: Must specify either a render prop, a render function as children, or a component prop to ${name}`
+      )
+      return null // warning will alert developer to their mistake
+    }
   }
   return children(rest)
 }

src/renderComponent.js

@@ -34,13 +34,14 @@ export type FieldArrayRenderProps = {
 }
 
 export type RenderableProps<T> = $Shape<{
-  children: ((props: T) => React.Node) | React.Node,
+  children: (props: T) => React.Node,
   component: React.ComponentType<*>,
   render: (props: T) => React.Node
 }>
 
 export type FieldArrayProps = {
   name: string,
   subscription?: FieldSubscription,
+  isEqual?: (any, any) => boolean,
   validate?: (value: ?(any[]), allValues: Object) => ?any
 } & RenderableProps<FieldArrayRenderProps>