Swinject / SwinjectStoryboard

Swinject extension for automatic dependency injection via Storyboard
MIT License
268 stars 141 forks source link

Objective-C class method 'initialize', which is not guaranteed to be invoked by Swift and will be disallowed in future versions #45

Closed yoichitgy closed 6 years ago

yoichitgy commented 7 years ago

Posted by @mrkd: https://github.com/Swinject/Swinject/issues/238

Objective-C class method 'initialize', which is not guaranteed to be invoked by Swift and will be disallowed in future versions

Xcode - Version 8.3 (8E162), Swift 3.1

/Pods/SwinjectStoryboard/Sources/Storyboard+Swizzling.swift:25:30: Method 'initialize()' defines Objective-C class method 'initialize', which is not guaranteed to be invoked by Swift and will be disallowed in future versions

mpdifran commented 7 years ago

I think this can be fixed by migrating this extension to Objective-C. Are there any objections to that? I can take a crack at it.

charles-oder commented 7 years ago

wouldn't that kinda defeat the purpose of this being an injection framework for Swift?

mpdifran commented 7 years ago

The project already uses an Objective-C class for the base storyboard subclass. Rewriting the swizzle extension in Objective-C shouldn't affect the public interface either.

That is assuming this can be done this way; I've looked into it a bit, and it seems like the swizzle extension references some static Swift methods, and I can't import SwinjectStoryboard-Swift.h for some reason.

yoichitgy commented 7 years ago

I agree with @mpdifran's idea. Writing the extension in Objective-C sounds good. Originally I just tried write code in Swift as much as possible, but now I think writing the extension in Objective-C is simpler.

mpdifran commented 7 years ago

@yoichitgy I've got most of the work done, but I'm stuck with the import SwinjectStoryboard-Swift.h problem. Mind taking a look at my branch if I push it up?

Opened a WIP PR here: https://github.com/Swinject/SwinjectStoryboard/pull/46

yoichitgy commented 7 years ago

@mpdifran, I answered in the pull request๐Ÿ˜ƒ

Przemyslaw-Wosko commented 7 years ago

@yoichitgy @mpdifran any progress?

mpdifran commented 7 years ago

Waiting on @yoichitgy for advice on how to proceed for the last initialize() warning. I'll update to remove the merge conflict though.

yoichitgy commented 6 years ago

v1.1.2 fixed the issue. Thanks @mpdifran ๐Ÿ‘๐Ÿ‘๐Ÿ‘