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

[Help wanted]SDWebImage 6.0 Proposal: Rewriten Swift API with the overlay framework instead of Objective-C exported one #2980

Open
dreampiggy opened this issue Apr 10, 2020 · 29 comments
Assignees
Labels
important proposal Proposal need to be discussed and implements
Milestone

Comments

@dreampiggy
Copy link
Contributor

dreampiggy commented Apr 10, 2020

Background

The current SDWebImage 5.0 Swift API is exported from Objective-C interface by clang importor. Which looks OK, but not really convenient compared with other pure Swift API. For example:

  • Objective-C API:
[imageView sd_setImageWithURL:url placeholderImage:nil options:0 context:@{SDWebImageContextQueryCacheType: @(SDImageCacheTypeMemory)} progress:nil completion:^(UIImage *image, NSError *error, SDImageCacheType cacheType, NSURL *url) {
  // do something with result
    if (error) {
        NSLog(@"%@", error);
    } else {
       // do with image
    }
}];
  • Swift API:
imageView.sd_setImage(with: url, placeholderImage:nil options:[] context:[.queryCacheType : SDImageCacheType.memory.rawValue] progress:nil) { image, error, cacheType, url in
  // do something with result
  if let error = error {
      print(error)
  } else {
      let image = image!
  }
} 

This looks not so Swifty in Swift world. A better design should looks like this, by using the powerful of Swift Language syntax, including:

  • Default Params
  • Enum with Associated Object
  • Result Type
  • The polished Swift API:
