nui
nui copied to clipboard
Compatibility Issue: Implementation of `didMoveToWindow` swizzling doesn't work with other frameworks
Is there any reason why you implemented didMoveToWindow
for UILabel (and the other classes) instead of just letting them all drop down to UIView's swizzled implementation? That would allow the swizzle chain to stay intact. The other way to do it would be to call [super didMoveToWindow]
in UILabel's override_didMoveToWindow]
. I've tested both and they seem to work.
Interesting observation. I personally can't think of a reason why this would be needed.
@tombenner can you comment on this?
@yjkogan A pr that cleans this up a bit would be very welcome.
@yjkogan Note however that super is in fact called already. Consider the following "implementation".
// vanilla UILabel
- (void)didMoveToWindow {
/* Maybe something else, maybe not */
[super didMoveToWindow];
}
// UILabel+NUI
- (void)override_didMoveToWindow
{
if (!self.isNUIApplied) {
[self applyNUI];
}
[self override_didMoveToWindow];
}
Method swizzeling swaps the two implementations:
- (void)override_didMoveToWindow
{
/* Maybe something else, maybe not */
[super didMoveToWindow];
}
- (void)didMoveToWindow
{
if (!self.isNUIApplied) {
[self applyNUI];
}
[self override_didMoveToWindow];
}
The [self override_didMoveToWindow];
call first calls the original implementation, which will call the super classes implementation.
Hey @timbodeit, can you clarify what you mean by "why this would be needed?" Do you mean the swizzle chain or the implementation of the method?
I don't remember much about this particular issue, since opened the issue so long ago. I believe what I found was that the SDK I was working on didn't work with NUI installed because NUI wasn't calling back up the swizzle chain. As you correctly point out, calling [self override_didMoveToWindow]
will call the original implementation, and if you do it that way then other SDKs can swizzle the same method (either before or after) and both SDKs will get a chance to intercept the call before it hits the original implementation. I believe that something in this SDK was skipping straight to the original implementation, bypassing anyone else who had swizzled the method
What I mean is: You are correct in your observation. I do not see a reason why one would need to swizzle every class individually instead of letting them "drop down". Essentially answering your first sentence with "I didn't implement it, but I can't see such a reason".
Got it, sounds good! Sorry to say but I know I wont have time to open a PR into this repo :(
About the swizzle chain: While I agree that only swizzling UIView would be the cleaner way, the chain seems to me like it is intact.
When calling didMoveToWindow
on UILabel:
- this call goes to
override_didMoveToWindow
inUILabel+NUI.m
- which calls
didMoveToWindow
on UIKit/UILabel (see my second comment) - which should call
super didMoveToWindow
- which goes to
override_didMoveToWindow
inUIView+NUI.m
- which calls
didMoveToWindow
on UIKit/UIView or any other swizzled implementations
I could only imagine that one of UIKit's classes doesn't make the super call. As in: Apple Engineer thinking: UIView doesn't do anything of relevance anyways, no need for me to call it.
If this is still relevant to you, I'd be happy to investigate further if you could provide an example project alongside actual and expected behavior.
Yeah I can't remember the exact details as it was a while ago. @tonyrzhang still works on the project though so he may be able to help? Miss you @tonyrzhang!
Aww thanks @yjkogan Miss you too!
Hey @timbodeit, It's definitely been a while since I looked at this as well, but IIRC it had to do with the way NUI was implementing its swizzling. We had a customer that had the Optimizely SDK as well as the NUI SDK and when both were initialized, the NUI swizzling didn't seem to play nice with ours. To remedy this I believe we ended up helping them make some adjustments to the NUI code base for their app. I don't have a copy or PR of it anymore but essentially we changed the NUI swizzling to follow this paradigm: https://blog.newrelic.com/2014/04/16/right-way-to-swizzle/
And things worked after that. LMK if you have any other questions.