Use async observer when target app is Ember 3.13+ (#777)

* Add `observer` util to opt in to async in 3.13+ * Remove code that skips column-reordering tests for 3.13+ * Use ember 3.12 in package.json * Fix lint * change eslint no-restricted-imports messages, pr feedback * Rename imported ember observer functions, pr feedback * Fix argument order for addObserver/removeObserver * Use `settled` to fix collapse-tree tests The use of `await settled()` seems to be required to properly wait for the now-async observers to finish notifying of property changes to the collapse tree. Also: * Fix propogate typo * Replace some hardcoded for loop lengths in tests with eg `expectedValue.length`
Addepar · Oct 30, 2019 · c5c9cee · c5c9cee
1 parent 8673471
commit c5c9cee
Show file tree

Hide file tree

Showing 14 changed files with 171 additions and 112 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -8,5 +8,24 @@ module.exports = {
     'ember/avoid-leaking-state-in-ember-objects': 'off',
 
     'ember-best-practices/require-dependent-keys': 'off',
+
+    'no-restricted-imports': [
+      'error',
+      {
+        paths: [
+          {
+            name: '@ember/object',
+            importNames: ['observer'],
+            message: 'For compatibility, use `import { observer } from "-private/utils/observer"`',
+          },
+          {
+            name: '@ember/object/observers',
+            importNames: ['addObserver', 'removeObserver'],
+            message:
+              'For compatibility, use `import { addObserver, removeObserver } from "-private/utils/observer"`',
+          },
+        ],
+      },
+    ],
   },
 };
diff --git a/addon/-private/collapse-tree.js b/addon/-private/collapse-tree.js
@@ -3,7 +3,7 @@ import EmberArray, { A as emberA, isArray } from '@ember/array';
 import { assert, warn } from '@ember/debug';
 
 import { computed } from '@ember/object';
-import { addObserver } from '@ember/object/observers';
+import { addObserver } from './utils/observer';
 
 import { objectAt } from './utils/array';
 import { notifyPropertyChange } from './utils/ember';
