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

Optional depends on javafx.graphics #29

Closed
wants to merge 1 commit into from
Closed

Conversation

Glavo
Copy link
Contributor

@Glavo Glavo commented Jan 6, 2023

In fact, the dependence of FX Gson on javafx.graphics is optional.

Although it is rare to use java.base without depends on javafx.graphics, I think it is a good thing to reduce the mandatory dependency on other modules as much as possible in module-info.java.

In fact, the dependence of FX Gson on javafx.graphics is optional. 

Although it is rare to use java.base without depends on javafx.graphics, I think it is a good thing to reduce the mandatory dependency on other modules as much as possible in module-info.java.
@joffrey-bion
Copy link
Owner

While I understand the change for javafx.graphics, could you please elaborate on why you removed the transitive keyword for javafx.base? The reason for the very existence of this library is to manipulate JavaFX properties and related classes. What would be the point in using this library without its transitive dependencies on JavaFX types?

@Glavo
Copy link
Contributor Author

Glavo commented Jan 7, 2023

While I understand the change for javafx.graphics, could you please elaborate on why you removed the transitive keyword for javafx.base? The reason for the very existence of this library is to manipulate JavaFX properties and related classes. What would be the point in using this library without its transitive dependencies on JavaFX types?

The main purpose of this change is to make these two requirements more "symmetrical". I think if one of them is declared as transient and the other is not, it will cause some subtle problems.

The second reason is that I believe that only when the API relies on other modules can it be declared as transient.
FxGson and FxGsonBuilder do not have API methods that take classes in javafx.base as parameters or return values, so transient is not required.

@joffrey-bion
Copy link
Owner

joffrey-bion commented Jan 7, 2023

The main purpose of this change is to make these two requirements more "symmetrical".

They were both transient, and with this change one becomes static and the other is not. I don't understand what you mean by "more symmetrical" - if anything, they are less symmetrical now.

FxGson and FxGsonBuilder do not have API methods that take classes in javafx.base as parameters or return values, so transient is not required.

I think you mean transitive, not transient. FxGson and FxGsonBuilder don't, but other public classes of this library do. Basically all the public type adapters have JavaFX classes in their public APIs. This is why those 2 modules were declared transitive, it's the equivalent of api in Gradle jargon.

This is true for both javafx.graphics and javafx.base, actually.

@Glavo Glavo closed this Jan 8, 2023
@Glavo Glavo deleted the patch-1 branch January 8, 2023 14:19
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.

None yet

2 participants