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

Removing FlowFactory and Flow. #380

Merged
merged 2 commits into from
Sep 4, 2024
Merged

Conversation

Laimiux
Copy link
Collaborator

@Laimiux Laimiux commented Sep 3, 2024

What

Removing FlowFactory and Flow because it's a rarely used feature and it's not built well. Instead, if this functionality is needed, it should be built outside the formula-android module.

  • Created FragmentFlowStoreFormula and moved most logic from FragmentFlowStore there
  • Removed Bindings, Binding and CompositeBinding classes

@carrotkite
Copy link

carrotkite commented Sep 3, 2024

JaCoCo Code Coverage 88.59% ✅

Class Covered Meta Status
com/instacart/formula/android/FragmentFlowStore 100% 0%
com/instacart/formula/android/FragmentStoreBuilder 100% 0%
com/instacart/formula/android/internal/FeatureBinding 100% 0%
com/instacart/formula/android/internal/FragmentFlowStoreFormula 80% 0%
com/instacart/formula/android/internal/MappedFeatureFactory 100% 0%

Generated by 🚫 Danger

@Laimiux Laimiux force-pushed the laimonas/remove-flow-factory branch 7 times, most recently from fde869a to 0e53190 Compare September 3, 2024 15:09
}
}


private val lifecycleEvents = PublishRelay.create<FragmentLifecycleEvent>()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this logic is moved into FragmentFlowStoreFormula.

@Laimiux Laimiux force-pushed the laimonas/remove-flow-factory branch 4 times, most recently from 9fe4eb4 to 95c0dcd Compare September 3, 2024 15:57
* Defines how specific keys bind to the state management associated
*/
@PublishedApi
internal abstract class Binding<in ParentComponent> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for an abstract binding class anymore

@@ -1,6 +0,0 @@
package com.instacart.formula.android.internal

internal class Bindings<in Component>(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed as we expose List<FeatureBinding<>> directly

output = Unit,
actions = context.actions {
val isInScope = input.activeFragments.any { binds(it.key) }
Action.onData(isInScope).onEvent {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we don't have scopes anymore, we don't need CompositeBinding anymore

*/
internal class FeatureBinding<in Component, in Dependencies, in Key : FragmentKey>(
private val type: Class<Key>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplified this to a simple class with properties and moved logic to FragmentFlowStoreFormula

)
}

private fun initFeature(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is moved from FeatureBinding

import com.instacart.formula.android.FragmentKey

@PublishedApi
internal class MappedFeatureFactory<Component, Dependencies, Key : FragmentKey>(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feature factory that maps Component to Dependency internally

)
}

@Test fun `component is shared between flow features`() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed flow related tests

@@ -29,26 +30,30 @@ class FragmentAndroidEventTest {
initialContract = TestLifecycleKey()
},
contracts = {

bind<TestLifecycleKey> { _, _ ->
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed bind with a lambda within FragmentStoreBuilder

@Laimiux Laimiux force-pushed the laimonas/remove-flow-factory branch from 95c0dcd to 2e08ceb Compare September 3, 2024 16:44
@PublishedApi
internal inline fun <Component> build(
init: FragmentStoreBuilder<Component>.() -> Unit
): Bindings<Component> {
): List<FeatureBinding<Component, *>> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets make sure to bump the version to note that this is not a backwards compatible change.

@Laimiux Laimiux merged commit d60112a into master Sep 4, 2024
4 checks passed
@Laimiux Laimiux deleted the laimonas/remove-flow-factory branch September 4, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants