dukke / FXSkins

Skins with new features and looks for existing JavaFX controls
https://pixelduke.com/fxskins/
47 stars 9 forks source link

Wrong dependency scope #10

Open satsen opened 3 months ago

satsen commented 3 months ago

https://github.com/openjfx/javafx-gradle-plugin?tab=readme-ov-file#5-dependency-scope

FXSkins should not bring runtime JavaFX dependencies into my program

Also, I think ControlsFX should be compileOnly as well. Right now I need to exclude it in my own build.gradle to avoid having ControlsFX in my shaded jar. (Like I suggested on JMetro)

dukke commented 3 months ago

Hmm...

FXSkins won't work if the javafx controls module isn't present since javafx classes from that module are exposed in the public API of FXSkins... seems to me that the correct way is to have them be required?

The same for ControlsFX as FXToggleSwitchSkin constructor receives a ToggleSwitch instance and ToggleSwitch is part of Controlsfx. I'd say that even perhaps ControlsFX dependency needs to be changed to "API".

satsen commented 2 months ago

@dukke

To put controlsfx as "api" means that all users of fxskins will also get controlsfx which doesn't make sense in my opinion. Optional dependencies aren't handled with "api". I know modules handle it but not all projects are modular. And it is supposed to be compileOnly for javafx as well.

Examples:

The link I sent in the first comment says "Native dependencies can be avoided by declaring the dependency configuration as compileOnly.". But you seem to have solved the issue in another way because look here at the POM for 1.0.0 which includes windows native binaries (bad), but the POM for 1.1.0 does not. If you didn't do that intentionally it might be worth to check why it did it incorrectly for 1.0.0 but not 1.1.0.