Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

this.get('store').unloadAll(type) works only once #5447

Closed
Fabschu opened this issue Apr 23, 2018 · 11 comments
Closed

this.get('store').unloadAll(type) works only once #5447

Fabschu opened this issue Apr 23, 2018 · 11 comments
Assignees
Labels
Needs Bug Verification 🌲 Project Trim 🌲 PRs related to https://github.com/emberjs/data/issues/6166

Comments

@Fabschu
Copy link

Fabschu commented Apr 23, 2018

DEBUG: -------------------------------
index.js:158 DEBUG: Ember : 2.16.4
index.js:158 DEBUG: Ember Data : 2.16.4
index.js:158 DEBUG: jQuery : 2.2.4
index.js:158 DEBUG: Ember Simple Auth : 1.6.0
index.js:158 DEBUG: -------------------------------

When i use this.get('store').unloadAll(type) an refetch the same models with the same ids with the RESTAdapter again, this.get('store').unloadAll(type) fails removing them from the store.
Only model entries with an different id will be removed.

this.get('store').findRecord('product', 1) --> Product with id 1 is in the store.
this.get('store').unloadAll('product') --> No Product in the store
this.get('store').findRecord('product', 1) --> Product with id 1 is in the store.
this.get('store').findRecord('product', 2) --> Product with id 1 and 2 is in the store.
this.get('store').unloadAll('product') --> Product with id 1 is in the store.

@runspired
Copy link
Contributor

runspired commented Apr 23, 2018

@Fabschu

  • do you wait for the promises to resolve before unloading ?
  • Also, do these products have relationships?
  • interleaving unloadAll and findRecord is odd, what is the use case?

@Fabschu
Copy link
Author

Fabschu commented Apr 24, 2018

We´ve got something like a wishlist.
The user can add a product to the wishlist with an ajax post.
Then he opens the wishlist by changing the route and all products are loaded in the model hook with this.store.findAll('wishlist-product') using RESTAdapter.
Now the user can click a button "remove all items", a ajax delete post is made and in the then block we clear all wishlist-products with this.get('store').unloadAll('wishlist-product').
Then he changes the route, adds the same Product with the same id again, opens the wishlist, clicks the "remove all items" button again, this.get('store').unloadAll('wishlist-product') is called but the wishlist-product is still in the store.

All promises are resolved, each step is an userinteraction and not directly chained.
The Product has 2 belongsTo dependencies, without an inverse relation.

@patodevilla
Copy link

I am also experiencing this problem, I need to unload models when a user changes the "project" context of our app and it seems to work only once and even some models do not unload at all.

@nojacko
Copy link

nojacko commented Sep 5, 2018

I'm having this issue too.

  • In my willTransition action I call unloadAll('model-name')
  • First time, it unloads the models.
  • Subsequent times, it doesn't.
  • I've console.log'd the willTransition action and that is definitely called every time as expected.

@CezaryH
Copy link

CezaryH commented Dec 4, 2018

I've run in to similar issue.

Based on first post I've created twiddle:
https://ember-twiddle.com/4fc11e3dce9ecc0e1cfbbbb006ffa1a1?openFiles=routes.application.js%2C

Model returns one post model, but it's called after unloadAll, so it should be empty.

Interesting thing is if you comment out line 9 then it works as expected.

@geekygrappler
Copy link

geekygrappler commented Dec 14, 2018

I might be able to shed a little light on this. I don't know how to fix it, but I think I know why it's happening... I had the same problem on one of my apps.

I copied the twiddle into a repo to debug easier (thanks @CezaryH).
https://github.com/geekygrappler/unload-all-post-example

Ember data has an InternalModelMap which has two properties, _idToModel, _models:
https://github.com/emberjs/data/blob/master/addon/-private/system/internal-model-map.ts#L16-L17

What I found was that on the second call of unloadAll() these two properties get out of sync. You can see it in the example app if you break here:
https://github.com/emberjs/data/blob/master/addon/-private/system/store.js#L2141

in this case _idToModels will have both ids in it, but _models will only have the previously uncleared post in it, and unloadAll will loop over the _models array when clearing out the models.

Sorry I can't be more helpful, I don't know enough about the internals of ED to figure out why the properties get out of sync, or even where/how they're set up.

If it's of help to anyone my workaround was to call store.peekAll('model').map(model => model.unloadRecord());

I would be willing to investigate further if anyone more familiar with ED is able to give me some hints/tips/suggestions.

@urbany
Copy link

urbany commented Dec 20, 2018

I'm also seeing this in 3.6.0 thanks for the workaround @geekygrappler

@abankspdx
Copy link

I'm encountering this as well on 3.5.0. unloadAll has been broken forever - is there a plan for fixing?

@tschoartschi
Copy link

We experience the same problems 🤔 @runspired let me know if you need more information or if the information provided by the others are enough.

We worked around this issue the following way:

const all = this.store.peekAll(this.modelName);
all.forEach((modelInstance) => modelInstance.unloadRecord());

Nevertheless it's never nice to use a workaround 😉

@runspired
Copy link
Contributor

I looked into this and the issue is basically that you need to force the destroy queue to flush before you try to reference the model again (accessing it via peekAll counts as referencing it again). Accessing it again before the destroy prevents full teardown. Wrapping unloadAll in a run call will force the destroy to complete.

import { run } from '@ember/runloop';
// ...
run(() => store.unloadAll('post'));

That said we're working on making some improvements to these APIs that should result in destroy completing synchronously and unload always fully unloading. We're able to do this now because references to these records that we need to keep for relationships can be referenced by identifier instead of by InternalModel or RecordData instance.

@runspired runspired self-assigned this May 21, 2021
@runspired runspired added the 🌲 Project Trim 🌲 PRs related to https://github.com/emberjs/data/issues/6166 label May 21, 2021
@runspired
Copy link
Contributor

Completed with the removal of InternalModel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Bug Verification 🌲 Project Trim 🌲 PRs related to https://github.com/emberjs/data/issues/6166
Projects
None yet
Development

No branches or pull requests

9 participants