[bugfix beta] Fetch cancels unload #5376
Conversation
addon/-private/system/store.js
Outdated
| @@ -921,6 +925,9 @@ Store = Service.extend({ | |||
| // will once again convert the records to snapshots for adapter.findMany() | |||
| let snapshots = new Array(totalItems); | |||
| for (let i = 0; i < totalItems; i++) { | |||
| if (internalModels[i].hasScheduledDestroy()) { | |||
| internalModels[i].cancelDestroy(); | |||
There was a problem hiding this comment.
Why are we canceling in two places? Does the test catch both instances where canceling is required?
There was a problem hiding this comment.
per OOB discussion, moving all cancelation to flush
hjdivad
force-pushed
the
hjdivad/cancel-unload-on-fetch
branch
from
1ae5181 to
e9441a5
Compare
When fetching records, cancel internal model destruction if it is
scheduled, ie if the model is dematerializing because it was unloaded.
It is not possible to construct snapshots for dematerializing internal
models. Prior to this commit, this could happen when fetching a record
in the same runloop that it was unloaded. A straightfoward way of
getting into this state was via
```
store.findRecord('book', 1).then(b => b.unloadRecord)
```
when the model is already cached in the store. Under these conditions,
the fetch is scheduled, then the promise fulfills with the cached record
and is unloaded and *then* the scheduled fetch is flushed.
[fix emberjs#5343]
hjdivad
force-pushed
the
hjdivad/cancel-unload-on-fetch
branch
from
e9441a5 to
7444444
Compare
|
@bmac A very kindlyi-smoothy-loving ping to ship a new beta... 😺 |
|
@sly7-7 I just published v3.1.0-beta.2. |
|
Thank you So much @bmac |
|
@bmac thank you this also fixes the same issue we were having! |
|
I just tested this on a |
|
Please release 2.18.4 ember data with this fix. 2.18 is still in LTS branch and we need this fix |
|
@aalasolutions - FWIW, there have not been any ember-data versions that are LTS. The only project that has released any LTS versions to date has been Ember itself (ember-source npm package). |
|
@rwjblue hmm, right. meanwhile I have totally ditched unloadRecord and in my component, i filtered the records which are not required. So I guess I can live with that for now |
|
I just cherry-picked this commit to the 2.18 branch and release Ember Data 2.18.4. (Sorry @rwjblue I should have pinged you to give you a heads up first). |
When fetching records, cancel internal model destruction if it is
scheduled, ie if the model is dematerializing because it was unloaded.
It is not possible to construct snapshots for dematerializing internal
models. Prior to this commit, this could happen when fetching a record
in the same runloop that it was unloaded. A straightfoward way of
getting into this state was via
when the model is already cached in the store. Under these conditions,
the fetch is scheduled, then the promise fulfills with the cached record
and is unloaded and then the scheduled fetch is flushed.
[fix #5343]