Wrapper Classes should be frozen on create
Closed this issue · 9 comments
If we freeze the wrapper classes after creation, we can then safely pass those around between iOS threads.
Simple example in pseudo swift code:
this will work as both the creation and the call happen on the main thread
THREAD:MAIN {
let loadUserUseCaseIos = LoadUserUseCaseIos(LoadUserUseCase(...))
loadUserUseCaseIos.loadUser(username: "foo").subscribe(
scope: coroutineScope, //this can be provided automatically, more on that below
onSuccess: { user in print(user?.description() ?? "none") },
onThrow: { error in print(error.description())}
)
}
this will FAIL as the creation happens on another thread than the call
THREAD:MAIN {
let loadUserUseCaseIos = LoadUserUseCaseIos(LoadUserUseCase(...))
}
THREAD:IO {
loadUserUseCaseIos.loadUser(username: "foo").subscribe(
scope: coroutineScope, //this can be provided automatically, more on that below
onSuccess: { user in print(user?.description() ?? "none") },
onThrow: { error in print(error.description())}
)
}
But if we freeze the LoadUserUseCaseIos after creation, both will work fine.
https://kotlinlang.org/docs/mobile/concurrency-overview.html#immutable-and-frozen-state
Problem with that approach is that it prevents wrapped classes from being mutable and sometimes they need to. Like I said in the other issue, the general idea is to pass data in worker classes through threads, not to pass workers.
That being said, I might quite easily add a param to annotation which would make it conditional, I've been thinking about it for some time.
That would be quite cool. I like the conditional approach in your suggestion
Another issue that would be resolved if we freeze both the subscriber and the wrapper class is that the returned value is not nullable. With freezing the generated obj-c code will be
__attribute__((objc_subclassing_restricted))
__attribute__((swift_name("SuspendWrapperV2")))
@interface BSTLFSuspendWrapperV2<T> : BSTLFBase
- (instancetype)initWithScope:(id<BSTLFKotlinx_coroutines_coreCoroutineScope>)scope suspender:(id<BSTLFKotlinSuspendFunction0>)suspender __attribute__((swift_name("init(scope:suspender:)"))) __attribute__((objc_designated_initializer));
- (id<BSTLFKotlinx_coroutines_coreJob>)subscribeOnSuccess:(void (^)(T))onSuccess onThrow:(void (^)(BSTLFKotlinThrowable *))onThrow __attribute__((swift_name("subscribe(onSuccess:onThrow:)")));
@end;compared to
__attribute__((objc_subclassing_restricted))
__attribute__((swift_name("KoruSuspendWrapper")))
@interface BSTLFKoruSuspendWrapper<T> : BSTLFBase
- (instancetype)initWithScopeProvider:(id<BSTLFKoruScopeProvider> _Nullable)scopeProvider suspender:(id<BSTLFKotlinSuspendFunction0>)suspender __attribute__((swift_name("init(scopeProvider:suspender:)"))) __attribute__((objc_designated_initializer));
- (id<BSTLFKotlinx_coroutines_coreJob>)subscribeOnSuccess:(void (^)(T _Nullable))onSuccess onThrow:(void (^)(BSTLFKotlinThrowable *))onThrow __attribute__((swift_name("subscribe(onSuccess:onThrow:)")));
- (id<BSTLFKotlinx_coroutines_coreJob>)subscribeScope:(id<BSTLFKotlinx_coroutines_coreCoroutineScope>)scope onSuccess:(void (^)(T _Nullable))onSuccess onThrow:(void (^)(BSTLFKotlinThrowable *))onThrow __attribute__((swift_name("subscribe(scope:onSuccess:onThrow:)")));
@end;The only difference I made was adding freeze() to init and to subscribe as per the example on https://touchlab.co/kotlin-coroutines-rxswift/
That would make the Swift code a lot cleaner.
I need to correct myself regarding my comment above. freezing the wrapper did not make the returned value non-nullable.
It was another change that that removed the nullability.
By setting a generic constraint to Any to the generic parameter the obj-c representation of the class will remove the nullability
class SuspendWrapper2<T : Any>
I'll make a PR for this: #34
@JohNan the problem is that sometimes you need nullable data passed in suspend functions or flows. And on the swift side you lose the nullability info, because obj-c (kotlin native is only interoperable with obj-c, not swift) doesn't have nullability info on generic types afaik. I might be wrong though.
@micHar Thank you for the explanation, I didn't think of that use case, and you are correct. What if we can create a second wrapper called NonNullSuspendWrapper that is used when the data is not null?
I'm a bit skeptical as well.
The problem with this approach is the complexity it brings. It can be nice on the producer side, i.e. you can easily check with the annotation processor if the wrapped type is nullable and then use null or non-null wrapper. But on the consumer side, i.e. in ios code you would probably need to have two different wrappers depending on whether you are accepting nulls or not. So 4 wrappers in total (flow / suspend).
This library is a bit complicated to explain as it is - I'm not sure if enhancing the strictness (which is out of Koru's control, since it's obj-c interoperability issue anyway) is worth the cost of increasing conceptual complexity.
Unless there's a way to not increase the complexity on consumer side, hmm...
I just looked through the repo by @russhwolf and he created 4 wrappers to be able to handle nullable and non-null data. I still think this is a good idea and would make the swift side of the code easier to work with. At least it would be up to the consumer of the library to choose and not forced.