Data: Cannot update watchers for model after it has been destroyed

Created on 18 Mar 2018  ·  25Comments  ·  Source: emberjs/data

When exiting a route, I need a specific model to be unloaded from the store.

However, if I then try to refresh the route, I get the error

Cannot update watchers for `name…-model::ember323:1>` after it has been destroyed

Here is an example Twiddle:

https://ember-twiddle.com/dfff8b960e58063511927f2768dd7b7c?openFiles=routes.application.js%2C
(Trying to come up with a better example)

The only other mention to this error I found was https://github.com/emberjs/data/issues/5111. (But I don't know enough to say if it is related)

Needs Bug Verification

Most helpful comment

A work around would be really handy. It's weird issues like these that me really frustrated when using ember-data.

All 25 comments

@amk221 given the other issue you mentioned is resolved by #5378, could you test against that PR?

Using the latest ED still has the problem.

I've narrowed down the scenario a bit now (Twiddle)

@amk221 did you try against @runspired's branch runspired:fix-peek-all ?

I don't think it's fixed it for my use case. I set up an example repo, and yarn link'd fix-peek-all

It's interresting, it seems like there are some timing issues, or maybe a problem with the routing process.
When you hit refresh, it goes to the model hook, load Foo and bar (which takes 250ms), so goes to the loading route, so goes in the resetController which unloads foo. So it setupController with an unloaded record (isDestroyed is true), and it seems like ember (glimmer maybe) does not like that.

I tried to unload the record directly in the model hook, and return it. Weird enough then, the record is not isDestroyed.

  • I wasn't sure how to use runspired:fix-peek-all via package.json, which is why I explained above that I used yarn link (hence package.json appearing not to use the fixed branch)
  • Yes, it does seem like a timing issue - Although the code 'reads correctly', the hooks fire in an order which is counter to what you'd expect.
  • In my app, I worked around the issue by utilising beforeModel instead.

@amk221 could you elaborate on how you used beforeModel? I'm getting this exact same issue where I have a model hook that returns a hash, when transitioning out of the route (sibling/non-sibling) I unload a bunch of records from that page, then in some cases if I am transitioning to a sibling route e.g. /fooA -> /fooB, I get this error and I haven't been able to figure out why... thanks!

@erichonkanen Originally put the unloadRecord in resetController, so to reset things _after_ the route has been used. But, as this was causing the timing issue I flipped it - so that I do the resetting _before_ the route is entered... to the same effect. That's all!

Using ember 3.3.2 and ember-data 3.3.1, our app triggers the following assertion.

Error: Assertion Failed: Cannot create a new chain watcher for <li@model:xxxx::ember1696:yyyy> after it has been destroyed.

We are executing store.push in setupController and unloadAll on resetController, the exception appears often when entering the same page and the previous unloaded records are pushed again.

Our current workaround has been to use store.unloadRecord instead of store.unloadAll.

_Before_

  resetController: function(controller, isExiting) {
    this._super(...arguments);

    this.store.unloadAll('serie');
  }

_After_

 resetController: function(controller, isExiting) {
    this._super(...arguments);

    this.store.peekAll('serie').forEach((item) => {
      this.store.unloadRecord(item);
    });
  }

Hi,
Wanted to know if a solution is found to above issue. I am upgrading Ember-cli from 2.11.1 to 2.18.2 in my project with Ember data - 2.18, and there are lot of issues triggered due to unloadAll not clearing the record properly from store.

I'm having a similar problem here, reproduced in this twiddle:
https://ember-twiddle.com/d87770d65d2c92aecb89ececff2574f7?openFiles=templates.index.hbs%2C

Click "Add Inner" twice
Then click "Remove Last Inner" twice.
Click "Add Inner" again and you get
"Assertion Failed: Cannot update watchers for isActive on <twiddle@model:model-inner::ember185:null> after it has been destroyed."

I don't know how to use ember inspector with a twiddle, but in my app it looks like I'm getting PromiseManyArray with length 1 and no entries (or an undefined entry, hard to tell)

Also note that you can never unload more than two inners - it just keeps claiming to unload the same lastObject after that.

A work around would be really handy. It's weird issues like these that me really frustrated when using ember-data.

One weak workaround I've found (though still testing for success) is to not use Ember.computed.filterBy and similar 'reduce computed' array functions. Especially if the value you are sorting or filtering on doesn't change, then you can probably get away with something like:

