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

🧪 Delegate Proxy & Runtime hacking #21

Merged
merged 31 commits into from
Jul 8, 2020

Conversation

jdisho
Copy link
Contributor

@jdisho jdisho commented Jul 8, 2020

For more information have a look at #10

jdisho and others added 28 commits July 8, 2020 12:55
via this protocol, we can set the delegate to the respected object
@freak4pc
Copy link
Member

freak4pc commented Jul 8, 2020

Awesome! Thanks or taking care of this, again :)
About the question from #10:

Looks great @jdisho ! Should we wait on tests or merge as is? This is a really strong addition and I dont want to delay it for more weeks etc, but if the tests can be done in a few days it might be worth waiting to get at least some basic coverage

@jdisho
Copy link
Contributor Author

jdisho commented Jul 8, 2020

Would love to add some tests, but I am having a hard time figuring out how to unit tests Development pods. Any help is very much appreciated.

@freak4pc
Copy link
Member

freak4pc commented Jul 8, 2020

Development pods specific are the issue? I think you can just do a @testable import CombineCocoa.

@jdisho
Copy link
Contributor Author

jdisho commented Jul 8, 2020

Hey @freak4pc, I added some test for UITableView and UICollectionView. WDYT?

@freak4pc
Copy link
Member

freak4pc commented Jul 8, 2020

I'm gonna pull this into master so we can keep moving from there, seems solid enough and CI isn't actually running unless this would be merged, we can have another PR to add some more tests / clean this up, perhaps.

@freak4pc freak4pc merged commit cec0163 into CombineCommunity:master Jul 8, 2020
@freak4pc
Copy link
Member

freak4pc commented Jul 8, 2020


private static func selectors(ofClass c: AnyClass, encodedReturnType: String) -> Set<Selector> {
var protocolsCount: UInt32 = 0
guard let protocolPointer = class_copyProtocolList(c, &protocolsCount) else { return .init() }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have a memory leak here, forgot to defer free(protocolPointer)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup that seems correct. @jdisho can you look into this?

@freak4pc
Copy link
Member

This is a very old thread but I wanted to share a relatively massive problem with this that I found while trying to use it in a newer project. When using the ScrollView proxy on a UICollectionView, the flow layout delegate is never called and the layout is entirely messed up. Not too sure why it's happening, tried to debug it unsuccessfully so far.

I imagine you probably don't remember this implementation, but any chance you have an idea? @jdisho

@freak4pc
Copy link
Member

freak4pc commented May 25, 2024

Generally speaking I might be missing something but I'm not seeing any form of forward delegation, I think this only intercepts the original delegate but it doesn't actually forward to the original delegate if such exists, basically overriding the original delegate ?

@jdisho
Copy link
Contributor Author

jdisho commented May 28, 2024

Interesting ... 🤔
I don't remember this being an issue back then, but maybe I'm wrong. Could you please share a code snippet?

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

Successfully merging this pull request may close these issues.

3 participants