Skip to content

flatten module order error#1999

Merged
arnaudgiuliani merged 1 commit intoInsertKoinIO:4.1.2from
luozejiaqun:bugfix/module-flatten
Sep 10, 2025
Merged

flatten module order error#1999
arnaudgiuliani merged 1 commit intoInsertKoinIO:4.1.2from
luozejiaqun:bugfix/module-flatten

Conversation

@luozejiaqun
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

@arnaudgiuliani
Copy link
Copy Markdown
Member

arnaudgiuliani commented Sep 10, 2025

After re-evaluation let's take it back to 4.1.2 👍
thanks @luozejiaqun

@arnaudgiuliani arnaudgiuliani changed the base branch from main to 4.1.1 September 10, 2025 14:45
@arnaudgiuliani arnaudgiuliani changed the base branch from 4.1.1 to 4.1.2 September 10, 2025 14:49
@arnaudgiuliani arnaudgiuliani merged commit 17f3218 into InsertKoinIO:4.1.2 Sep 10, 2025
@domgew
Copy link
Copy Markdown

domgew commented Nov 27, 2025

@arnaudgiuliani Will 4.1.2 be released anytime soon?

@arnaudgiuliani
Copy link
Copy Markdown
Member

4.1.2 has been merged into 4.2 because of some more impactful changes. See 4.2.0 Milestone around 👍

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

3 participants