Pass injector directly into FormViewModel
#5738
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
brnunes-stripe
commented
Oct 24, 2022
Comment on lines
335
to
+382
| val application = applicationSupplier() | ||
| val starterArgs = starterArgsSupplier() | ||
| injectWithFallback( | ||
| starterArgsSupplier().injectorKey, | ||
| FallbackInitializeParam(application, starterArgs.productUsage) | ||
| ) | ||
| return subComponentBuilderProvider.get() | ||
| val logger = Logger.getInstance(BuildConfig.DEBUG) | ||
|
|
||
| WeakMapInjectorRegistry.retrieve(starterArgs.injectorKey)?.let { | ||
| it as? NonFallbackInjector | ||
| }?.let { | ||
| logger.info( | ||
| "Injector available, " + | ||
| "injecting dependencies into ${this::class.java.canonicalName}" | ||
| ) | ||
| injector = it | ||
| it.inject(this) | ||
| } ?: run { | ||
| logger.info( | ||
| "Injector unavailable, " + | ||
| "initializing dependencies of ${this::class.java.canonicalName}" | ||
| ) | ||
| fallbackInitialize(FallbackInitializeParam(application, starterArgs.productUsage)) | ||
| } | ||
|
|
||
| val subcomponent = subComponentBuilderProvider.get() | ||
| .application(application) | ||
| .args(starterArgs) | ||
| .savedStateHandle(savedStateHandle) | ||
| .build().viewModel as T | ||
| .build() | ||
| val viewModel = subcomponent.viewModel | ||
| viewModel.injector = injector | ||
| return viewModel as T | ||
| } | ||
|
|
||
| override fun fallbackInitialize(arg: FallbackInitializeParam) { | ||
| val component = DaggerPaymentOptionsViewModelFactoryComponent.builder() | ||
| .context(arg.application) | ||
| .productUsage(arg.productUsage) | ||
| .build() | ||
|
|
||
| injector = object : NonFallbackInjector { | ||
| override fun inject(injectable: Injectable<*>) { | ||
| when (injectable) { | ||
| is FormViewModel.Factory -> component.inject(injectable) | ||
| else -> { | ||
| throw IllegalArgumentException("invalid Injectable $injectable requested in $this") | ||
| } | ||
| } | ||
| } | ||
| } | ||
| component.inject(this) |
There was a problem hiding this comment.
This logic is now replicated in PaymentOptionsViewModel, PaymentSheetViewModel and LinkActivityViewModel. I'll refactor to make it generic and part of the stripe-core module in the next PR.
brnunes-stripe
requested review from
tillh-stripe,
jameswoo-stripe,
ccen-stripe and
awush-stripe
as code owners
3 tasks
tillh-stripe
approved these changes
Oct 25, 2022
| @@ -150,7 +150,7 @@ complexity: | |||
| thresholdInObjects: 11 | |||
| thresholdInEnums: 11 | |||
| ignoreDeprecated: false | |||
| ignorePrivate: false | |||
| ignorePrivate: true | |||
There was a problem hiding this comment.
What’s the reason for the changes in this file?
There was a problem hiding this comment.
Detekt was failing for one of the files for having more than 11 methods, I think ignoring private methods is a good change to make it a little more flexible.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
FormViewModel's lifecycle is always strictly contained within the PaymentSheet lifecycle. Therefore, it doesn't need to be able to recreate the whole dependency graph.FormViewModel.Factorynow receives the injector as a parameter, which guarantees that it will reuse all already created dependencies. This allows us to deleteFormViewModelComponent, which was used byFormViewModelduring fallback injection.BaseSheetViewModelis now responsible for injecting into the ViewModels with smaller lifecycle, similar to howLinkActivityViewModelholds an injector that is passed to the screen-specific ViewModels.PaymentOptionsViewModel.FactoryandPaymentSheetViewModel.Factorycreate theNonFallbackInjectorand set it intoBaseSheetViewModel.Motivation
Fix #5737
Testing