graaljs icon indicating copy to clipboard operation
graaljs copied to clipboard

19.2.0 Date interop not working

Open hashtag-smashtag opened this issue 5 years ago • 5 comments

Hi,

I'm trying to use the new date interop in 19.2.0, but having issues in both directions across the Javascript/Java boundary:

  • When I create a JS Date in Javascript, it's only seen as a Java Date if I explicitly use value.as(Date.class). However, value.as(Object.class) instead gives me an empty Map. The workaround for this is to use the HostAccessBuilder#targetTypeMapping feature.
  • When I create a Date in java, it's not seen as a JS Date in Javascript. Implementing ProxyInstant/ProxyDate does not change this. And there is no Java-JS type mapping (see https://github.com/graalvm/graaljs/issues/144). Thus I still need to continue to use my own DateProxy (a ProxyObject that exposes a getTime member as a ProxyExecutable).

Here is a testng test in Groovy that demonstrates the issues:

// In Groovy
@Test
void "date interop troubles"() {
    // Java -> JS ----------------------------------------------------------
    def hostAccess = HostAccess.newBuilder()
            .allowAccessAnnotatedBy(HostAccess.Export.class)
            .allowArrayAccess(true)
            .allowListAccess(true)
            // we could use .targetTypeMapping, but are showing how date interop doesn't work without it
            .build()
    context = Context.newBuilder("js")
            .allowHostAccess(hostAccess)
            .allowExperimentalOptions(true)
            .option("js.experimental-foreign-object-prototype", "true")
            .build()

    def val = context.eval("js", "new Date();")

    that "asDate on js-date works", {
        def asResult = val.as(Date.class)
        assert asResult instanceof Date
        assertThat asResult isInstanceOf Date
    }

    that "asObject on js-date does NOT work", {
        def asResult = val.as(Object.class)
        assertThat asResult isNotInstanceOf Date
        assertThat asResult isInstanceOf Map
    }

    that "asMap on js-object-with-date does NOT work", {
        val = context.eval("js", "var abc = { str: 'hi', num: 123, date: new Date() }; abc;")
        def asResult = val.as(Map.class)
        assertThat asResult.date isNotInstanceOf Date
        assertThat asResult.date isInstanceOf Map
        asResult = val.as(new TypeLiteral<Map<String,Object>>() {})
        assertThat asResult.date isNotInstanceOf Date
        assertThat asResult.date isInstanceOf Map
    }

    // Java -> JS ----------------------------------------------------------
    context.getBindings("js").putMember("binding", new Date())

    that "java Date does NOT become or act like a js-date", {
        val = context.eval("js", "binding instanceof Date")
        assert val.isBoolean() && !val.asBoolean()

        try {
            context.eval("js", "binding.getTime();")
            assert false
        } catch (PolyglotException ignored) { /* expected */}
    }

    that "does NOT work for a POJO containing an explicit Date field in js", {
        context.getBindings("js").putMember("testClass", new TestClass(testDate: new Date()))
        val = context.eval("js", "testClass.testDate instanceof Date")
        assert val.isBoolean() && !val.asBoolean()

        try {
            context.eval("js", "testClass.testDate.getTime();")
            assert false
        } catch (PolyglotException ignored) { /* expected */}
    }

    // Not testing fancier things like Dates as values on Object/Serializable fields, within Collections, etc

    that "does NOT work for ProxyInstant", {
        context.getBindings("js").putMember("binding", ProxyInstant.from(new Date().toInstant()))
        val = context.eval("js", "binding instanceof Date")
        assert val.isBoolean() && !val.asBoolean()

        try {
            context.eval("js", "binding.getTime();")
            assert false
        } catch (PolyglotException ignored) { /* expected */}
    }
}

public class TestClass {
    @Export
    public Date testDate;
}

hashtag-smashtag avatar Aug 27 '19 19:08 hashtag-smashtag

Hi @hashtag-smashtag

I'm currently facing the same issue and was wondering if you have an example of your targetTypeMappings for JS Dates to Java Dates.

Cheers and thanks, David

Biglr avatar Aug 27 '20 13:08 Biglr

Hi @Biglr ,

For the JS->Java direction, we use something like this (note DateProxy references are our own proxy for Java->JS):

// Wherever you're setting up your HostAccess, ours is static:
private static void addTypeMappings(HostAccess.Builder builder) {
    //...
    builder.targetTypeMapping(Value.class, Object.class, Values::isDate, Values::toDateOrClassCastException);
    builder.targetTypeMapping(Value.class, Date.class, Values::isDate, Values::toDateOrClassCastException);
    builder.targetTypeMapping(Value.class, DateProxy.class, Values::isDate, Values::toDateOrClassCastException);
   //...
}

