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

Typhoon 4.x Migration Notes / Issues #562

Open
vitorhugomagalhaes opened this issue May 29, 2017 · 6 comments
Open

Typhoon 4.x Migration Notes / Issues #562

vitorhugomagalhaes opened this issue May 29, 2017 · 6 comments

Comments

@vitorhugomagalhaes
Copy link
Contributor

vitorhugomagalhaes commented May 29, 2017

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,

@vitorhugomagalhaes vitorhugomagalhaes changed the title Typhoon 4.x Migration Notes Typhoon 4.x Migration Notes / Issues May 29, 2017
@alexgarbarev
Copy link
Contributor

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
Copy link
Member

jasperblues commented May 30, 2017

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

  • include the explanation above
  • For TyphoonAssemblyActivator deprecation: What to do instead.
  • What worked before but won't now.

@vitorhugomagalhaes
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

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

@AdithyaReddy
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants