Use isComputed when available (#966)

* Attempt to use isComputed * Fix some failures * Point ember-table back at Addepar * Use correct path to isComputed * Pass object and key into isComputed and inspectValue * Get value from object * Update to ember canary * Point to commit for ember-table * Use object[key] rather than get(object, key) * Allow passing already computedValue * Fix error with removing listener * Add hacks to check for computed * Fix remaining failures * Set back to stable Ember version, disallow beta failures * Allow beta failures again * Fix lint * Run normal tests before ember-try
emberjs · May 9, 2019 · aede612 · aede612
1 parent 2237dc1
commit aede612
Show file tree

Hide file tree

Showing 11 changed files with 533 additions and 589 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -24,6 +24,13 @@ jobs:
     - env: EMBER_TRY_SCENARIO=ember-canary
 
   include:
+    # runs tests with current locked deps and linting
+    - stage: test
+      script:
+        - yarn lint:hbs
+        - yarn lint:js
+        - yarn test
+
     # runs tests for ember_debug against each supported Ember version
     - stage: ember debug test
       env: EMBER_TRY_SCENARIO=ember-2.7
@@ -34,18 +41,12 @@ jobs:
     - env: EMBER_TRY_SCENARIO=ember-lts-2.12
     - env: EMBER_TRY_SCENARIO=ember-lts-2.16
     - env: EMBER_TRY_SCENARIO=ember-lts-2.18
+    - env: EMBER_TRY_SCENARIO=ember-lts-3.4
     - env: EMBER_TRY_SCENARIO=ember-release
     - env: EMBER_TRY_SCENARIO=ember-beta
     - env: EMBER_TRY_SCENARIO=ember-canary
     - env: EMBER_TRY_SCENARIO=ember-default
 
-    # runs tests with current locked deps and linting
-    - stage: test
-      script:
-        - yarn lint:hbs
-        - yarn lint:js
-        - yarn test
-
 before_install:
   - curl -o- -L https://yarnpkg.com/install.sh | bash
   - export PATH=$HOME/.yarn/bin:$PATH

diff --git a/app/routes/model-types.js b/app/routes/model-types.js
@@ -12,7 +12,7 @@ export default TabRoute.extend({
   model() {
     const port = this.get('port');
     return new Promise(function(resolve) {
-      port.one('data:modelTypesAdded', this, function(message) {
+      port.one('data:modelTypesAdded', function(message) {
         resolve(message.modelTypes);
       });
       port.send('data:getModelTypes');

diff --git a/ember_debug/libs/promise-assembler.js b/ember_debug/libs/promise-assembler.js
@@ -1,18 +1,21 @@
 /**
-  Original implementation and the idea behind the `PromiseAssembler`,
-  `Promise` model, and other work related to promise inspection was done
-  by Stefan Penner (@stefanpenner) thanks to McGraw Hill Education (@mhelabs)
-  and Yapp Labs (@yapplabs).
+ Original implementation and the idea behind the `PromiseAssembler`,
+ `Promise` model, and other work related to promise inspection was done
+ by Stefan Penner (@stefanpenner) thanks to McGraw Hill Education (@mhelabs)
+ and Yapp Labs (@yapplabs).
  */
 
 import Promise from 'ember-debug/models/promise';
+
 const Ember = window.Ember;
 const { Object: EmberObject, Evented, A, computed, RSVP, isNone } = Ember;
 
 let PromiseAssembler = EmberObject.extend(Evented, {
   // RSVP lib to debug
   RSVP,
 
+  isStarted: false,
+
   all: computed(function() { return A(); }),
 
   promiseIndex: computed(function() { return {}; }),
@@ -40,25 +43,30 @@ let PromiseAssembler = EmberObject.extend(Evented, {
     this.RSVP.on('rejected', this.promiseRejected);
     this.RSVP.on('fulfilled', this.promiseFulfilled);
     this.RSVP.on('created', this.promiseCreated);
+
+    this.isStarted = true;
   },
 
   stop() {
-    this.RSVP.configure('instrument', false);
-    this.RSVP.off('chained', this.promiseChained);
-    this.RSVP.off('rejected', this.promiseRejected);
-    this.RSVP.off('fulfilled', this.promiseFulfilled);
-    this.RSVP.off('created', this.promiseCreated);
-
-    this.get('all').forEach(item => {
-      item.destroy();
-    });
-    this.set('all', A());
-    this.set('promiseIndex', {});
-
-    this.promiseChained = null;
-    this.promiseRejected = null;
-    this.promiseFulfilled = null;
-    this.promiseCreated = null;
+    if (this.isStarted) {
+      this.RSVP.configure('instrument', false);
+      this.RSVP.off('chained', this.promiseChained);
+      this.RSVP.off('rejected', this.promiseRejected);
+      this.RSVP.off('fulfilled', this.promiseFulfilled);
+      this.RSVP.off('created', this.promiseCreated);
+
+      this.get('all').forEach(item => {
+        item.destroy();
+      });
+      this.set('all', A());
+      this.set('promiseIndex', {});
+
+      this.promiseChained = null;
+      this.promiseRejected = null;
+      this.promiseFulfilled = null;
+      this.promiseCreated = null;
+      this.isStarted = false;
+    }
   },
 
   willDestroy() {

diff --git a/ember_debug/object-inspector.js b/ember_debug/object-inspector.js
@@ -1,21 +1,36 @@
 import PortMixin from 'ember-debug/mixins/port-mixin';
 import { compareVersion } from 'ember-debug/utils/version';
+import { isComputed, isDescriptor, getDescriptorFor } from 'ember-debug/utils/type-check';
 
 const Ember = window.Ember;
 const {
   Object: EmberObject, inspect: emberInspect, meta: emberMeta, typeOf,
-  computed, get, set, ComputedProperty, guidFor, isNone, removeObserver,
+  computed, get, set, guidFor, isNone, removeObserver,
   Mixin, addObserver, cacheFor, VERSION
 } = Ember;
 const { oneWay } = computed;
 
 const keys = Object.keys || Ember.keys;
 
-function inspectValue(value) {
+/**
+ * Determine the type and get the value of the passed property
+ * @param {*} object The parent object we will look for `key` on
+ * @param {string} key The key for the property which points to a computed, EmberObject, etc
+ * @param {*} computedValue A value that has already been computed with calculateCP
+ * @return {{inspect: (string|*), type: string}|{computed: boolean, inspect: string, type: string}|{inspect: string, type: string}}
+ */
+function inspectValue(object, key, computedValue) {
   let string;
+  const value = computedValue ? computedValue : object[key];
+
+  // TODO: this is not very clean. We should refactor calculateCP, etc, rather than passing computedValue
+  if (computedValue) {
+    return { type: `type-${typeOf(value)}`, inspect: inspect(value) };
+  }
+
   if (value instanceof EmberObject) {
     return { type: 'type-ember-object', inspect: value.toString() };
-  } else if (isComputed(value)) {
+  } else if (isComputed(object, key)) {
     string = '<computed>';
     return { type: 'type-descriptor', inspect: string, computed: true };
   } else if (isDescriptor(value)) {
@@ -25,11 +40,6 @@ function inspectValue(value) {
   }
 }
 
-function isDescriptor(value) {
-  // Ember >= 1.11
-  return value && typeof value === 'object' && value.isDescriptor;
-}
-
 function inspect(value) {
   if (typeof value === 'function') {
     return 'function() { ... }';
@@ -360,7 +370,7 @@ export default EmberObject.extend(PortMixin, {
         if (!name && typeof mixin.toString === 'function') {
           try {
             name = mixin.toString();
-          } catch (e) {
+          } catch(e) {
             name = '(Unable to convert Object to string)';
           }
         }
@@ -407,7 +417,7 @@ export default EmberObject.extend(PortMixin, {
     }
 
     if (!value || !(value instanceof CalculateCPError)) {
-      value = inspectValue(value);
+      value = inspectValue(object, property, value);
       value.computed = true;
 
 
@@ -424,7 +434,7 @@ export default EmberObject.extend(PortMixin, {
 
     let handler = () => {
       let value = get(object, property);
-      value = inspectValue(value);
+      value = inspectValue(object, property, value);
       value.computed = computed;
 
       this.sendMessage('updateProperty', { objectId, property, value, mixinIndex });
@@ -501,8 +511,8 @@ function addProperties(properties, hash) {
       }
     }
 
-    if (isComputed(hash[prop])) {
-      options.dependentKeys = (hash[prop]._dependentKeys || []).map((key) => key.toString());
+    if (isComputed(hash, prop)) {
+      options.dependentKeys = (getDescriptorFor(hash, prop)._dependentKeys || []).map((key) => key.toString());
       if (!options.isService) {
         if (typeof hash[prop]._getter === 'function') {
           options.code = Function.prototype.toString.call(hash[prop]._getter);
@@ -512,7 +522,7 @@ function addProperties(properties, hash) {
       }
       options.readOnly = hash[prop]._readOnly;
     }
-    replaceProperty(properties, prop, hash[prop], options);
+    replaceProperty(properties, prop, inspectValue(hash, prop), options);
   }
 }
 
@@ -531,7 +541,7 @@ function replaceProperty(properties, name, value, options) {
     properties.splice(i, 1);
   }
 
-  let prop = { name, value: inspectValue(value) };
+  let prop = { name, value };
   prop.isMandatorySetter = options.isMandatorySetter;
   prop.readOnly = options.readOnly;
   prop.dependentKeys = options.dependentKeys || [];
@@ -611,7 +621,7 @@ function calculateCPs(object, mixinDetails, errorsForObject, expensiveProperties
         if (cache !== undefined || expensiveProperties.indexOf(item.name) === -1) {
           let value = calculateCP(object, item.name, errorsForObject);
           if (!value || !(value instanceof CalculateCPError)) {
-            item.value = inspectValue(value);
+            item.value = inspectValue(object, item.name, value);
             item.value.computed = true;
           }
         }
@@ -778,7 +788,6 @@ function getDebugInfo(object) {
 
   let meta = Ember.meta(object);
   for (let prop in object) {
-
     // when in Ember 3.1
     if (compareVersion(VERSION, '3.1.0') !== -1) {
       // in Ember 3.1+ CP's are eagerly invoked via a normal
@@ -798,9 +807,7 @@ function getDebugInfo(object) {
   return debugInfo;
 }
 
-function isComputed(value) {
-  return value instanceof ComputedProperty;
-}
+
 
 function toArray(errors) {
   return keys(errors).map(key => errors[key]);
@@ -810,7 +817,7 @@ function calculateCP(object, property, errorsForObject) {
   delete errorsForObject[property];
   try {
     return get(object, property);
-  } catch (error) {
+  } catch(error) {
     errorsForObject[property] = { property, error };
     return new CalculateCPError();
   }

diff --git a/ember_debug/promise-debug.js b/ember_debug/promise-debug.js
@@ -113,12 +113,10 @@ export default EmberObject.extend(PortMixin, {
     this.get('promiseAssembler').on('chained', this, this.promiseChained);
 
     this.get('releaseMethods').pushObject(() => {
-
       this.get('promiseAssembler').off('created', this, this.promiseUpdated);
       this.get('promiseAssembler').off('fulfilled', this, this.promiseUpdated);
       this.get('promiseAssembler').off('rejected', this, this.promiseUpdated);
-      this.get('promiseAssembler').off('fulfilled', this, this.promiseChained);
-
+      this.get('promiseAssembler').off('chained', this, this.promiseChained);
     });
 
     this.promisesUpdated(this.get('promiseAssembler').find());
@@ -166,8 +164,8 @@ export default EmberObject.extend(PortMixin, {
       serialized.children = this.promiseIds(promise.get('children'));
     }
     serialized.parent = promise.get('parent.guid');
-    serialized.value = this.inspectValue(promise.get('value'));
-    serialized.reason = this.inspectValue(promise.get('reason'));
+    serialized.value = this.inspectValue(promise, 'value');
+    serialized.reason = this.inspectValue(promise, 'reason');
     if (promise.get('createdAt')) {
       serialized.createdAt = promise.get('createdAt').getTime();
     }
@@ -182,12 +180,18 @@ export default EmberObject.extend(PortMixin, {
     return promises.map(promise => promise.get('guid'));
   },
 
-  inspectValue(value) {
+  /**
+   * Inspect the promise and pass to object inspector
+   * @param {Promise} promise The promise object
+   * @param {string} key The key for the property on the promise
+   * @return {*|{inspect: (string|*), type: string}|{computed: boolean, inspect: string, type: string}|{inspect: string, type: string}}
+   */
+  inspectValue(promise, key) {
     let objectInspector = this.get('objectInspector');
-    let inspected = objectInspector.inspectValue(value);
+    let inspected = objectInspector.inspectValue(promise, key);
 
     if (inspected.type === 'type-ember-object' || inspected.type === "type-array") {
-      inspected.objectId = objectInspector.retainObject(value);
+      inspected.objectId = objectInspector.retainObject(promise.get(key));
       this.get('releaseMethods').pushObject(function() {
         objectInspector.releaseObject(inspected.objectId);
       });

diff --git a/ember_debug/utils/type-check.js b/ember_debug/utils/type-check.js
@@ -0,0 +1,37 @@
+const Ember = window.Ember;
+const { ComputedProperty } = Ember;
+
+/**
+ * Check if given key on the passed object is a computed property
+ * @param object
+ * @param key
+ * @return {boolean|*}
+ */
+export function isComputed(object, key) {
+  // Ember > 3.10
+  if (Ember.Debug.isComputed) {
+    return Ember.Debug.isComputed(object, key) || getDescriptorFor(object, key);
+  }
+
+  // Ember < 3.10
+  return object[key] instanceof ComputedProperty;
+}
+
+export function isDescriptor(value) {
+  // Ember >= 1.11
+  return value && typeof value === 'object' && value.isDescriptor;
+}
+
+/**
+ * This allows us to pass in a COMPUTED_DECORATOR function and get the descriptor for it.
+ * It should be implemented Ember side eventually.
+ * @param {EmberObject} object The object we are inspecting
+ * @param {String} key The key for the property on the object
+ */
+export function getDescriptorFor(object, key) {
+  if (Ember.Debug.isComputed) {
+    return Ember.__loader.require('@ember/-internals/metal').descriptorForDecorator(object[key]);
+  }
+
+  return object[key];
+}
diff --git a/package.json b/package.json
@@ -46,8 +46,8 @@
     "broccoli-stew": "^2.1.0",
     "broccoli-string-replace": "^0.1.1",
     "compare-versions": "^3.4.0",
-    "del": "4.1.0",
-    "ember-cli": "~3.10.0-beta.1",
+    "del": "^4.1.1",
+    "ember-cli": "^3.9.0",
     "ember-cli-app-version": "^3.2.0",
     "ember-cli-babel": "^7.7.3",
     "ember-cli-dependency-checker": "^3.1.0",
@@ -73,23 +73,23 @@
     "ember-maybe-import-regenerator": "^0.1.6",
     "ember-qunit": "^4.4.1",
     "ember-resolver": "^5.0.1",
-    "ember-source": "~3.9.1",
+    "ember-source": "^3.9.1",
     "ember-source-channel-url": "^1.1.0",
     "ember-svg-jar": "^1.2.2",
-    "ember-table": "Addepar/ember-table#master",
+    "ember-table": "Addepar/ember-table#a1d5122a902efe176d9740f04dc26a06247792b9",
     "ember-test-selectors": "^2.1.0",
     "ember-truth-helpers": "^2.1.0",
     "ember-try": "^1.1.0",
     "eslint-plugin-ember": "^6.3.0",
     "fstream": "^1.0.8",
-    "gulp": "4.0.1",
-    "gulp-zip": "4.2.0",
+    "gulp": "^4.0.2",
+    "gulp-zip": "^4.2.0",
     "js-string-escape": "^1.0.0",
     "loader.js": "^4.7.0",
     "mkdirp": "^0.5.1",
     "qunit-dom": "^0.8.4",
     "rimraf": "^2.5.2",
-    "sass": "^1.19.0",
+    "sass": "^1.20.1",
     "stylelint": "^10.0.1",
     "stylelint-config-ship-shape": "^0.5.2",
     "yauzl": "^2.10.0"