[bugfix] Properly alias cellValue (#567)

* [bugfix] Properly alias cellValue

cellValue is a dynamically computed value that depends on the row value
and a dynamic key (the `column.valuePath` property). Currently it
updates the value once, and never after that. It can't respond to
external changes, and changes can't be bound.

This introduces a computed property that aliases based on a dynamic key.
It's based on the work in ember-macro-helpers, simplified a bit since we
don't need all the functionality in there. It could possibly be
simplified further in the future, but should get us where we need to be
for now.

* fix older ember

.travis.yml

@@ -20,7 +20,7 @@ global:
 env:
   # we recommend new addons test the current and previous LTS
   # as well as latest stable release (bonus points to beta/canary)
-  - EMBER_TRY_SCENARIO=ember-1.11
+  - EMBER_TRY_SCENARIO=ember-1.12
   - EMBER_TRY_SCENARIO=ember-1.13
   - EMBER_TRY_SCENARIO=ember-lts-2.8
   - EMBER_TRY_SCENARIO=ember-lts-2.18

addon/-private/utils/computed.js

@@ -0,0 +1,119 @@
+import EmberObject, { defineProperty, computed, observer } from '@ember/object';
+import { addListener } from '@ember/object/events';
+import { alias } from '@ember/object/computed';
+import { macro } from '@ember-decorators/object/computed';
+
+const PROPERTIES = new WeakMap();
+
+function findOrCreatePropertyInstance(propertyClass, context, key) {
+  if (!PROPERTIES.has(context)) {
+    PROPERTIES.set(context, {});
+  }
+
+  let propertiesForContext = PROPERTIES.get(context);
+
+  let property = propertiesForContext[key];
+  if (property) {
+    return property;
+  }
+
+  property = propertyClass.create({
+    _key: key,
+    _context: context,
+  });
+
+  addListener(
+    context,
+    'willDestroy',
+    () => {
+      property.destroy();
+    },
+    true
+  );
+
+  propertiesForContext[key] = property;
+  return property;
+}
+
+const ClassBasedComputedProperty = EmberObject.extend({
+  _context: null,
+  _key: null,
+  _computedFunction: null,
+  _dependencies: null,
+
+  init() {
+    this._redefineProperty();
+  },
+
+  // eslint-disable-next-line
+  _contentDidChange: observer('_content', function() {
+    if (!this._isUpdating) {
+      this._context.notifyPropertyChange(this._key);
+    }
+  }),
+
+  _redefineProperty() {
+    let dependencies = this.get('_dependencies');
+    let isDynamicList = this.get('_isDynamicList');
+
+    let computed = this._computedFunction(
+      ...dependencies.map((d, i) => (isDynamicList[i] ? this.get(d) : d))
+    );
+
+    defineProperty(this, '_content', computed);
+  },
+
+  _get() {
+    return this.get('_content');
+  },
+
+  _set(key, value) {
+    this._isUpdating = true;
+    this.set('_content', value);
+    this._isUpdating = false;
+
+    return this._get();
+  },
+});
+
+function classComputedProperty(isDynamicList, computedFunction) {
+  return function(...dependencies) {
+    let extension = {
+      _computedFunction: computedFunction,
+      _isDynamicList: isDynamicList,
+      _dependencies: dependencies,
+    };
+
+    dependencies.forEach((dep, index) => {
+      extension[dep] = alias(`_context.${dep}`);
+
+      if (isDynamicList[index] === true) {
+        // eslint-disable-next-line
+        extension[`${dep}DidChange`] = observer(`_context.${dep}`, function() {
+          this._redefineProperty();
+        });
+      }
+    });
+
+    let klass = ClassBasedComputedProperty.extend(extension);
+
+    return computed(...dependencies, {
+      get(key) {
+        let property = findOrCreatePropertyInstance(klass, this, key);
+
+        return property._get();
+      },
+      set(key, value) {
+        let property = findOrCreatePropertyInstance(klass, this, key);
+
+        return property._set(key, value);
+      },
+    });
+  };
+}
+
+export const dynamicAlias = macro(
+  classComputedProperty([false, true], function(...segments) {
+    return alias(segments.join('.'));
+  })
+);

addon/components/-private/row-wrapper.js

@@ -10,17 +10,13 @@ import { argument } from '@ember-decorators/argument';
 import { computed } from '@ember-decorators/object';
 
 import { objectAt } from '../../-private/utils/array';
+import { dynamicAlias } from '../../-private/utils/computed';
 
 import { guidFor } from '@ember/object/internals';
 
 class CellWrapper extends EmberObject {
-  @computed('rowValue', 'columnValue.valuePath')
-  get cellValue() {
-    let row = get(this, 'rowValue');
-    let valuePath = get(this, 'columnValue.valuePath');
-
-    return get(row, valuePath);
-  }
+  @dynamicAlias('rowValue', 'columnValue.valuePath')
+  cellValue;
 
   @computed('rowValue', 'columnValue.valuePath')
   get cellMeta() {

addon/components/ember-td/component.js

@@ -2,7 +2,7 @@ import Component from '@ember/component';
 import { htmlSafe } from '@ember/string';
 
 import { action, computed } from '@ember-decorators/object';
-import { readOnly, equal } from '@ember-decorators/object/computed';
+import { alias, readOnly, equal } from '@ember-decorators/object/computed';
 import { tagName, attribute, className } from '@ember-decorators/component';
 import { argument } from '@ember-decorators/argument';
 import { type, optional } from '@ember-decorators/argument/type';
@@ -63,17 +63,22 @@ export default class EmberTd extends Component {
   @type(optional(Action))
   onDoubleClick;
 
-  @readOnly('api.cellValue') cellValue;
-  @readOnly('api.cellMeta') cellMeta;
+  @computed('api.api')
+  get unwrappedApi() {
+    return this.get('api.api') || this.get('api');
+  }
+
+  @alias('unwrappedApi.cellValue') cellValue;
+  @readOnly('unwrappedApi.cellMeta') cellMeta;
 
-  @readOnly('api.columnValue') columnValue;
-  @readOnly('api.columnMeta') columnMeta;
+  @readOnly('unwrappedApi.columnValue') columnValue;
+  @readOnly('unwrappedApi.columnMeta') columnMeta;
 
-  @readOnly('api.rowValue') rowValue;
-  @readOnly('api.rowMeta') rowMeta;
+  @readOnly('unwrappedApi.rowValue') rowValue;
+  @readOnly('unwrappedApi.rowMeta') rowMeta;
 
-  @readOnly('api.rowSelectionMode') rowSelectionMode;
-  @readOnly('api.checkboxSelectionMode') checkboxSelectionMode;
+  @readOnly('unwrappedApi.rowSelectionMode') rowSelectionMode;
+  @readOnly('unwrappedApi.checkboxSelectionMode') checkboxSelectionMode;
 
   @className
   @equal('columnMeta.index', 0)

addon/components/ember-td/template.hbs

@@ -44,4 +44,3 @@
     {{cellValue}}
   {{/if}}
 {{/if}}
-

addon/components/ember-tr/template.hbs

@@ -16,6 +16,8 @@
       )}}
     {{else}}
       {{yield (hash
+        api=api
+
         cellValue=api.cellValue
         cellMeta=api.cellMeta
 
@@ -25,9 +27,6 @@
         rowValue=api.rowValue
         rowMeta=api.rowMeta
 
-        rowSelectionMode=api.rowSelectionMode
-        checkboxSelectionMode=api.checkboxSelectionMode
-
         cell=(component "ember-td" api=api)
       )}}
     {{/if}}

config/ember-try.js

@@ -10,15 +10,15 @@ module.exports = function() {
       useYarn: true,
       scenarios: [
         {
-          name: 'ember-1.11',
+          name: 'ember-1.12',
           bower: {
             dependencies: {
-              ember: '~1.11.0',
+              ember: '~1.12.0',
               'ember-cli-shims': 'ember-cli/ember-cli-shims#0.0.3',
               'ember-data': '~1.13.0',
             },
             resolutions: {
-              ember: '~1.11.0',
+              ember: '~1.12.0',
               'ember-cli-shims': '0.0.3',
               'ember-data': '~1.13.0',
             },

package.json

@@ -37,7 +37,7 @@
     "ember-cli-sass": "^7.0.0",
     "ember-cli-version-checker": "^2.1.2",
     "ember-compatibility-helpers": "^1.0.0",
-    "ember-decorators": "^2.0.0-beta.1",
+    "ember-decorators": "^2.2.0",
     "ember-getowner-polyfill": "^2.2.0",
     "ember-legacy-class-shim": "^1.0.0",
     "ember-raf-scheduler": "^0.1.0",

tests/integration/components/cell-test.js

@@ -1,9 +1,15 @@
 import { module, test } from 'ember-qunit';
+import hbs from 'htmlbars-inline-precompile';
 
-import { generateTable } from '../../helpers/generate-table';
+import { generateTable, generateColumns } from '../../helpers/generate-table';
 import { componentModule } from '../../helpers/module';
+import { set, get } from '@ember/object';
+
+import { fillIn } from 'ember-native-dom-helpers';
+import wait from 'ember-test-helpers/wait';
 
 import TablePage from 'ember-table/test-support/pages/ember-table';
+import { run } from '@ember/runloop';
 
 let table = new TablePage();
 
@@ -51,4 +57,65 @@ module('Integration | cell', function() {
       await table.getCell(0, 0).doubleClick();
     });
   });
+
+  componentModule('mutation', function() {
+    test('it updates cell values when changed externally', async function(assert) {
+      let columnCount = 2;
+      let rows = [
+        {
+          A: 'A',
+          B: 'B',
+        },
+      ];
+
+      await generateTable(this, { rows, columnCount });
+
+      assert.equal(table.getCell(0, 0).text, 'A', 'renders correct initial value');
+      assert.equal(table.getCell(0, 1).text, 'B', 'renders correct initial value');
+
+      run(() => {
+        set(rows[0], 'A', 'Y');
+        set(rows[0], 'B', 'Z');
+      });
+
+      await wait();
+
+      assert.equal(table.getCell(0, 0).text, 'Y', 'renders correct updated value');
+      assert.equal(table.getCell(0, 1).text, 'Z', 'renders correct updated value');
+    });
+
+    test('Can update cell values directly', async function(assert) {
+      let columnCount = 1;
+      let rows = [
+        {
+          A: 'A',
+        },
+      ];
+
+      this.set('columns', generateColumns(columnCount));
+      this.set('rows', rows);
+
+      this.render(hbs`
+        <div id="container" style="height: 500px;">
+          {{#ember-table as |t|}}
+            {{ember-thead api=t columns=columns}}
+
+            {{#ember-tbody api=t rows=rows as |b|}}
+              {{#ember-tr api=b as |r|}}
+                {{#ember-td api=r as |cellValue|}}
+                  {{input value=cellValue}}
+                {{/ember-td}}
+              {{/ember-tr}}
+            {{/ember-tbody}}
+          {{/ember-table}}
+        </div>
+      `);
+
+      await wait();
+
+      fillIn('input', 'Z');
+
+      assert.equal(get(rows[0], 'A'), 'Z', 'value updated successfully');
+    });
+  });
 });