@@ -758,7 +758,7 @@ export default EmberObject.extend(EmberArray, {
   init() {
     this._super(...arguments);
 
-    // Whenever the root node's length changes we need to propogate the change to
+    // Whenever the root node's length changes we need to propagate the change to
     // users of the tree, and since the tree is meant to work like an array we should
     // trigger a change on the `[]` key as well.
     addObserver(this, 'root.length', () => notifyPropertyChange(this, '[]'));

diff --git a/addon/-private/column-tree.js b/addon/-private/column-tree.js
@@ -1,6 +1,6 @@
 /* eslint-disable getter-return */
 import EmberObject, { get, set } from '@ember/object';
-import { addObserver, removeObserver } from '@ember/object/observers';
+import { addObserver, removeObserver } from './utils/observer';
 import { A as emberA } from '@ember/array';
 import { DEBUG } from '@glimmer/env';
 
@@ -172,7 +172,7 @@ const ColumnTreeNode = EmberObject.extend({
       meta.endReorder = (...args) => tree.endReorder(this, ...args);
 
       // Changes to the value directly should properly update all computeds on this
-      // node, but we need to manually propogate changes upwards to notify any other
+      // node, but we need to manually propagate changes upwards to notify any other
       // watchers
       this._notifyMaxChildDepth = () => notifyPropertyChange(parent, 'maxChildDepth');
       this._notifyLeaves = () => notifyPropertyChange(parent, 'leaves');

diff --git a/addon/-private/utils/computed.js b/addon/-private/utils/computed.js
@@ -1,4 +1,5 @@
-import EmberObject, { defineProperty, computed, observer } from '@ember/object';
+import EmberObject, { defineProperty, computed } from '@ember/object';
+import { observer } from './observer';
 import { alias } from '@ember/object/computed';
 
 const PROPERTIES = new WeakMap();

diff --git a/addon/-private/utils/observer.js b/addon/-private/utils/observer.js
@@ -0,0 +1,64 @@
+import { gte } from 'ember-compatibility-helpers';
+import { assert } from '@ember/debug';
+
+// eslint-disable-next-line no-restricted-imports
+import { observer as emberObserver } from '@ember/object';
+
+// eslint-disable-next-line no-restricted-imports
+import {
+  addObserver as emberAddObserver,
+  removeObserver as emberRemoveObserver,
+} from '@ember/object/observers';
+
+const USE_ASYNC_OBSERVERS = gte('3.13.0');
+
+function asyncObserver(...args) {
+  let fn = args.pop();
+  let dependentKeys = args;
+  let sync = false;
+
+  return emberObserver({ dependentKeys, fn, sync });
+}
+
+function asyncAddObserver(...args) {
+  let obj, path, target, method;
+  let sync = false;
+  obj = args[0];
+  path = args[1];
+  assert(
+    `Expected 3 or 4 args for addObserver, got ${args.length}`,
+    args.length === 3 || args.length === 4
+  );
+  if (args.length === 3) {
+    target = null;
+    method = args[2];
+  } else if (args.length === 4) {
+    target = args[2];
+    method = args[3];
+  }
+
+  return emberAddObserver(obj, path, target, method, sync);
+}
+
+function asyncRemoveObserver(...args) {
+  let obj, path, target, method;
+  let sync = false;
+  obj = args[0];
+  path = args[1];
+  assert(
+    `Expected 3 or 4 args for addObserver, got ${args.length}`,
+    args.length === 3 || args.length === 4
+  );
+  if (args.length === 3) {
+    target = null;
+    method = args[2];
+  } else {
+    target = args[2];
+    method = args[3];
+  }
+  return emberRemoveObserver(obj, path, target, method, sync);
+}
+
+export const observer = USE_ASYNC_OBSERVERS ? asyncObserver : emberObserver;
+export const addObserver = USE_ASYNC_OBSERVERS ? asyncAddObserver : emberAddObserver;
+export const removeObserver = emberRemoveObserver ? asyncRemoveObserver : emberRemoveObserver;
diff --git a/addon/components/-private/base-table-cell.js b/addon/components/-private/base-table-cell.js
@@ -1,7 +1,8 @@
 import Component from '@ember/component';
 import { equal } from '@ember/object/computed';
-import { observer, computed } from '@ember/object';
+import { observer } from '../../-private/utils/observer';
 import { scheduleOnce } from '@ember/runloop';
+import { computed } from '@ember/object';
 
 export default Component.extend({
   // Provided by subclasses

diff --git a/addon/components/ember-tbody/component.js b/addon/components/ember-tbody/component.js
@@ -1,6 +1,7 @@
 import Component from '@ember/component';
 
-import { computed, observer } from '@ember/object';
+import { computed } from '@ember/object';
+import { observer } from '../../-private/utils/observer';
 import { bool, readOnly, or } from '@ember/object/computed';
 
 import { SUPPORTS_INVERSE_BLOCK } from 'ember-compatibility-helpers';

diff --git a/addon/components/ember-thead/component.js b/addon/components/ember-thead/component.js
@@ -3,6 +3,7 @@ import Component from '@ember/component';
 import { bind } from '@ember/runloop';
 import { A as emberA } from '@ember/array';
 import defaultTo from '../../-private/utils/default-to';
+import { addObserver } from '../../-private/utils/observer';
 import { computed } from '@ember/object';
 import { notEmpty, or, readOnly } from '@ember/object/computed';
 
@@ -213,20 +214,20 @@ export default Component.extend({
     this._updateApi();
     this._updateColumnTree();
 
-    this.addObserver('sorts', this._updateApi);
-    this.addObserver('sortFunction', this._updateApi);
-    this.addObserver('reorderFunction', this._updateApi);
+    addObserver(this, 'sorts', this._updateApi);
+    addObserver(this, 'sortFunction', this._updateApi);
+    addObserver(this, 'reorderFunction', this._updateApi);
 
-    this.addObserver('sorts', this._updateColumnTree);
-    this.addObserver('columns.[]', this._onColumnsChange);
-    this.addObserver('fillMode', this._updateColumnTree);
-    this.addObserver('fillColumnIndex', this._updateColumnTree);
-    this.addObserver('resizeMode', this._updateColumnTree);
-    this.addObserver('widthConstraint', this._updateColumnTree);
+    addObserver(this, 'sorts', this._updateColumnTree);
+    addObserver(this, 'columns.[]', this._onColumnsChange);
+    addObserver(this, 'fillMode', this._updateColumnTree);
+    addObserver(this, 'fillColumnIndex', this._updateColumnTree);
+    addObserver(this, 'resizeMode', this._updateColumnTree);
+    addObserver(this, 'widthConstraint', this._updateColumnTree);
 
-    this.addObserver('enableSort', this._updateColumnTree);
-    this.addObserver('enableResize', this._updateColumnTree);
-    this.addObserver('enableReorder', this._updateColumnTree);
+    addObserver(this, 'enableSort', this._updateColumnTree);
+    addObserver(this, 'enableResize', this._updateColumnTree);
+    addObserver(this, 'enableReorder', this._updateColumnTree);
   },
 
   _updateApi() {

diff --git a/config/ember-try.js b/config/ember-try.js
@@ -167,9 +167,6 @@ module.exports = function() {
         },
         {
           name: 'ember-release',
-          env: {
-            SKIP_REORDERING_TESTS: true,
-          },
           npm: {
             devDependencies: {
               'ember-source': urls[0],
@@ -179,9 +176,6 @@ module.exports = function() {
         },
         {
           name: 'ember-beta',
-          env: {
-            SKIP_REORDERING_TESTS: true,
-          },
           npm: {
             devDependencies: {
               'ember-source': urls[1],
@@ -191,9 +185,6 @@ module.exports = function() {
         },
         {
           name: 'ember-canary',
-          env: {
-            SKIP_REORDERING_TESTS: true,
-          },
           npm: {
             devDependencies: {
               'ember-source': urls[2],

diff --git a/tests/dummy/config/environment.js b/tests/dummy/config/environment.js
@@ -16,7 +16,6 @@ module.exports = function(environment) {
     fastboot: {
       hostWhitelist: [/^localhost:\d+$/],
     },
-    skipReorderingTests: process.env.SKIP_REORDERING_TESTS,
     EmberENV: {
       FEATURES: {
         // Here you can enable experimental features on an ember canary build

diff --git a/tests/integration/components/headers/reorder-test.js b/tests/integration/components/headers/reorder-test.js
@@ -9,7 +9,6 @@ import { getScale } from 'ember-table/test-support/helpers/element';
 
 import TablePage from 'ember-table/test-support/pages/ember-table';
 import { toBase26 } from 'dummy/utils/base-26';
-import config from 'dummy/config/environment';
 
 const table = new TablePage();
 
@@ -46,13 +45,6 @@ async function reorderToRightEdge(column, edgeOffset = 0) {
 }
 
 module('Integration | headers | reorder', function() {
-  if (config.skipReorderingTests) {
-    skip('Skipping column reordering tests, see https://github.com/Addepar/ember-table/issues/775', async function(assert) {
-      assert.ok(true, 'Skipping column reordering tests');
-    });
-    return;
-  }
-
   componentModule('reordering', function() {
     test('standard columns', async function(assert) {
       await generateTable(this);

diff --git a/tests/integration/components/selection-test.js b/tests/integration/components/selection-test.js
@@ -585,7 +585,7 @@ module('Integration | selection', () => {
         let rows = generateRows(1, 1);
         await generateTable(this, { rows });
 
-        this.set('selection', [...rows, { fakeRow: true }]);
+        run(() => this.set('selection', [...rows, { fakeRow: true }]));
         assert.ok(true, 'after setting bad selection, no error');
         assert.ok(table.validateSelected(0), 'First row is selected');