search

google / jsinterop-generator

Generates Java annotated with JsInterop from JavaScript extern sources
Apache License 2.0
75 stars 24 forks source link

Emit constant values from externs #33

Closed realityforge closed 5 years ago

realityforge commented 5 years ago

So some of closures externs have values for properties marked with @const. i.e. WebSocket. CONNECTING.

Currently jsinterop-generator just assumes the values will be external and must be extracted at runtime. However, given that most of these values are constant and specified by specs, they are unlikely to be changed and could be directly imported into java code. i.e. rather than:

@JsOverlay public static final int CONNECTING = WebSocket__Constants.CONNECTING;

we could generate

@JsOverlay public static final int CONNECTING = 0;

In some scenarios this can lead to a slight code size improvement. However a better reason is that it improves the developer experience to see the values inline.

It seems that on the closure team there is mixed opinions on whether including constant values is a good thing or a bad thing. Sometimes PRs get merged adding them and sometimes not ;) The main reason for not having them is articulated in google/closure-compiler#3293 as

Any compiler that is consuming externs files with values, and changing behavior with respect to whether the externs has values or not is in my opinion showcasing a bug of it's own

Before I ran into this I had already started to play with importing the values into the generator (at least integer and double values). It was an ugly hack that mostly looked something like

      final Node next = jsField.getDeclaration().getNode().getNext();
      final Double value = null != next && next.isNumber() ? next.getDouble() : null;

With a little more sanity checking and type conversion.

However I was considering cleaning this up and submitting it if it would be accepted. At this stage it is unclear whether the closure team will allow values to be added to the externs or not. If they allow them, then it will be simple to read them.

If they decided to strip them, it will be relatively easy to retrieve the constant values from the WebIDL. I have been playing with data from Microsofts Typescript schema generator and think that would make things relatively easy.

gkdn commented 5 years ago

We shouldn't not use value that are manually maintained and could be incorrect (d.ts or externs) and alternative solution is complicated without adding much value. For code size, in GWT 2.x, we discussed this earlier but this should be fixed on that side.

I will close this bug.