appsquickly / typhoon

Powerful dependency injection for Objective-C ✨✨ (https://PILGRIM.PH is the pure Swift successor to Typhoon!!)✨✨
https://pilgrim.ph
Apache License 2.0
2.7k stars 269 forks source link

Multiple instances injected into view controller when using storyboard reference #505

Closed tpesce closed 8 years ago

tpesce commented 8 years ago

I'm seeing a case where injection into a view controller unexpectedly happens multiple times (with multiple instances) when the view controller is loaded via a storyboard reference.

I've been able to reproduce the problem with a simple project: https://github.com/tpesce/typhoon-issue

This project started with the Xcode "Tabbed Application" template. I then moved the second tab view controller to it's own storyboard, and attached it to the tab bar controller via a storyboard reference.

Both the first and second view controller have a property:

@property(strong, nonatomic) Model *model;

and this property is injected by type. Here's the assembly:

@implementation Assembly

- (FirstViewController *)firstViewController
{
    return [TyphoonDefinition
        withClass:[FirstViewController class]
        configuration:^(TyphoonDefinition *definition) {
            [definition injectProperty:@selector(model)];
        }];
}

- (SecondViewController *)secondViewController
{
    return [TyphoonDefinition
        withClass:[SecondViewController class]
        configuration:^(TyphoonDefinition *definition) {
            [definition injectProperty:@selector(model)];
        }];
}

- (Model *)model
{
    return [TyphoonDefinition withClass:[Model class]];
}

@end

Injection into the first view controller happens as expected. However, for the second view controller, two different model instances are injected into the single instance of the second view controller.

Environment:

Typhoon 3.4.7 (via Carthage) Xcode 7.3.1 iOS 9.3 (Simulator)

vasilenkoigor commented 8 years ago

Hi @tpesce, you want that model will be injected to ViewController as new instance? Do I understand correctly?

tpesce commented 8 years ago

Hey @spbvasilenko, maybe not quite...

In FirstViewController, one single instance of Model is created and injected as expected. However, in SecondViewController, two different instances of Model are created and the injection happens twice; once for each model instance.

If I override the -[setModel:] method for both view controller, the -[init] method for the model, and then log the instances, here's what I see:

2016-06-08 09:43:46.202 TyphoonIssue[1246:76837] Model.init: self = <Model: 0x7fc9d9c0e290>
2016-06-08 09:43:46.203 TyphoonIssue[1246:76837] SecondViewController.setModel: self = <SecondViewController: 0x7fc9d9eada60>, model = <Model: 0x7fc9d9c0e290>
2016-06-08 09:43:46.206 TyphoonIssue[1246:76837] Model.init: self = <Model: 0x7fc9d9d253d0>
2016-06-08 09:43:46.207 TyphoonIssue[1246:76837] FirstViewController.setModel: self = <FirstViewController: 0x7fc9dc11e510>, model = <Model: 0x7fc9d9d253d0>
2016-06-08 09:43:46.207 TyphoonIssue[1246:76837] Model.init: self = <Model: 0x7fc9dc107a10>
2016-06-08 09:43:46.207 TyphoonIssue[1246:76837] SecondViewController.setModel: self = <SecondViewController: 0x7fc9d9eada60>, model = <Model: 0x7fc9dc107a10>

The difference between the two view controllers is that FirstViewController is included in the main storyboard, but SecondViewController is included via a storyboard reference:

storyboard

My expectation is that injections for SecondViewController should behave the same as for FirstViewController: that only one instance of Model would be created and injected.

vasilenkoigor commented 8 years ago

@tpesce thank you for more better explanation. It's look as a really strange behaviour. I'll try analyzed and solve this issue

tpesce commented 8 years ago

Thanks @spbvasilenko! One thing I should have noted: I don't consider this to be an urgent issue. Now that I understand this behavior, I've been able to work around it successfully.

alexgarbarev commented 8 years ago

I didn't debug this issue to know the real reason, but it looks like inject: method were called twice for SecondViewController - once in first and once in second storyboard..

If you want to debug, try to set breakpoint into

- (id)instantiateViewControllerWithIdentifier:(NSString *)identifier

in TyphoonStoryboard and see what happens

vasilenkoigor commented 8 years ago

@alexgarbarev I got it. As SecondViewController is an instantiate controller of SecondStoryboard, injection for this controller happens twice: when SecondStoryboard firstly loaded with SecondViewController as instantiate and second injection happens when in FirstStoryboard we loading UITabController, that has SecondViewController as child controller.

for (UIViewController *controller in viewController.childViewControllers) { [self injectPropertiesForViewController:controller]; }

alexgarbarev commented 8 years ago

Alright. I've noticed that too (in my current project). It looks like a bug. I know the intension why this code was written: for (UIViewController *controller in viewController.childViewControllers) { [self injectPropertiesForViewController:controller]; }.. But this doesn't work correctly, when viewController added as child view..

vasilenkoigor commented 8 years ago

@alexgarbarev May be this enumeration childs viewcontrollers and injection them is redundant?

vasilenkoigor commented 8 years ago

@alexgarbarev because each viewcontroller instantiated from storyboard will be injected whatever, right?

alexgarbarev commented 8 years ago

As you noticed, FirstViewController from above was injected once. Without this enumeration, it will be not injected at all. We might check viewController's storyboard property, but this is not always a right solution. (Sometimes you might manually add child viewController during injection and before that enumeration)

alexgarbarev commented 8 years ago

Hm.. Maybe this is a right solution:

for (UIViewController *controller in viewController.childViewControllers) {
    if (controller.storyboard == self) {
        [self injectPropertiesForViewController:controller]; 
    }
}
vasilenkoigor commented 8 years ago

Looks good

vasilenkoigor commented 8 years ago

@alexgarbarev After debugging with this fix, issue was solved

alexgarbarev commented 8 years ago

@spbvasilenko you may want to make PR with a fix and unit tests for these cases?

vasilenkoigor commented 8 years ago

@alexgarbarev sure! I'll will be happy to make PR 👍

alexgarbarev commented 8 years ago

Pushed as 3.4.8. @tpesce please try latest verison on your project

tpesce commented 8 years ago

Thank you @spbvasilenko and @alexgarbarev! I updated my test project to 2.4.8, and the problem is resolved.

vasilenkoigor commented 8 years ago

@tpesce You are welcome!