store.all() is not treated as a filteredRecordArray anymore #3167
Conversation
|
|
||
| if (this.liveRecordArrays.has(typeClass)) { | ||
| var liveRecordArray = this.liveRecordArrays.get(typeClass); | ||
| if (!liveRecordArray.isDestroyed && !liveRecordArray.isDestroying) { |
There was a problem hiding this comment.
The check for the record array not being destroyed is needed as otherwise this test fails.
I am not satisfied about this check; how should this be tackled? Check if the RecordArray which is about to be destroyed is a liveRecordArray and if so, remove it from the map?
There was a problem hiding this comment.
it seems like previously it was handled by unregisterFilteredRecordArray, called when in the recordArray's destroy method.
Maybe this could be rename as unregisterRecordArray which could handle both filtered/live record arrays ?
| return this.liveRecordArrays.get(typeClass); | ||
| }, | ||
|
|
||
| /** | ||
| Create a `DS.RecordArray` for a type and register it for updates. |
There was a problem hiding this comment.
This comment needs to be updated since the register it for updates is no more valid ...
|
I refactored the whole thing a little more and kept the commits so the changes are more comprehensible. |
| if (!recordArrays.has(array)) { | ||
| array.addRecord(record); | ||
| recordArrays.add(array); | ||
| updateLiveRecordArray: function(array, modelName) { |
There was a problem hiding this comment.
This doesn't really update anymore, just initially populates it, so a different name/location might be better
|
Can you squash please |
| var typeClass = array.type; | ||
|
|
||
| // unregister filtered records arrays | ||
| var recordArrays = this.filteredRecordArrays.get(typeClass); |
There was a problem hiding this comment.
Seems like we could just know which one it is, and don't need to try both?
|
Thanks @pangratz ! |
This refactors the handling of the record array returned by `store.all()`. Previously it has been treated as a FilteredRecordArray without a filter function which resulted in awkward code paths. Now there is a list of liveRecordArrays which contains the RecordArrays which hold all records for a type.
|
This looks like it makes it so if you call |
|
Looks like it. 👍 |
store.all() is not treated as a filteredRecordArray anymore
|
@fivetanley that was the case before as well,this just refactors the code |
|
@pangratz Great Work !! Also next time instead of |
@sly7-7 I used to do this in my commit messages but I started to refrain from this since otherwise the referenced issue is cluttered with I should get into the habit of adding the |
|
|
This addresses #2542
I haven't squashed the commits so the changes are more comprehensible. If this is accepted, I will squash em all!