Ember.js: [Bug release] Issue with observers/computed properties after switching over to Ember 2.10

[dusanstanojeviccs]

We've been using Ember 2.9 without any issues, but after switching over to Ember 2.10 we've started seeing some issues. The issue we are seeing seems to be happening when an observer or a computed property of a destroyed component is triggered on route transition.

To demonstrate the issue we've created a github repo:
https://github.com/dusanstanojeviccs/issue-demo

The steps we used to recreate the issue are:

1. clone repo
2. npm install && bower install
3. ember s
4. With js console open go to /random-route
5. Click on the name of a person
6. Press the like to go to page two
7. Press link to go to page one

Expected no errors in the web browsers console
Result: Error in the console.

"Error
    at assert (http://localhost:4200/assets/vendor.js:16266:13)
    at Object.assert (http://localhost:4200/assets/vendor.js:27202:34)
    at Meta.(anonymous function) [as writableChainWatchers] (http://localhost:4200/assets/vendor.js:29618:24)
    at addChainWatcher (http://localhost:4200/assets/vendor.js:26284:7)
    at ChainNode (http://localhost:4200/assets/vendor.js:26337:7)
    at ChainNode.chain (http://localhost:4200/assets/vendor.js:26436:30)
    at ChainNode.chain (http://localhost:4200/assets/vendor.js:26445:14)
    at ChainNode.add (http://localhost:4200/assets/vendor.js:26409:12)
    at ChainNode.copy (http://localhost:4200/assets/vendor.js:26395:13)
    at Meta.(anonymous function) [as writableChains] (http://localhost:4200/assets/vendor.js:29643:75)"

Cheers

[chadhietala]

@krisselden There appears to be an issue where we are trying to install a chain watcher on a destroyed object. I tried it out on Canary as well and saw the same error.

[rwjblue]

In Ember 2.10 the way destroyed objects meta is handled was changed.

In < 2.10 when an object was destroyed the meta is removed (by setting it to null). This caused a number of issues because we would then recreate a meta for the destroyed object.

In 2.10+ we mark the object and its meta as destroyed but keep it around (preventing wasteful work during normal object destruction and allowing things like .get on a CP to continue to function properly). When this change was made, I added these assertions to prevent us from changing/mutating the meta object after it was destroyed (which seems to be the case here).

---

^ is mostly background info. I'll try to dig into the reproduction a bit more after I'm back from a family vacation on Tues.

[rwjblue]

OK, so I had a few minutes to play and was curious here. I do not think this is a bug in Ember at all, but it is a case of "singleton controller state" biting you.

Basic summary of what is happening:

1. Visit /random-route
   a. has this for its model hook: return this.store.query("person", {});
   b The above query triggers the application serializer's normalizeResponse to fire which contains store.unloadAll(primaryModelClass.modelName); (this destroys any existing instances of the type in question (person)).
2. Click name of person
   a. action in random-route.hbs sets the current person on the controllers internal (singleton) state
   b. Display persons name in template (and therefore add a watcher to ensure we rerender when needed)
3. Click page-two link which is a {{link-to pointing at /random-route-two
   a. has this for its model hook: return this.store.query("person", {});
   b. Causes the application serializer to call unloadAll again (same code as fired in /random-route visit)
4. Click page-one link (which goes back to /random-route)
   a. Attempt to render persons name stashed from 2.a above, however this object was destroyed by the application serializer in 3.a/3.b.
   b. :boom:

[dusanstanojeviccs]

Hello, thank you for your fast response, we use unloadAll so that if we have a customer editing a record, on route change, all our customers see updates, this was the recommended way in the Ember slack chanel. Could this problem be solved by using an intermediary component and avoiding the controller and then passing the whole model loaded to it and then having it contain the actual component we are passing selected "person" to?

[stefanpenner]

I guess the question is more, what is the best behavior:

a. treat a destroy object as missing from the template layer/binding system
b. error, and tell users they are trying to watch/observer an destroyed object

The later is the current (and my current preference) as it helps catch strange issues sooner.
It does mean those upgrading (with such issues) will be encounter the issue explicitly, rather then via heisenbug later.

[dusanstanojeviccs]

Hi @stefanpenner I think you are completely right and b makes more sense. So a solution in our case would be to completely avoid controllers and create one more level of components, correct?

[stefanpenner]

Could this problem be solved by using an intermediary component and avoiding the controller and then passing the whole model loaded to it and then having it contain the actual component we are passing selected "person" to?

If the state was stored on the component, and the unload occurs after the component is un-rendered. Yes that most likely (given the provided context) would insta solve the problem.

---

Persistent state on controllers can be done, but may be quite tricky. As the controllers are intended to be long/lived, state on them must be manually managed.

When possible, I try to only put state in places that live the same or shorter life them the state I an placing on it. If they live longer, extra care is required.

[dusanstanojeviccs]

Thank you, that makes perfect sense! I've tested and when saving state only in components (as opposed to the controller) the issue is gone. I've committed the change to the original issue repo in case someone else was using controllers to store state. Could we maybe make the error currently being thrown optional through some settings, since this will make transitioning to the next Ember version require a lot of refactoring?