JetBrains / compose-multiplatform

Compose Multiplatform, a modern UI framework for Kotlin that makes building performant and beautiful user interfaces easy and enjoyable.
https://jetbrains.com/lp/compose-multiplatform
Apache License 2.0
15.95k stars 1.16k forks source link

Add the ability to not depend on the "compose.material" module #497

Open igordmn opened 3 years ago

igordmn commented 3 years ago

Probably we can make compose.desktop.currentOs dependent on compose.foundation, not on compose.material.

In this case developers who don't want to use material module can write:

implementation(compose.desktop.currentOs)

or

implementation(compose.desktop.currentOs)
implementation(compose.foundation)

or

implementation(compose.desktop.foundation.currentOs)

If material module is needed then developers can write:

implementation(compose.desktop.currentOs)
implementation(compose.material)

or

implementation(compose.desktop.material.currentOs)
olonho commented 3 years ago

What's the point for such a change?

igordmn commented 3 years ago

What's the point for such a change?

If someone will write a library-replacement for material module (with a bunch of UI components), there is no easy way to use it without material (because our desktop module depends on material) .

After this change we can use only that library, without material

jimgoog commented 3 years ago

I'm actually a little confused by the whole currentOs thing; why is this needed when using MPP? That is to say, why is it not sufficient to just add a reference to the maven artifact with gradle metadata and then let the platform-specific questions get resolved automatically?

igordmn commented 3 years ago

maven artifact with gradle metadata

Yes, I am also thought about that! But at the time I tried (Summer 2020) there were some issues.

Maybe I just didn't know MPP plugin very well, maybe MPP plugin didn't support different jvm binary targets.

But we can try again, and if we will need something from MPP plugin, we can communicate with MPP team.

olonho commented 3 years ago

currentOs is to provide different artifacts on different platforms (as Skia is pretty big, and we don't want to pack all possible native libs into single artifact). Do not think MPP will help here.

igordmn commented 3 years ago

Do not think MPP will help here

Theoretically we can still ship separate artifacts for each platform (mac-arm64, mac-x64, win64, linux64), and make MPP plugin resolve platform automatically:

kotlin {
    android()
    jvm("desktop")
    jvm("macosArm64")
    jvm("windowsX64")

    sourceSets {
        val commonMain by getting {
            dependencies {
                api(compose.material)
            }
        }

        val desktopMain by getting {
            dependsOn(commonMain)
            dependencies {
                api(compose.desktop)
            }
        }

        val macosArm64Main by getting {
            dependsOn(desktopMain)
        }

        val windowsX64Main by getting {
            dependsOn(desktopMain)
        }
    }
}

Compose Gradle plugin task "./gradlew run" will determine current Os and will run appropriate target.

It looks verbose, so we need to think can we reduce the code size.

Wertual08 commented 1 year ago

Are there any workarounds for now to not include compose.material?

malliaridis commented 1 year ago

@Wertual08 I am using Material 3 and Material 2 was causing many naming conflicts and styling issues when wrongly imported, so I excluded it from every package that includes it (works fine so far, but tested only with Material 3 included). For example:

implementation(compose.desktop) {
  exclude("org.jetbrains.compose.material")
}
Wertual08 commented 1 year ago

@malliaridis Many thanks!

Omico commented 9 months ago

Bought it from #4035,

Maybe it is time to remove DesktopTheme.jvm.kt completely.

Then we can safely remove compose.material.

PMARZV commented 5 months ago

Any ETA for this?? is there any blocker that prevents this from happening???

igordmn commented 1 month ago

curentOs maybe not needed if https://github.com/JetBrains/compose-multiplatform/issues/3282 is resolved. compose.desktop module can be removed after this as well.

okushnikov commented 3 weeks ago

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.