Skip to content
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

Wrong dependency scope #10

Open
satsen opened this issue Aug 22, 2024 · 2 comments
Open

Wrong dependency scope #10

satsen opened this issue Aug 22, 2024 · 2 comments

Comments

@satsen
Copy link
Contributor

satsen commented Aug 22, 2024

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
Copy link
Owner

dukke commented Aug 23, 2024

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
Copy link
Contributor Author

satsen commented Sep 18, 2024

@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.

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

No branches or pull requests

2 participants