opened
ivmarkov
username_0
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
JsFunction has a wrong signature
issue
created
ivmarkov
username_0
Any feedback?
comment
created
pstockley
username_1
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.
comment
created
ivmarkov
username_0
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.
comment
created
pstockley
username_1
Do you want to work on a pull request for this?
comment
created
ivmarkov
username_0
Yes, however, we have quite some time pressure at work ATM, so this might come slowly over time.
comment
created
pstockley
username_1
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.
comment
created
ivmarkov
username_0
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()"?
comment
created
ivmarkov
username_0
What are your thoughts?
comment
created
pstockley
username_1
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.
comment
created
ivmarkov
username_0
Huh.

You mean, this won't work:

```
@JsFunction
interface JsRunnable {
    void run();

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

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

?
comment
created
pstockley
username_1
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.
comment
created
ivmarkov
username_0
For now, let's just rework JsProcedure  to match the Runnable API contract (method name "run" and class name JsRunnable instead of JsProcedure).

Shall I do this or will you do it?
comment
closed
pstockley
username_1
issue