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 270 forks source link

Typhoon 4.x Migration Notes / Issues #562

Open vitorhugomagalhaes opened 7 years ago

vitorhugomagalhaes commented 7 years ago

Hi,

I've tried to use the new 4.0.2 release but there were some API changes that I cannot understand. Some of them cause the app to crash.

Here goes a list of issues:

TyphoonAssemblyActivator was deprecated.

Some of our assemblies are not listed in the plist file and thus, we have the following code to manually activate them.

@interface FactoryAssembly : TyphoonAssembly

@end
@implementation FactoryAssembly

- (id) init {
    self = [super init];

    if (self) {
        [self activate];
    }

    return self;
}

@end

@implementation FactoryAssembly

- (id) init {
    self = [super init];

    if (self) {
        [self activate];
    }

    return self;
}

@end

After checking the docs it was not clear how to refactor this. Could you please provide some more insights here ?

Plist activation + Modularizing Assemblies

The order used when declaring the assemblies in the plist file was not relevant until now. However, with this new version it has changed somehow.

@interface SupportStoryboardViewFactoryAssembly  : TyphoonAssembly<SupportStoryboardViewFactoryProtocol>

@property(nonatomic, strong, readonly) ServiceAssembly                          *serviceAssembly;
@property(nonatomic, strong, readonly) CommonStoryboardViewFactoryAssembly      *commonStoryboardFactoryAssembly;

@end

If serviceAssembly is declared after SupportStoryboardViewFactoryAssembly the following code will crash:

- (id<HelpCenterArticleDetailsViewModelProtocol>) helpCenterArticleDetailsViewModel:(NSString *) articleID
{
    return [TyphoonDefinition withClass:[HelpCenterArticleDetailsViewModel class] configuration:^(TyphoonDefinition *definition)
            {
                [definition useInitializer:@selector(initWithSupportService:inesService:supportAssembly:settingsService:articleID:) parameters:^(TyphoonMethod *initializer) {
                    [initializer injectParameterWith:[self->_serviceAssembly supportService]];
                    [initializer injectParameterWith:[self->_serviceAssembly inesService]];
                    [initializer injectParameterWith:self];
                    [initializer injectParameterWith:[self->_serviceAssembly settingsService]];
                    [initializer injectParameterWith:articleID];
                }];
            }];
}

Moving ServiceAssembly before SupportStoryboardViewFactoryAssembly solves the issue. Still, due to circular dependencies I cannot follow this approach.

Any ideas how to handle this ?

Thanks in advance,

alexgarbarev commented 7 years ago

TyphoonAssemblyActivator was deprecated.

Yes. This one was removed, because Typhoon was changed a lot. Main reason of that change - improve performance.

If you want to understand technical details.

Before

We list all assembly methods and.. SWIZZLE THEM ALL! Main reason for swizzle was: rename methods, and then intercept invocations and handle properly. That's possible because methods moved from original place, so we can implement "forwardInvocation" there.

Example: You have your FactoryAssembly instance, and you have fooService instance method that returns TyphoonDefinition. TyphoonAssemblyActivator swizzles fooService into __typhoon_swizzled_fooService for example. Now, __typhoon_swizzled_fooService returns TyphoonDefinition and fooService doesn't exists. Then you calling fooService instance method which doesn't exists, and we handle that with forwardInvocation - we creating real instance based on TyphoonDefinition

Problem

Method swizzling is slow, when you swizzling hundreds of methods on startup. It can be measured with seconds, when you have hundreds assemblies.

Now

Instead of swizzling assembly methods on actual TyphoonAssembly subclass, we using "proxy" (It's internal object called TyphoonAssemblyAccessor in Typhoon) object that handles your call with forwardInvocation: and calls original Assembly method if needed to get TyphoonDefinition.

So in terms for performance, we don't need to spent any time on swizzling.

jasperblues commented 7 years ago

@alexgarbarev Let's prepare a migration guide as @vitorhugomagalhaes requested.

vitorhugomagalhaes commented 7 years ago

Hi,

Thanks for the insights.

Even so, what would be the fix for FactoryAssembly class ?

Regarding the plist load order, are you aware of such issue ?

Thanks,

alexgarbarev commented 7 years ago

Even so, what would be the fix for FactoryAssembly class ?

The correct way to get activated assembly (outside plist or appDelegate bootstrapping):

FactoryAssembly *activatedAssembly = [FactoryAssembly activated];

Regarding the plist load order, are you aware of such issue ?

This shouldn't be a problem

alexgarbarev commented 7 years ago

Does it make sense @vitorhugomagalhaes? Can we close this issue?

AdithyaReddy commented 6 years ago

@vitorhugomagalhaes am facing currently facing the same issue of circularly dependant assemblies. Could you please tell me how the issue was solved?