Fixed: completeBatchFetching is called on a background thread #731
Conversation
…round thread when no items are added to the collection
|
|
| @@ -67,7 +67,9 @@ final class PhotoFeedModel { | |||
| private func fetchNextPageOfPopularPhotos(replaceData: Bool, numberOfAdditionsCompletion: @escaping (Int, NetworkingErrors?) -> ()) { | |||
|
|
|||
| if currentPage == totalPages, currentPage != 0 { | |||
| return numberOfAdditionsCompletion(0, .customError("No pages left to parse")) | |||
| DispatchQueue.main.async { | |||
| return numberOfAdditionsCompletion(0, .customError("No pages left to parse")) | |||
There was a problem hiding this comment.
You can remove the "return" here.
| return numberOfAdditionsCompletion(0, .customError("No pages left to parse")) | ||
| DispatchQueue.main.async { | ||
| return numberOfAdditionsCompletion(0, .customError("No pages left to parse")) | ||
| } |
There was a problem hiding this comment.
You need to add a "return" here otherwise it would execute this path although it should return like it did before.
|
Does this bug also exist in the Objective-C version? Also, are all the ASBatch calls on ASCollectionNode behaving correctly, and only communicating with the delegate on main? |
|
@maicki I made those changes. Sorry about the delay; I need to tweak my Github notifications. I found the same issue in the Objective-C version (95% sure...I'm not very experienced with Objective-C) and fixed it. @appleguy I'm not sure if all the examples are correctly communicating with the delegate, if that's what you mean. |
There was a problem hiding this comment.
This seems reasonable. Thanks! Note @appleguy the collectionNode:willBeginBatchFetchWithContext: is documented to be called off-main. Although it would make a lot of sense to have specified it to be main thread and warn the user not to do too much.
Fix header
…eGroup#731) * Fixed breaking issue where completeBatchFetching is called on a background thread when no items are added to the collection * Changed spaces to tabs for consistency * Moved return statement for Code Review feedback * Fixed the same issue in the Objective-C version of ASDKgram * One more * Update PhotoFeedModel.m Fix header
When there are no additions to the collection,
numberOfAdditionsCompletion(and thereforecompleteBatchFetching) is called on a background thread instead of the main thread.This issue has not come up in this example because the user would have to scroll literally 1000 pages in order to experience it. This is important to fix to maintain consistency in the examples.