Skip to content

flatten module order error #1999

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

luozejiaqun
Copy link
Contributor

Fixes #1998

@luozejiaqun luozejiaqun force-pushed the bugfix/module-flatten branch from 05c7a11 to 3640db9 Compare October 23, 2024 02:44
@arnaudgiuliani arnaudgiuliani added this to the 4.1.0 milestone Jan 9, 2025
@arnaudgiuliani
Copy link
Member

I don't see other feedback on that. Let's reopen if needed in 4.2

@arnaudgiuliani arnaudgiuliani modified the milestones: 4.1.0, 4.2.0 Apr 30, 2025
@luozejiaqun
Copy link
Contributor Author

@Test
fun test_flatten_common() {
    val m1 = module { }
    val m2 = module { }
    val m3 = module { includes(m1, m2) }
    val modules = m3 + m2

    assertSetEqualsInOrder(flatten(modules), setOf(m3, m1, m2))
}

the test can illustrate the issue clearly. Without the fix, the result will be m3, m2, m1, which is obviously wrong.

@luozejiaqun
Copy link
Contributor Author

Let me explain it in a more practical case:

interface Cryptographer {
  fun encrypt()
  fun decrypt()
}

// implement in common
class SoftwareImplement : Cryptographer

expect val platformCryptographerModule: Module

val cryptographerModule = module {
  singleOf(::SoftwareImplement) binds Cryptographer::class
}

// implement in android & iOS
class HarewareAccelerator : Cryptographer

// in androidMain & iOSMain
actual val platformCryptographerModule = module {
  singleOf(::HarewareAccelerator) binds Cryptographer::class
}

val appModule = module {
  includes(
    cryptographerModule,
    platformCryptographerModule,
  )
}

// without the fix, the result will be
koin.get<Cryptographer>() is SoftwareImplement

Of course, if I know the issue, or I just prefer the following usage:

val cryptographerModule = module {
  singleOf(::SoftwareImplement) binds Cryptographer::class
  includes(platformCryptographerModule)
}

val appModule = module {
  includes(
    cryptographerModule,
  )
}

// no problem
koin.get<Cryptographer>() is HarewareAccelerator

This can cause an issue only when we include two modules, and they contain some overrided parts. This maybe the reason why there is no other feedback.

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.

flatten modules order error
2 participants