mkarneim / pojobuilder

A Java Code Generator for Pojo Builders
Other
334 stars 44 forks source link

Add support for GWT #140

Closed dojcsak closed 6 years ago

dojcsak commented 6 years ago

Add support for GWT compile.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.02%) to 87.5% when pulling 5a3d464f3f82491772ae8cca75affa30dd866262 on dojcsak:add-support-for-gwt into 68ab037004bffa829b2977bc62817b92925cbdbc on mkarneim:master.

mkarneim commented 6 years ago

Hi dojcsak,

Thank you for your contribution.

I would love to add it to PB, but I guess we need to change it a little bit since I don't want to enforce a runtime dependency to PB classes. That means, we can't use PB classes inside the generated builder's source code. So we don't need to create our own copy of @GwtIncompatible.

Instead I would prefer to handle this case equivalent to the Optional case, where the user must configure which Optional class (the Guava one or that from Java 8) to use (done by the @GeneratePojoBuilder.withOptionalProperties=<class>).

I suggest to add a new element to the @GeneratePojoBuilder annotation, that allows to specify which concrete class to use for marking gwt-incompatible methods.

For example:

@GeneratePojoBuilder(markGwtIncompatible=com.google.gwt.core.shared.GwtIncompatible.class)
public class MyPojo {
  ...
}

The default value of this element would be Void.class, which means that those methods wouldn't get marked.

elias-adam commented 6 years ago

The @GwtIncompatible annotation is a compile time dependency, because the GWT compiler uses it when it translates the source code to JS. The @GeneratePojoBuilder annotation a compile the dependency itself, so there is no runtime dependency on @GwtIncompatible.

mkarneim commented 6 years ago

Alright, I will recheck this.

dojcsak commented 6 years ago

Hi mkarneim,

Thank you for your attention.

I agree with elias-adam, @GwtIncompatible is a compile-time dependency. It does not disturb “normal Java” usage. I tested it.

I copied @GwtIncompatible, because of GWT recommendation (in @GwtIncompatible Javadoc): “Since only the name of the annotation matters, Java libraries may use their own copy of this annotation class to avoid adding a compile-time dependency on GWT.”

I like your suggestion about parameterization and I can implement it, but because of GWT recommendation and @GwtIncompatible does not disturb previous usage I think it is the easiest way to add GWT compatibility.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.02%) to 87.506% when pulling f0e6ea46a08a69a6a08d99cdf65659238748e298 on dojcsak:add-support-for-gwt into 16ff2d1dda482ab14784e3543da8166605353128 on mkarneim:master.

mkarneim commented 6 years ago

Alright, that looks good. Since only the annotation's name is relevant, I moved it into our own package.

Thank you for your contribution!

dojcsak commented 6 years ago

Thank you for merged!