Ember.computed('myHasMany.[]', function() {
  return this.get('myHasMany').filter(item => {
    return item && item.get('isActive')
  }
}

instead of:
Ember.computed.filterBy('myHasMany', 'isActive')
That way ember doesn't have to bind to the individual members of the hasMany, just to the array size, and you can check for empty values in your filter callback.

Edit: Of course, this doesn't work if the value you're filtering on can change - only if the value is constant and the containing objects are simply added to or removed from the array

So it turns out my problem was that I had an inverse relationship, and unloading a hasMany item of an inverse relationship doesn't unload cleanly. I overrode 'unloadRecord' of my hasMany target model to be the following:

  unloadRecord() {
    this.set('modelOutter', null);
    return this._super(...arguments);
  }

as shown in this updated twiddle
https://ember-twiddle.com/d87770d65d2c92aecb89ececff2574f7?openFiles=models.model-inner.js%2C

That seems to have solved my problem. Note that store.unloadRecord() simply calls model.unloadRecord() so it handles both situations. unloadAll does not use model.unloadRecord so may still cause the same problem, but it's a significantly different code path .

Facing lot of issues including watchers issue, cannot find a new tag once model is destroyed, hasMany/belongsTo relationships issue when unloadAll is being used for the model. Is there any release/fix version planned which will have a solution to above issues as workarounds tend to fail in some or another scenario.

Also would be really helpful, to have suggestions if anyone successfully downgraded to Ember data 2.12 with Ember-cli > 2.18.

I am able to reproduce this using @amk221's demo app against https://github.com/emberjs/data/commit/4c539f466a21e91499ff18ba0318827db1f27252.
Our workaround has been to do something like:

const list = this.store.peekAll('foo');
list && list.forEach(rec => rec.unloadRecord());

Note this workaround is the same as what @ppcano demonstrated above.

@amk221 thank you for posting this demo app; it was helpful demonstrating this actually is not an ember-data bug.
The reproduction linked above actually has a race condition that ultimately leads to the unloadRecord being called on the same reference to foo. As such, it is not an ember-data issue but rather route's loading and unloading data without properly cleaning up. Please see this example that corrects the timing error. The comments in the route explain how to toggle to a version of the demo with broken behavior. Additionally, you can see the same exception thrown in this updated example that does not use ember-data. Thanks to @runspired for pointing out the replication bug and walking me through in detail discord.

After having gone through the reproduction I'm going to close this issue as it seems fairly clear to me that at least in the reproduction the fault lies with app code.

The discord thread linked by @efx above in which I discuss this is a good starting point if you want a deeper explanation, but a couple of key take-aways:

  • Route.resetController occurs after the model() hook is called in a refresh, which means you are handling the new model.
  • record.unloadRecord() attempts to move the underlying state of a record into a "never loaded" state, which also means that the existing record given to the ui will be torn down (since we shouldn't have a record anymore). In this reproduction, the issue is primarily that unloadRecord is being called on the refreshed model (e.g. "reload" then "unload", vs the intent of "unload" then "reload")
  • any time you tell the store to unload a record, or you destroy a record, you should also eliminate any retained references to the record's class instance. In this example, controller.model.foo is a retained instance and since controller is a long lived singleton unloading controller.model.foo without then setting foo to null is a mistake: a mistake that happens to not matter if proper timing of requests results in replacing foo with a different record.

you should also eliminate any retained references to the record's class instance

This recommendation has fixed a case where we experienced this problem.

In an app with some more complexity, the mentioned workarounds aren't sufficient. Here is how I ended up fixing the issue:

    model(params) {

        return this.store.findRecord('mymodel', params.mymodel_id, { reload: true })
    resetController(controller, isExiting) {
        this._super(...arguments)

        controller.set('model', null)

I'm not sure why, it was a last-ditch attempt, but it works.

@Redsandro Did you do that instead of or in addition to using unloadAll/unloadRecord?

@e-monson To my own surprise, I did it _instead of_ and it worked.

Full disclosure: I added it in an attempt to fix my issue. Which worked. Then I removed all the fixes mentioned here. And it still worked. So the net result is the above.

I just want to chime in and say that this "Cannot update watchers" change is causing us a lot of pain during our upgrade attempt. I understand it's probably for the best and likely due to some bad practices on our end, but we're having a really hard time tracking down the exact causes of this in our code.

In our case, we have a bunch of nested routes, controllers, templates, and components _(legacy app 👍)_ that rely on the computed properties of a record that gets unloaded due to a web socket event.

We've ended up triggering a custom willUnloadRecord event on the ED record in question so that we can try to clean up any references before the record actually gets unloaded... but it's proving to be a lot of work and feels pretty hacky.

One thing that would make this easier to deal with is clear error messages that tell us exactly where we can find the offending watchers _(template/computed property)_. As it stands, we are just sifting through stack traces and commenting things out.

Feel for anyone else going through this - it's a real pain 😔

One thing that would make this easier to deal with is clear error messages that tell us exactly where we can find the offending watchers (template/computed property).

Oh yes please would love this. I have some small projects where I just couldn't find the cause, too. I've simply removed some functionality because I don't have time to do the trial and error. The null trick doesn't work in all cases.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

bartocc picture bartocc  ·  4Comments

kennethlarsen picture kennethlarsen  ·  3Comments

jherdman picture jherdman  ·  4Comments

toobulkeh picture toobulkeh  ·  3Comments

buschtoens picture buschtoens  ·  4Comments