Safe list refs modification

.travis.yml

@@ -1,13 +1,6 @@
 language: node_js
 node_js:
-  - "0.12"
+  - "9"
 before_script:
   - export DISPLAY=:99.0
   - sh -e /etc/init.d/xvfb start
-# TODO: This should be using latest for firefox. It is currently turned off because
-#       of https://github.com/travis-ci/travis-ci/issues/8242 When this issue is
-#       resolved, this should be updated back to latest
-dist: trusty
-sudo: false
-addons:
-  chrome: stable

karma.conf.js

@@ -4,10 +4,7 @@ var path = require('path') // eslint-disable-line
 module.exports = function getConfig(config) {
   config.set({
     browserNoActivityTimeout: 30000,
-    // TODO: This should include "Firefox" for CI. It is currently turned off because
-    //       of https://github.com/travis-ci/travis-ci/issues/8242 When this issue is
-    //       resolved, this should be updated to include firefox
-    browsers: [process.env.CONTINUOUS_INTEGRATION ? 'Chrome' : 'Chrome'],
+    browsers: [process.env.CONTINUOUS_INTEGRATION ? 'Firefox' : 'Chrome'],
     singleRun: true,
     frameworks: ['tap'],
     files: [

package.json

@@ -43,11 +43,11 @@
     "eslint-config-airbnb": "1.0.0",
     "eslint-plugin-react": "3.10.0",
     "isparta-loader": "1.0.0",
-    "karma": "0.13.9",
-    "karma-chrome-launcher": "0.2.0",
-    "karma-coverage": "0.5.3",
-    "karma-firefox-launcher": "0.1.7",
-    "karma-tap": "1.0.3",
+    "karma": "2.0.0",
+    "karma-chrome-launcher": "2.2.0",
+    "karma-coverage": "1.1.1",
+    "karma-firefox-launcher": "1.1.0",
+    "karma-tap": "3.1.1",
     "karma-webpack": "1.7.0",
     "react": "^15.0.0",
     "react-dom": "^15.0.0",

src/components.js

@@ -461,9 +461,15 @@ export class Datetime extends Component {
 
 }
 
-class ComponentWithChildRefs extends Component {
+@decorators.templates
+export class Struct extends Component {
+
+  static transformer = {
+    format: value => Nil.is(value) ? noobj : value,
+    parse: value => value
+  }
 
-  childRefs = {};
+  childRefs = {}
 
   setChildRefFor = prop => ref => {
     if (ref) {
@@ -473,16 +479,6 @@ class ComponentWithChildRefs extends Component {
     }
   }
 
-}
-
-@decorators.templates
-export class Struct extends ComponentWithChildRefs {
-
-  static transformer = {
-    format: value => Nil.is(value) ? noobj : value,
-    parse: value => value
-  }
-
   isValueNully() {
     return Object.keys(this.childRefs).every((key) => this.childRefs[key].isValueNully())
   }
@@ -609,7 +605,7 @@ function toSameLength(value, keys, uidGenerator) {
 }
 
 @decorators.templates
-export class List extends ComponentWithChildRefs {
+export class List extends Component {
 
   static transformer = {
     format: value => Nil.is(value) ? noarr : value,
@@ -632,6 +628,24 @@ export class List extends ComponentWithChildRefs {
     })
   }
 
+  childRefsByKey = {}
+
+  get childRefs() {
+    return this.state.keys.reduce((acc, key, index) => {
+      acc[index] = this.childRefsByKey[key]
+
+      return acc
+    }, {})
+  }
+
+  setChildRefFor = key => ref => {
+    if (ref) {
+      this.childRefsByKey[key] = ref
+    } else {
+      delete this.childRefsByKey[key]
+    }
+  }
+
   isValueNully() {
     return this.state.value.length === 0
   }
@@ -729,6 +743,7 @@ export class List extends ComponentWithChildRefs {
     const templates = this.getTemplates()
     const value = this.state.value
     return value.map((itemValue, i) => {
+      const key = this.state.keys[i]
       const type = this.typeInfo.innerType.meta.type
       const itemType = getTypeFromUnion(type, itemValue)
       const itemOptions = getComponentOptions(options.item, noobj, itemValue, type)
@@ -757,7 +772,7 @@ export class List extends ComponentWithChildRefs {
       }
       return {
         input: React.createElement(ItemComponent, {
-          ref: this.setChildRefFor(i),
+          ref: this.setChildRefFor(key),
           type: itemType,
           options: itemOptions,
           value: itemValue,
@@ -773,7 +788,7 @@ export class List extends ComponentWithChildRefs {
             path: ctx.path.concat(i)
           }
         }),
-        key: this.state.keys[i],
+        key,
         buttons: buttons
       }
     })

test/components/List.js

@@ -1,5 +1,8 @@
 import tape from 'tape'
 import t from 'tcomb-validation'
+import React from 'react'
+import ReactTestRenderer from 'react-test-renderer'
+import { UIDGenerator } from '../../src/util'
 import { List } from '../../src/components'
 import { ctx } from './util'
 
@@ -47,4 +50,115 @@ tape('List', ({ test }) => {
 
     assert.strictEqual(component.getItems()[0].input.props.type, UnknownAccount)
   })
+
+  test('should properly track refs for cases with asynchronous items elements manipulations', (assert) => {
+    assert.plan(4)
+
+    class LayoutWithAsyncItemsRender extends React.Component {
+      state = {
+        items: [],
+      }
+
+      componentWillReceiveProps() {
+        this.updateItemsFromProps()
+      }
+
+      updateItemsFromProps() {
+        this.setState({
+          items: this.props.items,
+        })
+      }
+
+      removeFirstItem() {
+        this.props.items[0].buttons[0].click()
+      }
+
+      render() {
+        const currentItems = this.props.items
+        const prevItems = this.state.items
+
+        return (
+          <div>
+            {currentItems.map(item => (
+              <div key={item.key}>
+                {item.input}
+              </div>
+            ))}
+            {
+              /*
+              * We render removed children to emulate disappear animation,
+              * tools like react-transition-group do the same.
+              * `updateItemsFromProps` method sync items passed by factory with internal state
+              * so it can be used like animation finish handler.
+              */
+            }
+            {prevItems.map(prevItem => {
+              // item already rendered
+              if (currentItems.some(curItem => curItem.key === prevItem.key)) {
+                return null
+              }
+
+              return (
+                <div key={prevItem.key}>
+                  {prevItem.input}
+                </div>
+              )
+            })}
+          </div>
+        )
+      }
+    }
+
+    let usersRef = null
+    let usersLayoutRef = null
+    const users = React.createElement(List, {
+      ref: ref => usersRef = ref,
+      type: t.list(t.String),
+      value: ['Bob', 'Alice'],
+      options: {
+        template: locals => React.createElement(LayoutWithAsyncItemsRender, {
+          ...locals,
+          ref: ref => usersLayoutRef = ref,
+        }),
+      },
+      onChange() {},
+      ctx: {
+        context: {},
+        uidGenerator: new UIDGenerator('0'),
+        i18n: {},
+        path: [],
+        templates: {
+          textbox: () => null,
+        },
+      }
+    })
+
+    /**
+     * Initial render. For 2 passed values should be collected 2 refs
+     */
+    const renderer = ReactTestRenderer.create(users)
+    assert.strictEqual(Object.keys(usersRef.childRefs).length, 2, '#1.1')
+
+    /**
+     * Remove first item from value. Factory produce single child that will override ref for
+     * "removed" child that still exists in layout state.
+     * It happens because now both children pretend to set ref for same index.
+     */
+    usersLayoutRef.removeFirstItem()
+    renderer.update(users)
+    assert.strictEqual(Object.keys(usersRef.childRefs).length, 1, '#1.2')
+    /**
+     * Ensure that List has ref to correct (actual) item
+     */
+    assert.strictEqual(usersRef.childRefs[0].props.value, 'Alice', '#1.3')
+
+    /**
+     * Apply changes in layout and remove cached outdated child from elements tree.
+     * It will try to remove ref for index that it has in model previously.
+     * It should not remove ref for element that has same index in actual value state.
+     */
+    usersLayoutRef.updateItemsFromProps()
+    renderer.update(users)
+    assert.strictEqual(Object.keys(usersRef.childRefs).length, 1, '#1.4')
+  })
 })