// Use the HostAccess when you're constructing your Contexts:
Builder contextBuilder = Context.newBuilder("js)
    .allowHostAccess(hostAccess)
    //...
    ;

public final class Values {
    public static boolean isDate(@Nullable Value value) {
        if (value == null || value.isNull() || value.isHostObject()) {
            return false;
        }

        // Could consider other formats like long epoch
        // Currently just looking for js Date class or DateProxy

        if (value.isProxyObject() && (value.asProxyObject() instanceof DateProxy)) {
            return true;
        }

        Value metaObject = value.getMetaObject();
        if ((metaObject == null) || !Objects.equals(metaObject.getMetaSimpleName(), "Date")) {
            return false;
        }

        return value.hasMember("getTime") && value.getMember("getTime").canExecute();
    }

    public static Date toDateOrClassCastException(@Nullable Value value) {
        Date date = toDate(value);
        if (value == null) {
            // So mapper can move onto other mappings
            throw new ClassCastException();
        }
    }

    @Nullable
    public static Date toDate(@Nullable Value value) {
        if (value == null || value.isNull()) {
            return null;
        }

        if (value.isProxyObject() && value.asProxyObject() instanceof DateProxy) {
            return (DateProxy) value.asProxyObject();
        }

        checkArgument(value.hasMember("getTime"), "Value [%s] is not a Date.", value);
        Value time = value.getMember("getTime").execute();
        checkArgument((time != null) && time.isNumber() && time.fitsInLong(),
                      "Value [%s] is not a Date.", value);

        return new DateProxy(time.asLong());
    }
}

Cheers, Matt

hashtag-smashtag avatar Aug 27 '20 14:08 hashtag-smashtag

Hi @hashtag-smashtag

Thank you very much for your reply!

I was fiddling around with targetTypeMaps in the mean time and I came up with this (found something similar in a close GitHub issue somewhere, there really is not much in terms of examples for this around...):

.targetTypeMapping(Value.class, Date.class, v -> v.hasMember("getTime"), v -> new Date(v.getMember("getTime").execute().asLong()))

however, this never worked and just ignored the mapping no matter what i set for accepts.

I guess the critical point was to not only specify Date.class or LocalDate.class but Object.class as well (which you did gladly, otherwise I might have never come to that conclusion).

I'll stick to your implementation though, since it offers much more extensibility :wink:

Thank you once again very much, you saved me a lot time and headaches :grinning:

Cheers, David

Biglr avatar Aug 27 '20 15:08 Biglr

Java->JS interop is still mostly broken. Using 20.2.0, it looks like JSRuntime.toPrimitiveFromForeign doesn't handle Date/Instance correctly and calls toString and tries to coerce it to a double. When I try to use the Intl library for formatting a straight ProxyDate, it gives a rangeError. If my ProxyDate implements ProxyObject and implements toString as the same as getTime, it works, but it's incorrect. Date.toString() should return a formatted date and not the epoch date.

JSRuntime.toPrimitiveFromForeign should check InteropLibrary.isDate/isInstant/etc. and then call the appropiate method.

You can validate it by running js --jvm --js.intl-402 then trying this

var pdate = Java.type('org.graalvm.polyglot.proxy.ProxyDate').from(Java.type('java.time.LocalDate').now())
new Intl.DateTimeFormat('en-US').format(pdate)

It returns

RangeError: Provided date is not in valid range.
	at <js> null(Unknown)
	at <js> :program(<shell>:2:1:0-45)

instead of properly coercing as a JSDate

steventamm avatar Sep 03 '20 15:09 steventamm

Hi @steventamm

For Java -> JS we use something like this as a workaround:

public class DateProxy extends Date implements ProxyObject {

    private static final Set<String> PROTOTYPE_FUNCTIONS = ImmutableSet.of(
            "getTime",
            "toISOString",
            "toJSON",
            "toString"
    );

    public DateProxy(Date date) {
        super(date.getTime());
    }

    public DateProxy(long epoch) {
        super(epoch);
    }

    @Override
    public Object getMember(String key) {
        switch (key) {
            case "getTime":
                return (ProxyExecutable) arguments -> getTime();
            case "toJSON":
            case "toISOString":
                return (ProxyExecutable) arguments -> ISO8601Utils.format(this, true);
            case "toString":
                // Currently defaulting to Date.toString, but could improve
                return (ProxyExecutable) arguments -> toString();
            default:
                throw new UnsupportedOperationException("This date does not support: " + key);
        }
    }

    @Override
    public Object getMemberKeys() {
        return PROTOTYPE_FUNCTIONS.toArray();
    }

    @Override
    public boolean hasMember(String key) {
        return PROTOTYPE_FUNCTIONS.contains(key);
    }

    @Override
    public void putMember(String key, Value value) {
        throw new UnsupportedOperationException("This date does not support adding new properties/functions.");
    }
}

hashtag-smashtag avatar Sep 08 '20 19:09 hashtag-smashtag