imageView.sd.setImage(with: url, options: [.queryCacheType(.memory)] { result
    switch result {
    case .success(let value):
        let image = value.image
        let data = value.data
    case .failure(let error):
        print(error)
    }
}

Note: The options arg is a enum with associated object, which combine the SDWebImageOptions and SDWebImageContext together, don't need to separate. Objective-C API has to use two type because of the pool C enum limitation to bind to Int value.

Solution

To achieve this goal of polished Swift API, we have some choices.

Modify the Objective-C source code interface with NS_SWIFT_NAME

This can solve the cases like renaming, for example: SDImageCache can renamed into SDWebImage.ImageCache, which drop the prefix of SD

However, this can not solve the issue like SDWebImageOptions + SDWebImageContext into the new API SDWebImage.ImageOptions enum cases.

Create another framework (overlay) and rewrite API

See: https://pspdfkit.com/blog/2018/first-class-swift-api-for-objective-c-frameworks/
See: https://forums.swift.org/t/how-apple-sdk-overlays-work/11317
See: https://github.com/apple/swift/blob/06685bcb516942d6b4ae2cb0c6be5ce324029898/stdlib/public/Darwin/Network/Network.swift

This can be done like this: Create a new framework called SwiftWebImage (naming can be discuessed), which have a source files named SDWebImage.swift and have the following contents:

@_exported import SDWebImage

Then, maked all the public API in SDWebImage as @unavailable, like this:

@available(*, unavailable, renamed: "SwiftWebImage.ImageOptions")
public struct SDWebImageOptions {}

@available(*, unavailable, renamed: "SwiftWebImage.ImageOptions")
public struct SDWebImageContext {}

Finally, implements the overlay logic by calling the original SDWebImage API, like this (the sd wrapper is from Kingfisher's design, actually the same) :

/// Wrapper for SDWebImage compatible types. This type provides an extension point for
/// connivence methods in SDWebImage.
public struct SDWebImageWrapper<Base> {
    public let base: Base
    public init(_ base: Base) {
        self.base = base
    }
}

extension SDWebImageCompatible {
    /// Gets a namespace holder for SDWebImage compatible types.
    public var sd: SDWebImageWrapper<Self> {
        get { return SDWebImageWrapper(self) }
        set { }
    }
}

extension UIImageView : SDWebImageCompatible {}

public protocol ImageResource {}

extension URL : ImageResource {}

extension SDWebImageWrapper where Base: UIImageView {
    @discardableResult
    public func setImage(
        with resource: ImageResource?,
        placeholder: UIImage? = nil,
        options: ImageOptions? = nil,
        progress: LoaderProgressBlock? = nil,
        completion: ((Result<ImageResult, ImageError>) -> Void)? = nil) -> CombinedOperation? {
            // convert the `ImageOptions` into the actual `SDWebImage` and `SDWebImageContext`
            // finally call the exist `sd_setImage(with:) API
        }
}

Done. When you import the SwiftWebImage framework, your original SDWebImage old Swift API will be marked as unavailable, so you can enjoy the new API.

import SwiftWebIamge
import SDWebImage // This will be overlayed and not visible, actuallly you don't need to import this

Overlay framework naming

Question: For Apple Standard Framework like Network.framework, the Swift Runtime provide a overlay framework which module name is the same as Network. So if you write :

import Network

You actually don't import the Network.framework, but import the libSwiftNetwork.dylib and its module.

import SwiftNetwork // Actually what you do
// The libSwiftNetwork has this:
@_exported import Network

@available(*, unavailable, renamed: "Network.NWInterface")
typealias nw_interface_t = OS_nw_interface

public class NWInterface {
    // ...Call C API for internal implementations
}

We want to adopt this design as well, so that you don't need to replace the import SDWebImage into import SwiftWebImage. However, due to the reality that we support 3 different package manager:

  • CocoaPods: Available to do so by using the custom prepare_script script phase to copy the module name
  • Carthage: Available to do so by using the custom Build Phase in Xcode Project
  • SwiftPM: Not available to do so because you can not declare two framework with same module name :)

And, for exist SDWebImage 5.0 user, if he/she really don't want to update to the new Swifty API, they can still use import SDWebImage to use the old Objective-C exported API. Naming the overlay framework different from SDWebImage can allows for this choice.

Which means: there are two types of Swift API, depends on how you import:

  • Don't use overlay framework
import SDWebImage

let imageCache = SDImageCache.shared
imageView.sd_setImage(with: url, options: [], context: [.imageScaleFactor : 3])
  • Use overlay framework
import SwiftWebImage

let imageCache = ImageCache.shared
imageView.sd.setImage(with: url, options: [.scaleFactor(3)])

Rewrite SDWebImage totally with Swift and drop Objective-C user

As my personal option, this is not always a good idea. There are already some awesome Swift-only Image Loading framerwork for iOS community:

We have some common design and solution for the same thing. Rewrite entirely need some more timeing and unit testing to ensure function.

And, SDWebImage 5.0 current is still popular in many Objective-C project and users, expecially in China. Nearly 80% users from Objective-C use it in the company's App (not personal App). And most of them have a codebase which mixed the two language. Drop these Objective-C user need to be carefuly considerated.

And I think: Currently, Objective-C and Swift is a implementation details, the benefit from Swift written in implementation may including:

  • Thread Safe: I disagree with this. The thread safe issues exists in Kingfisher and Nuke, which is not what a language can solve. But Swift have strict optiona type to avoid some common mistake like null check.
  • Performance: However, the Image Loading framework performance is not becaused of Objective-C runtime message sending architecture, it's because of some queue dispatch issue, or Image Decoder code. Both of them can not been solved by Swift language level.
  • Maintainess: This is the main reason. The new iOS programmer now have less knowledge about Objecitive-C practice and best coding style, using Swift in implementation can attract better contribution from new users.

So, my personal idea for SDWebImage 6.0.0, it's that we only rewrite the API level for Swift, still using Objecitive-C for internel implementation. Both Swift and Objective-C shall share the same function and optimization from version upgrade.

@dreampiggy dreampiggy added the proposal Proposal need to be discussed and implements label Apr 10, 2020
@dreampiggy dreampiggy added this to the 6.0.0 milestone Apr 10, 2020
@dreampiggy
Copy link
Contributor Author

This is the draft proposal. I need some SDWebImage's Swift user about the design and idea for this. You can leave comment here about this.

The SDWebImage 6.0 development is this 2020 year's goal.

@dreampiggy
Copy link
Contributor Author

dreampiggy commented Apr 12, 2020

We can learn more from Facebook iOS SDK what they do: https://github.com/facebook/facebook-ios-sdk

It create a separate Swift Module(target) for Swift user, new naming and Swift Helper code.

I think, SDWebImage can be as complicated as Facebook SDK if it grows up. Re-write entirely using Swift does not help for anything (either on performance, or safety), because both of them already solved by using the lowever C API and thread safe lock (Which Swift can not magically solve).

The main pros is that Swift become more attractive for beginner iOS developer. However, for decoding && transformer && code which need the deep domain knowledge, is not easy for beginner iOS developer to write. Re-write them using Swift is as ugly as Objective-C 😅

So, provide a Swift friendly API is much more meaningful than totally rewriten by copying line to line in Swift.

@bpoplauschi
Copy link
Member

@dreampiggy Facebook used to have a separate Swift wrapper around their Objc-C SDK and they have deprecated this Swift wrapper: https://github.com/facebookarchive/facebook-swift-sdk/blob/master/Announcement.md

My 2 cents:

  • we have a solid Obj-C component with a long history of fixes and improvements. Writing up a Swift framework from 0 would take a while, there's no real way to port over all those logical pieces of code from Obj-C and I don't know why someone would migrate over to that (there are already Swift-only proven image caching libraries out there).
  • I think a lot of our users are still using SDWebImage (even if they write Swift) because it's a proven and reliable library which still performs well even if written in Obj-C, so they don't see a reason to change
  • I agree adding a Swift module on top of our current API will probably help people.
  • perhaps we can write up an interface for that Swift API before we actually start coding it, so we can get feedback.

@dreampiggy
Copy link
Contributor Author

dreampiggy commented Apr 14, 2020

Yes. I think it does not help so much if we rewrite SD with Swift. None of the things changed in performance if you're a SDWebImage users. Only change the way of the SDWebImage developer and future maintainer. Framework user and framework author are different.

My proposal is simple for user:

  1. If you have import SwiftWebImage, you loss the ability to use UIImageView.sd_setImage(with:) and all SD prefix API like SDImageCache.queryCacheOperation(key:options:context:cacheType:done:). You must use UIImageView.sd.setImage(with:) and things like ImageCache.queryImage(key:options:completion:) instead. No prefix
  2. If you still think the Old 5.0 SD prefixed API is OK, you can upgrade to use it with only import SDWebImage.

Which means, from 6.0, we have 3 type of users:

  1. Swift user who use SwiftWebImage polished Swift API
  2. Swift user who use SDWebImage old Objective-C generated Swift API
  3. Objective-C user who use SDWebImage Objective-C API

@dreampiggy
Copy link
Contributor Author

dreampiggy commented Apr 14, 2020

If someone agree that it's hard to maintain such 3 types of combination is a good way out. We can simply to remove the support for type 2 user. All SwiftUI have to use new API instead. This is what Facebook SDK team they choose to do.

@looseyi
Copy link

looseyi commented Apr 17, 2020

Overlay framework naming, i thought is better.
We can base Objc's framework and add independent SD swift's podSpec, which wrap current‘s API

@bpoplauschi
Copy link
Member

@dreampiggy I think it can become a bit difficult to maintain 2 sets of Swift APIs.
I would consider making all the ObjC APIs unavailable directly to Swift (I think we used NS_REFINED_FOR_SWIFT for this).
Or is it better to keep the ones we have and just add that extra framework SwiftWebImage on top of what we have? I'm not sure at this point.
Also something to consider would be what happens if you had an import SDWebImage from earlier versions and you now do import SwiftWebImage, would you be able to access both APIs?
From that point of view, I think disabling the old interface (the one from ObjC) would make the upgrade straight forward.

I would like to hear some more opinions here @SDWebImage/collaborators

@dreampiggy
Copy link
Contributor Author

Or is it better to keep the ones we have and just add that extra framework SwiftWebImage on top of what we have? I'm not sure at this point.

Not a extra git repo. One git repo, but have to be 2 Xcode target. This is because of suck of SwiftPM.

See: https://medium.com/@joesusnick/swift-package-manager-with-a-mixed-swift-and-objective-c-project-part-1-2-19fbb43d0460

@dreampiggy
Copy link
Contributor Author

dreampiggy commented Apr 17, 2020

For CocoaPods, we can choose to use Subspec, or create one new Podspec for this. The difference is how user import.

# SDWebImage.podspec
s.subspec `Core` do |ss|
    ss.sources_files = 'SDWebImage/Classes/**/*'
end

s.subspec `Swift` do |ss|
    ss.sources_files = 'SDWebImage/Swift/**/*'
end

or

# SwiftWebImage.podpsec
s.sources_files = 'SDWebImage/Swift/**/*'

You can check Facebook SDK again, see what they do: https://github.com/facebook/facebook-ios-sdk/blob/master/FacebookSDK.podspec

They choose the seond one. (FacebookSDK module is for Swift user, and FBSDKCoreKit is for Objective-C user)

My idea: If we use subspec (the first approach), user from SwiftPM can use

import SDWebImage // They can import, because SwiftPM have two targets
import SwiftWebImage

However, user from CocoaPods have to use

import SDWebImage
// import SwiftWebImage // They can't, beucase there are no SwiftWebImage module. The `SDWebImage/Swift` subspec module name is still `SDWebImage`

This is because CocoaPods does not allow subspec to override the s.module_name property. see: CocoaPods/CocoaPods#3979

Which may cause confusing, because now they are different thing. Swift standard library use the same name to acutaly overlay the original one (Their SwiftNetwork module name is actually Network, and overlay the exist one from system). But we can't

@dreampiggy
Copy link
Contributor Author

dreampiggy commented Apr 17, 2020

Seems Facebook team assume : Objective-C user have to use CocoaPods, don't try to use SwiftPM. See the https://github.com/facebook/facebook-ios-sdk/blob/master/Package.swift#L30-L46

Their SwiftPM only export the things FacebookSDK(like us's SwiftWebImage overlay framework), not the original FBSDKCoreKit (like our SDWebImage original framework)

This is not what I want and disagree with. Swift Package Manager does allows distribution of Objective-C/C/C++ code, it's not any reason to disable Objective-C user from using it.

@dreampiggy
Copy link
Contributor Author

They mention this for Objective-C and Swift user, to use the different import:
https://developers.facebook.com/docs/ios/getting-started

image

@bpoplauschi
Copy link
Member

Like I said, Facebook deprecated their separate Swift framework, as it creates extra confusion (and other reasons).

Also, I think adding the Swift API in the form of a separate framework that requires an extra import is just overhead, especially for people that use CocoaPods or Carthage with default settings (which mean building frameworks as dynamic). Most of the users fall into those categories and an extra framework means a slower startup time.

So my suggestion is:

  • let's use Facebook's model: add some Swift files to our existing repo and existing targets, expose them via CocoaPods as a Swift subspec

@dreampiggy
Copy link
Contributor Author

dreampiggy commented Apr 19, 2020

Like I said, Facebook deprecated their separate Swift framework, as it creates extra confusion (and other reasons).

SDWebImage and SwiftWebImage are not separate framework. They are same one. Swift user's our framework module name is SwiftWebImage, Objective-C user's module name is SDWebImage.

let's use Facebook's model: add some Swift files to our existing repo and existing targets, expose them via CocoaPods as a Swift subspec

Maybe we can't, after I re-think the case again.

The SwiftWebImage must exist, for Swift Package Manager.

SwiftPM will become the main iOS package manager in the next few years, all of CocoaPods && Carthage will disappear in history. We have to support it.

SwiftPM don't use subspec, and I don't think CocoaPods subspec is correct. Subspec is the tool about combination related components, but not about components which have strict dependency. The Swift dependends Core, but they does not share anything, it's better to create a new Podspec instead, but keeping in same git repo to make it easy to merge and commit at the same time.

So, we should place Two Podspec in SDWebImage single git repo, and also, one Package.swift contains two target.

For both CocoaPods or SwiftPM user, they should use the same import syntax, just

import SwiftWebImage

The @import SDWebImage is for Objc user, not Swift user.

If Swift user really do this import SDWebImage, they loss some polished API (but not all of them). For example, you still call ImageCache instead of SDImageCache, but you can not use imageView.sd.setImage(with:) and only old imageView.sd_setImage(with:) exists.

It just like Swift user import FBSDKCoreKit instead of FacebookSDK, you can have a try yourself as well to see the difference.

@dreampiggy
Copy link
Contributor Author

dreampiggy commented Apr 19, 2020

All this complicated issue is because of this limit:

Swift Package Manager each Target (A module, actually) can only contains single language, either C/C++, Objective-C, or Swift. You can not mixed them.

However, standard Xcode Target (what Carthage/CocoaPods generated one), can contains mixed language without limit.

See: https://developer.apple.com/documentation/xcode/creating_a_standalone_swift_package_with_xcode
https://forums.swift.org/t/using-a-swift-package-in-a-mixed-swift-and-objective-c-project/27348

This current limit makes it's not possible to add some polished API (need pure Swift code, but not only some NS_SWIFT_NAME attributes in Objective-C header) in a single SDWebImage module.

image

SwiftPM source code about this: https://github.com/apple/swift-package-manager/blob/master/Sources/PackageLoading/TargetSourcesBuilder.swift#L138-L141

@dreampiggy
Copy link
Contributor Author

dreampiggy commented Apr 19, 2020

Let's assume if we follows your suggestion to use CocoaPods's subspec:

  • CocoaPods
    CocoaPods use two subspecs, Core for pure Objective-C code with some NS_SWIFT_NAME, Swift for pure Swift overlay by polish the API, and depdency the Core. CocoaPods will only generated one Target (Module)

CocoaPods user have to use SDWebImage new API using:

import SDWebImage

imageView.sd.setImage(with: buffer, options: [.priority(.high)]) { result
    switch(result) {
        //...
    }
}
  • Swift Package Manager
    Swift Package Manger have to use two Target (Module), one named SDWebImage, one named SwiftWebImage.

SwiftPM user have to use SDWebImage new API using:

import SwiftWebImage

imageView.sd.setImage(with: buffer, options: [.priority(.high)]) { result
    switch(result) {
        //...
    }
}

See ? The problem is : For end user, now I have to write different code to import the same framework, just because of the Package Manager I choose.

  • For framework author who dependends SDWebImage

Things also applied for our downstream dependency, like SDWebImageSwiftUI. Because SDWebImageSwiftUI supports both CocoaPods and SwiftPM as well.

Now I have to write like this:

#if canImport(SDWebImage)
import SDWebImage
typealias SDModuleName = SDWebImage // Have to do so
#endif
#if canImport(SwiftWebImage)
import SwiftWebImage
typealias SDModuleName = SwiftWebImage // Have to do so
#endif

public struct AnimatedImage: UIVIewRepresentable {
    typealias UIViewType = SDModuleName.AnimatedImageView // Really tricky
}

As the author and maintainer of SDWebImageSwiftUI, I don't think it's a good solution, which make things more complicated for maintain. Each Package Manager should use the Same Code to import and use for end-user.

@dreampiggy
Copy link
Contributor Author

dreampiggy commented Apr 19, 2020

Another framework to reference about Swift API transition: PSPDFKit

They used to have true overlay framework, but now merged into it's own mainstream codebase.

Note: They support Carthage/CocoaPods, but not SwiftPM. They put the mixed Objecitive-C and Swift code into a single framework target.

This, of cource, will force the Objecitive-C only client have to ship with Swift Runtime binary 7MB size, which it's not good. I don't think pure Objecitve-C user must make their ipa size become bigger because someone supports better Swift API, they don't gain what they paid.

See: https://pspdfkit.com/guides/ios/current/migration-guides/pspdfkit-93-migration-guide/#swift-only-changes

@dreampiggy
Copy link
Contributor Author

dreampiggy commented Apr 19, 2020

Maybe, we can just mixed coding with Objc and Swift into a single "Target". No actual "overlay".

Put "Overlay" on logic, but not actual a target. The Swift and Objc source code is mixed into compile unit and produce single Mach-O Library.

Which means, the Package.swift may looks like:

.library(name: "_SDWebImageCore", dependencies: [], path: "SDWebImage", sources: ["Core", "Private"]) // implements details, Objc code only
.library(name: "SDWebImage", dependencies: ["_SDWebImageCore"], path: "Swift")

And, for CocoaPods, there are no subspec. Just put all source code of Objc/Swift into s.source_files = '*.{h,m,swift}'

I'll have a try tomorrow.

@phoney
Copy link

phoney commented Apr 23, 2020

This all sounds great. I'm a swift user. Once there is a swifty API I won't have any interest in the clang imported Obj-C -> swift API. I don't see any benefit to maintaining that if that causes any extra development time or test time.

I currently use Cocoapods and would like to move to using SPM but haven't yet. The SPM team does read and respond to forum posts at forums.swift.org so one can post feature requests there and pain points.

@bpoplauschi
Copy link
Member

bpoplauschi commented Apr 24, 2020

Great @phoney

The thing is SPM does gain traction and more and more users will be migrating to it as a default dependency management system.
But I don't see CocoaPods going away anytime soon, because:

  • they have a huge amount of specs that make the number of libs you can install via CP way bigger than what SPM has and will have, cause inactive projects are not adding SPM
  • also, for a repo out there (open source without any CP or SPM support), it's a lot easier to write it a podspec if it doesn't have it and add it to your project
  • SPM is great for straight-forward cases where you just want to bring that dependency, but in more complicated cases where you need customization, CP offers a good amount of control (with their pre_install, post_install hooks and more). Cases where an open source is missing a SWIFT_VERSION or needs some extra Xcode settings.
  • with CP you have control over how the dependencies will be linked to your app (static libs, static frameworks, dynamic frameworks), where SPM only does static linking (from what I know).

Given all those reasons and more, I think CocoaPods will be here for a few more years at least, so our goal as open source contributors is to support SPM and CocoaPods in equal measures, one can't take precedence over the other. Not to mention Carthage is also a compatibility I would aim to keep. They just give people more options.
Worth mentioning there are still people that install SDWebImage via the old git clone ... :)

@stale
Copy link

stale bot commented Jun 23, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue, please make sure it is up to date and if so, add a comment that this is still an issue to keep it open. Thank you for your contributions.

@stale stale bot added the stale label Jun 23, 2020
@dreampiggy dreampiggy removed the stale label Jun 23, 2020
@stale
Copy link

stale bot commented Aug 22, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue, please make sure it is up to date and if so, add a comment that this is still an issue to keep it open. Thank you for your contributions.

@willbattel
Copy link

willbattel commented Jun 2, 2022

Assuming this improved Swift API is still on the table, I wanted to request structured-concurrency equivalents to asynchronous methods in SDWebImage.

For each current method that takes a completion handler closure, we would love to have an async counterpart available to developers using the new structured concurrency features of Swift.

For example, this method call:

imageManager.loadImage(with: url, options: .retryFailed, progress: nil) { image, _, error, cacheType, _, _ in
    if let error = error {
        // do something after error
    }
    else {
        // do something after success
    }
}

would have an async twin callable with a try await pattern like this:

do {
    let (image, _, cacheType, _, _) = try await imageManager.loadImage(with: url, options: .retryFailed, progress: nil)
    // do something after success
}
catch {
    // do something after error
}

For the time being, Swift developers can use the new CheckedContinuation mechanism to effectively bridge the old behavior to the new behavior, like so:

func loadImage(url: URL) async throws -> UIImage {
    typealias ImageContinuation = CheckedContinuation<UIImage, Error>
    return try await withCheckedThrowingContinuation { (continuation: ImageContinuation) in
        imageManager.loadImage(with: url, options: .retryFailed, progress: nil) { image, _, error, _, _, _ in
            if let image = image {
                continuation.resume(returning: image)
            }
            else {
                continuation.resume(throwing: error!)
            }
        }
    }
}

Let me know if you would prefer I make a new issue for this specific request.

https://docs.swift.org/swift-book/LanguageGuide/Concurrency.html
https://developer.apple.com/videos/play/wwdc2021/10132
https://developer.apple.com/videos/play/wwdc2021/10134

@dreampiggy
Copy link
Contributor Author

dreampiggy commented Jun 7, 2022

Seems too long after the first proposal...

My personal job and open-source work may related to some other compiler-based stack (not iOS UI level) 😓

But it's still really welcomed to achieve this for any interested community contributors and users 👋

For example, if you think this is useable, you can drive some sample about the new API looks like, and some implementation can be easily achive via overlay framework tech I talked above.

Or, we can make it two-way to achieve:

  1. First try to write a separate Swift-only framework (depends SDWebImage) which provide extensions and renaming the classed with dropped prefix. Like SDWebImageSwift. You can create a new GitHub repo about this.

  2. Second I try to integrate into the overlay framework solution with the created standalone SDWebImageSwift, and handling some infrastructure and project settings.

@dreampiggy dreampiggy changed the title SDWebImage 6.0 Proposal: Rewriten Swift API with the overlay framework instead of Objective-C exported one [Help wanted]SDWebImage 6.0 Proposal: Rewriten Swift API with the overlay framework instead of Objective-C exported one Jun 4, 2023
@dreampiggy
Copy link
Contributor Author

dreampiggy commented Jun 4, 2023

Recall with the roadmap to SDWebImage 6.0

See draft implementation in https://github.com/dreampiggy/SDWebImage/tree/swift

Which means: SDWebImage 6.0 will become Objective-C && Swift Mixed project

You can not use SDWebImage without linking the libswiftCore.dylib. And we will bump the minOS version to iOS 11+ to match Xcode 14's require.

Or, maybe Xcode 15's require after WWDC 23

@o-nnerb
Copy link

o-nnerb commented Jun 28, 2023

@dreampiggy I can work on this. I think the best way to achieve this is to start from scratch by bringing pieces of code to the Swift version, like Apple is doing with Foundation.

On a second guess, we can work on this as a fork, like you've already is doing 😅.

@o-nnerb
Copy link

o-nnerb commented Jun 29, 2023

To keep everybody updated, I'm working on it SDWebImage rewritten in Swift. Currently is 8.3%!

I think the best way is to deliver the code in Swift format first, run the tests and after start making improvements. I've done this in other private projects and it's very overwhelming adding new features while refactoring the code.

@dreampiggy
Copy link
Contributor Author

Hi.

Thanks for your support !!!

Is that rewritten in Swift means, the pure Swift implementation ? Or just the API refinement ?

Because at the original design or RFC, I want to support both Swift / Objc users of SDWebImage. So at that https://github.com/dreampiggy/SDWebImage/tree/swift, I only rewrite the API site using Swift, and keep the Objc core as usual (I can not use features which Objc don't support, like struct, like default params, like associated object enum)

@dreampiggy
Copy link
Contributor Author

dreampiggy commented Jun 30, 2023

Which means, we'd better, for example:

For this sd_setImageWithURL: API:

objc UIImageView.sd_setImageWithURL call `__sd_setImageWithURL` internal method
swift UIImageView.sd.setImage(with:) call `__sd_setImageWithURL` internal method
__sd_setImageWithURL is a global objc(or even C) method, which need to extract the Swift associated enum object from Swift, and convert into the internal form object, then dispatch to next job.

So, as you can see, both Swift/Objc users can still use their API (but which is designed differently for each languages and use the best features their languages support).

Just like Apple's Foundation, there are still Objc/Swift two APIs, but the Swift API looks Swifty on Swift site (we does not use the Objc generated Interface, but the interface written by our hand)

For class, actually we can hide the SDWebImageManager to Swift user, and create a new Class WebImageManager and itself has a wrapper property of the original SDWebImageManager. Or if the API does not need changes too much, using the SWIFT_CLASS_NAME to refine the name to Swift user without that wrapper.

For that stupid SDWebImageRequestModifier, which should be a Swift closure in Swift, we can use a proxy to do the similiar thing. (This was introduced because the NSMutableDictionary can not hold Swift closure bridging from Swift...😣)

@o-nnerb
Copy link

o-nnerb commented Jun 30, 2023

Hmm, I understand now the proposal. I didn’t read the details and assumed that this required the code being rewritten in Swift.

I think the idea of making methods exclusive for Swift just to make the API not objc style can be an unnecessary work. As you explain very well, this changes will deliver a new package that calls the objc code, but using Swift on the surface.

However, I'll search for more details to keep contributing with the original proposal.

Thanks for clarifying @dreampiggy! 🫶🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
important proposal Proposal need to be discussed and implements
Projects
None yet
Development

No branches or pull requests

8 participants