Skip to content

[HACK week] Modularization of SelectedSite #15086

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.

Sign up for GitHub

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
samiuelson wants to merge 14 commits into
base: trunk
Choose a base branch
from
Open

[HACK week] Modularization of SelectedSite #15086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
f99f1cf
Update RELEASE-NOTES.txt
samiuelson Nov 26, 2025
185bdcc
Move `SiteComponent` and site utilities to `commons` module
samiuelson Dec 11, 2025
0a60e64
Move `ResourceProvider` to `libs/commons` along with its dependencies
samiuelson Dec 12, 2025
bec6c5a
Merge branch 'trunk' into hack-modularize-selected-site
samiuelson Dec 12, 2025
65b2bc7
Merge branch 'trunk' into hack-modularize-selected-site
samiuelson Dec 12, 2025
9fa84ad
Merge branch 'trunk' into hack-modularize-selected-site
samiuelson Dec 12, 2025
4c1580d
Clean up code
samiuelson Dec 12, 2025
03b653b
Clean up code
samiuelson Dec 12, 2025
fd7cff9
Clean up code
samiuelson Dec 12, 2025
6ca3357
Merge branch 'trunk' into hack-modularize-selected-site
samiuelson Dec 12, 2025
d2cf0a5
Clean up code
samiuelson Dec 12, 2025
4f73066
Satisfy detekt's complaints
samiuelson Dec 12, 2025
3bd48ff
Add lint options and baseline for StateFlowValueCalledInComposition i…
samiuelson Dec 15, 2025
e766de2
Merge branch 'trunk' into hack-modularize-selected-site
samiuelson Dec 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions WooCommerce/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,6 @@ dependencies {
implementation(libs.automattic.tracks.android)
implementation(libs.automattic.tracks.crashlogging)

implementation(project(":libs:fluxc"))
Copy link
Contributor Author

@samiuelson samiuelson Dec 12, 2025

Choose a reason for hiding this comment

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

💡 I'm looking for opinions on whether it's better to declare fluxc as a transitive dependency in libs/commons and remove it from the WooCommerce module (this is what I did here since think it's cleaner) or keep it as a regular dependency in both of them. cc @wzieba

Copy link
Contributor

@wzieba wzieba Dec 15, 2025

Choose a reason for hiding this comment

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

I'm leaning into declaring fluxc as regular dependency. I can imagine a scenario in which we add a new module that will use common but won't have to use anything from fluxc. Not having fluxc as transitive dependency would be an advantage in such a case, to not share unnecessary classes, recuding dependencies etc.

Copy link
Contributor

@malinajirka malinajirka Dec 16, 2025

Choose a reason for hiding this comment

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

Unless I'm missing something I believe Fluxc will be part of common either case @wzieba. The difference is just in the WooCommerce module - whether it's declared explicitly there or its a transitive dependency.

I mean, ideally, I would also prefer not having fluxc in common, but I don't think that's what's being discussed here.

Copy link
Contributor

@wzieba wzieba Dec 16, 2025

Choose a reason for hiding this comment

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

Ok so I understand the issue as @samiuelson described it is

Option A: fluxc transitive

// libs/commonts/build.gradle
dependencies {
    api(project(":libs:fluxc"))
}

// WooCommerce/build.gradle
dependencies {
    // nothing
}

Option B: fluxc non-transitive

// libs/commonts/build.gradle
dependencies {
    implementation(project(":libs:fluxc"))
}

// WooCommerce/build.gradle
dependencies {
    implementation(project(":libs:fluxc"))
}

My opinion is that I think Option B is better, because in the future, if we introduce a new module NewModule and we'll want to use common there but we don't need to use fluxc , then with Option B we don't unnecessarily offer fluxc. This way we reduce the dependency chain.

Copy link
Contributor

@wzieba wzieba Dec 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I mean, ideally, I would also prefer not having fluxc in common, but I don't think that's what's being discussed here.

That's also an interesting point to discuss: it depends on what common is for us. I guess we might need both: a common that includes fluxc and a common that doesn't have to and shares different types of things. But this split is still doable incrementally later, so I think I would proceed with this PR as it's a step in right direction.

Copy link
Contributor

@malinajirka malinajirka Dec 16, 2025

Choose a reason for hiding this comment

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

Ahh, I see - makes sense, thanks for elaborating on it! I agree that option B sounds better.

implementation(project(":libs:fluxc-plugin"))
implementation(project(":libs:login"))
implementation(project(":libs:pos"))

Expand Down
16 changes: 16 additions & 0 deletions WooCommerce/src/main/kotlin/com/woocommerce/android/di/SiteComponentEntryPoint.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.woocommerce.android.di

import androidx.datastore.core.DataStore
import com.woocommerce.android.ui.mystore.data.DashboardDataModel
import dagger.hilt.EntryPoint
import dagger.hilt.InstallIn
import kotlinx.coroutines.CoroutineScope

@InstallIn(SiteComponent::class)
@EntryPoint
interface SiteComponentEntryPoint {
fun dashboardDataStore(): DataStore<DashboardDataModel>

@SiteCoroutineScope
fun siteCoroutineScope(): CoroutineScope
}
26 changes: 26 additions & 0 deletions libs/commons/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ plugins {
alias(libs.plugins.android.library)
alias(libs.plugins.kotlin.android)
alias(libs.plugins.kotlin.parcelize)
alias(libs.plugins.ksp)
alias(libs.plugins.google.dagger.hilt)
alias(libs.plugins.dependency.analysis)
}

Expand All @@ -19,17 +21,41 @@ android {
targetCompatibility = libs.versions.java.get()
}

buildFeatures {
buildConfig = true
}

lintOptions {
// Workaround for kotlinx-metadata version incompatibility:
// Kotlin 2.2.21 generates metadata v2.2.0, but AGP 8.13.2's Compose lint detector
// only supports up to v2.0.0, causing crashes when analyzing Kotlin files.
// This can be removed when AGP updates its bundled kotlinx-metadata-jvm dependency.
Copy link
Contributor

@malinajirka malinajirka Dec 16, 2025

Choose a reason for hiding this comment

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

💡 I think we might want to create a linear task for this to ensure we don't forget about it.

disable "StateFlowValueCalledInComposition"

baseline file("lint-baseline.xml")
}

testFixtures.enable = true
}

dependencies {
implementation(libs.androidx.annotation)
implementation(libs.androidx.core.ktx)
implementation(libs.androidx.compose.ui.main)
implementation(libs.glassfish.javax.annotation)
implementation(libs.kotlinx.coroutines.core)
implementation(libs.automattic.tracks.crashlogging)
implementation(libs.androidx.lifecycle.process)
implementation(libs.androidx.preference.ktx)
implementation(libs.eventbus.android)
implementation(libs.google.dagger.hilt.android.main)
implementation(libs.wordpress.utils)
implementation(libs.apache.commons.text)

ksp(libs.google.dagger.hilt.compiler)

api project(":libs:fluxc")
api project(":libs:fluxc-plugin")

testImplementation(libs.kotlinx.coroutines.test)
testImplementation(libs.assertj.core)
Expand Down
15 changes: 15 additions & 0 deletions libs/commons/lint-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?xml version="1.0" encoding="UTF-8"?>
<issues format="6" by="lint 8.7.2" type="baseline" client="gradle" dependencies="false" name="AGP (8.7.2)" variant="all" version="8.7.2">

<issue
id="StateFlowValueCalledInComposition"
message="Calling value on a StateFlow inside of a composable function will not observe changes to the StateFlow"
errorLine1=" state.value?.let { return it }"
errorLine2=" ~~~~~">
<location
file="src/main/java/com/woocommerce/android/tools/SelectedSite.kt"
line="64"
column="15"/>
</issue>

</issues>
13 changes: 0 additions & 13 deletions ...m/woocommerce/android/di/SiteComponent.kt → ...m/woocommerce/android/di/SiteComponent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
package com.woocommerce.android.di

import androidx.datastore.core.DataStore
import com.woocommerce.android.ui.mystore.data.DashboardDataModel
import dagger.BindsInstance
import dagger.hilt.DefineComponent
import dagger.hilt.EntryPoint
import dagger.hilt.InstallIn
import dagger.hilt.components.SingletonComponent
import kotlinx.coroutines.CoroutineScope
import org.wordpress.android.fluxc.model.SiteModel
Expand All @@ -29,15 +25,6 @@ interface SiteComponent {
}
}

@InstallIn(SiteComponent::class)
@EntryPoint
interface SiteComponentEntryPoint {
fun dashboardDataStore(): DataStore<DashboardDataModel>

@SiteCoroutineScope
fun siteCoroutineScope(): CoroutineScope
}

@Qualifier
@MustBeDocumented
@Retention(RUNTIME)
Expand Down
0 ...ocommerce/android/extensions/NumberExt.kt → ...ocommerce/android/extensions/NumberExt.kt
View file
Open in desktop
File renamed without changes.
0 ...ocommerce/android/extensions/StringExt.kt → ...ocommerce/android/extensions/StringExt.kt
View file
Open in desktop
File renamed without changes.
0 ...woocommerce/android/tools/SelectedSite.kt → ...woocommerce/android/tools/SelectedSite.kt
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion ...merce/android/tools/SiteConnectionType.kt → ...merce/android/tools/SiteConnectionType.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.woocommerce.android.tools

import com.woocommerce.android.BuildConfig
import com.woocommerce.android.util.WooLog
import com.woocommerce.commons.BuildConfig
import org.wordpress.android.fluxc.model.SiteModel

enum class SiteConnectionType {
Expand Down
0 ...m/woocommerce/android/util/StringUtils.kt → ...m/woocommerce/android/util/StringUtils.kt
View file
Open in desktop
File renamed without changes.
0 ...rce/android/viewmodel/ResourceProvider.kt → ...rce/android/viewmodel/ResourceProvider.kt
View file
Open in desktop
File renamed without changes.