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

Backward matching of the unlabeled trailing closure is deprecated #1518

Closed
3 tasks done
lexrus opened this issue Sep 18, 2020 · 2 comments
Closed
3 tasks done

Backward matching of the unlabeled trailing closure is deprecated #1518

lexrus opened this issue Sep 18, 2020 · 2 comments

Comments

@lexrus
Copy link
Contributor

lexrus commented Sep 18, 2020

Check List

Thanks for considering to open an issue. Before you submit your issue, please confirm these boxes are checked.

Issue Description

I was just compiling our code base with Xcode 12. I got a dozens warnings sort of:

Screen Shot 2020-09-18 at 3 06 31 PM

Seems like this new proposal trying to catch my eyes.
https://forums.swift.org/t/se-0286-forward-scan-for-trailing-closures/38529

Okay, I'm good to add the completionHandler: label. But after some "auto fix"s and "auto completion"s. Some API callings could be ambiguous to my colleagues, like:

imageView.kf.setImage(with: LocalFileImageDataProvider(fileURL: URL(string: "")!)) { (_, _) in
    // How do I know this is a progress block from this API perspective?
} completionHandler: { _ in

}

This might not be an issue with Kingfisher. But IMO some API improvements could be made.
At least we should prevent Xcode to automatically hide the progressBlock:.

I'd prefer something like what Alamofire did:

AF.download("")
    .downloadProgress { _ in

    }
    .responseJSON { _ in

    }

Regards.

@onevcat
Copy link
Owner

onevcat commented Sep 20, 2020

It is a quite annoying change in Swift 5.3, basically only for writing better SwiftUI code. For any other use cases, it is a bit of regression to apply the new trailing closure rule.

For you case, if you have both the progress block and completion block, try to explicitly name the first block to avoid the ambiguous meaning:

imageView.kf.setImage(
    with: LocalFileImageDataProvider(fileURL: URL(string: "")!),
    progressBlock: { (_, _) in

    }
) { _ in

}

Or even write out them all:

imageView.kf.setImage(
    with: LocalFileImageDataProvider(fileURL: URL(string: "")!),
    progressBlock: { (_, _) in

    },
    completionHandler: { _ in

    }
)

The factory pattern is very nice in Alamofire and it definitely a good direction to improve the API. But it requires a major upgrade and I will consider if it is easy enough to migrate. I will try to see it I can implement it soon. Thank you for your suggestion.

@onevcat
Copy link
Owner

onevcat commented Jan 1, 2021

Done and fixed.

@onevcat onevcat closed this as completed Jan 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants