JsFunction has a wrong signature

[ivmarkov]

I would expect JsFunction to correspond to java.util.Function, i.e. to be a one-arg rather than 0-arg function.

0-arg functions are modeled in Java 8 with java.util.Supplier, so I suggest:

• Rename JsFunction to JsSupplier
• Implement a new JsFunction that matches the signature of java.util.Function

Furthermore, it would be good if:

• The method names of JsSupplier, JsFunction, JsBiFunction and JsConsumer match the corresponding names of Supplier, Function, BiFunction and Consumer, i.e. get(), apply(..) and accept(...) respectively.
• We need a JsRunnable, matching java.util.Runnable

Last but not least, it would be good if each of the Js* interfaces are having two additional methods:

• A default method, which converts a Js* instance, to the corresponding java.util instance
• A static @JsOverlay method, which takes a java.util instance and constructs a corresponding Js* instance

[ivmarkov]

Any feedback?

[pstockley]

Sorry, I haven't had much chance to take a look. Given that it could be a breaking change, I would like to tackle this at the same time as the refactoring for elemental2. I am waiting for this to stabilize before I make any changes.

[ivmarkov]

That's fine of course - I can wait as well. I just wanted to raise the topic.

On a related note, there are a lot of extra helper @JsFunction-s which are probably not strictly necessary either. A few examples:

• ChildrenForEachFn - JsConsumer<ReactElement<?, ?>> can be used instead
• ChildrenMapFn - JsFunction<ReactElement<?, ?>, ReactElement<?, ?>> can be used instead
• SetStateCallback - JsBiFunction<S, P, S> can be used instead
• All functions/predicates currently used in Array.java from gwt-interop-utils, like Testfn, TestFullFn, ForEachFn, ForEachFullFn, MapFn, MapFullFn, ReduceFn, ReduceFullFn, SortFn, SortFullFn

To summarize, I think it would be easier for the user if she sees familiar Supplier/Consumer/Function interfaces everywhere, even if those are not the java util ones, but your "@JsFunction" ones. And then we can implement converters from one to the other.

[pstockley]

Do you want to work on a pull request for this?

[ivmarkov]

Yes, however, we have quite some time pressure at work ATM, so this might come slowly over time.

[pstockley]

No problem. I think after the react 16.1 release, I would like to do one big release with as many breaking changes as possible. So there is no rush.

[ivmarkov]

Furthermore, it would be good if:

The method names of JsSupplier, JsFunction, JsBiFunction and JsConsumer match the corresponding names of Supplier, Function, BiFunction and Consumer, i.e. get(), apply(..) and accept(...) respectively. We need a JsRunnable, matching java.util.Runnable

I see you have started this. The one bit in need of fixing is JsProcedure, which is identical to the j.u.Runnable contract. How about renaming JsProcedure to JsRunnable then, and renaming JsProcedure's method to "run()"?

[ivmarkov]

Not sure about this one:

Last but not least, it would be good if each of the Js* interfaces are having two additional methods:

A default method, which converts a Js instance, to the corresponding java.util instance A static @JsOverlay method, which takes a java.util instance and constructs a corresponding Js instance

What are your thoughts?

[pstockley]

I will look into making JsProcedure match runnable. Regarding the conversions I am less sure. The problem is that you cannot take a java.util lambda and convert it to a Js equivalent because the java.util class wasn't annotated with @JsFunction. So it wouldn't be equivalent and would cause strange errors if you passed it to Js. For that reason, I think it would be better if we didn't provide these conversion functions. You can always create helpers outside the library to do this.

[ivmarkov]

Huh.

You mean, this won't work:

@JsFunction
interface JsRunnable {
    void run();

    default Runnable asRunnable() {
        return () -> run();
    }

    static JsRunnable create(Runnable runnable) {
        return () -> runnable.run();
    }
}

?

[pstockley]

I thought you meant a direct cast not wrapping with another lambda. This should work. I don't really have any need for this. However, if you want to make the change I will happily merge it.

[ivmarkov]

For now, let's just rework JsProcedure to match the Runnable API contract (method name "run" and interface name JsRunnable instead of JsProcedure).

Shall I do this or will you do it?