Merge pull request #97 from thefrontside/ember-data-rollback-error

Don't update value if tearing component down

addon/components/x-select.js

@@ -71,11 +71,7 @@ export default Ember.Component.extend({
    * component's action with the current value.
    */
   change(event) {
-    if (this.get('multiple')) {
-      this._updateValueMultiple();
-    } else {
-      this._updateValueSingle();
-    }
+    this._updateValue();
 
     this.sendAction('action', this.get('value'), this);
     this.sendAction('onchange', this, this.get('value'), event);
@@ -135,19 +131,30 @@ export default Ember.Component.extend({
     this.set('value', Ember.A(options).mapBy('value'));
   },
 
+  /**
+   * A utility method to determine if the select is multiple or single and call
+   * its respective method to update the value.
+   *
+   * @private
+   * @utility
+   */
+  _updateValue: function() {
+    if (this.get('multiple')) {
+      this._updateValueMultiple();
+    } else {
+      this._updateValueSingle();
+    }
+  },
+
   /**
    * If no explicit value is set, apply default values based on selected=true in
    * the template.
    *
    * @private
    */
   _setDefaultValues: function() {
-    if( !this.get('value')){
-      if (this.get('multiple')) {
-        this._updateValueMultiple();
-      } else {
-        this._updateValueSingle();
-      }
+    if (!this.get('value')) {
+      this._updateValue();
     }
   },
 
@@ -200,6 +207,10 @@ export default Ember.Component.extend({
    */
   unregisterOption: function(option) {
     this.get('options').removeObject(option);
-    this._updateValueSingle();
+
+    // We don't want to update the value if we're tearing the component down.
+    if (!this.get('isDestroying')) {
+      this._updateValue();
+    }
   }
 });

bower.json

@@ -2,7 +2,7 @@
   "name": "emberx-select",
   "dependencies": {
     "ember": "1.13.7",
-    "ember-cli-shims": "ember-cli/ember-cli-shims#0.0.3",
+    "ember-cli-shims": "ember-cli/ember-cli-shims#0.1.0",
     "ember-cli-test-loader": "ember-cli-test-loader#0.1.3",
     "ember-mocha": "0.8.8",
     "chai-jquery": "~2.0.0",

package.json

@@ -31,9 +31,11 @@
     "ember-cli-mocha": "0.9.8",
     "ember-cli-release": "0.2.5",
     "ember-cli-uglify": "^1.2.0",
+    "ember-data": "2.4.0",
     "ember-disable-prototype-extensions": "^1.0.0",
     "ember-disable-proxy-controllers": "^1.0.0",
     "ember-export-application-global": "^1.0.3",
+    "ember-inflector": "1.9.4",
     "ember-try": "0.0.6"
   },
   "keywords": [

tests/acceptance/ember-data-test.js

@@ -0,0 +1,41 @@
+/* jshint expr:true */
+import { describe, it, beforeEach, afterEach } from 'mocha';
+import { expect } from 'chai';
+import startApp from '../helpers/start-app';
+import Ember from 'ember';
+
+describe('Acceptance: EmberData', function() {
+  let application;
+
+  beforeEach(function() {
+    application = startApp();
+    visit('/ember-data');
+  });
+
+  afterEach(function() {
+    Ember.run(application, 'destroy');
+  });
+
+  it('can visit /ember-data', function() {
+    expect(currentPath()).to.equal('ember-data');
+  });
+
+  describe("selecting a new value", function() {
+    beforeEach(function() {
+      select('.x-select', "Ollie");
+    });
+
+    describe("navigating to another route", function() {
+      beforeEach(function() {
+        return click("a:contains('Single')");
+      });
+
+      it("doesn't blow up", function() {
+        expect(currentPath()).to.equal('single');
+      });
+
+    });
+
+  });
+
+});

tests/dummy/app/controllers/ember-data.js

@@ -0,0 +1,9 @@
+import Ember from 'ember';
+
+export default Ember.Controller.extend({
+  actions: {
+    rollback() {
+      this.get('model').rollbackAttributes();
+    }
+  }
+});

tests/dummy/app/models/person.js

@@ -0,0 +1,5 @@
+import DS from 'ember-data';
+
+export default DS.Model.extend({
+  name: DS.attr('string', { defaultValue: "Wally" })
+});

tests/dummy/app/router.js

@@ -15,6 +15,7 @@ Router.map(function() {
     this.route('click');
     this.route('focus-out');
   });
+  this.route('ember-data');
 });
 
 export default Router;

tests/dummy/app/routes/ember-data.js

@@ -0,0 +1,14 @@
+import Ember from 'ember';
+
+export default Ember.Route.extend({
+  model() {
+    return this.store.createRecord('person');
+  },
+  actions: {
+    willTransition() {
+      this._super.apply(this, arguments);
+
+      this.controller.send('rollback');
+    }
+  }
+});

tests/dummy/app/styles/app.css

@@ -4,3 +4,7 @@ a {
 a.active {
   color: blue;
 }
+
+div {
+  margin: 10px 0;
+}

tests/dummy/app/templates/application.hbs

@@ -4,7 +4,8 @@
 {{#link-to 'multiple'}}Multiple{{/link-to}} |
 {{#link-to 'zany-embedded-html'}} Embedded HTML{{/link-to}} |
 {{#link-to 'default-value'}}With Defaults{{/link-to}} |
-{{#link-to 'events'}}Events & actions{{/link-to}}
+{{#link-to 'events'}}Events & actions{{/link-to}} |
+{{#link-to 'ember-data'}}Ember Data{{/link-to}}
 
 <div>
   {{outlet}}

tests/dummy/app/templates/ember-data.hbs

@@ -0,0 +1,10 @@
+<div>
+  {{#x-select value=model.name}}
+    {{#x-option value="Larry"}}Larry{{/x-option}}
+    {{#x-option value="Ollie"}}Ollie{{/x-option}}
+    {{#x-option value="Wally"}}Wally{{/x-option}}
+  {{/x-select}}
+
+  <p>Model: {{model.name}}</p>
+
+</div>