Generate TypeScript definitions for JS API #3567
Conversation
niloc132
added
jsapi
DocumentationNeeded
ReleaseNotesNeeded
| public String type; | ||
| // TODO (deephaven-core#3442) change to some kind of String+int union type | ||
| public String position; | ||
|
|
||
| public boolean log = false; |
There was a problem hiding this comment.
Currently the generated type for this is:
log:boolean;
If I add @JsNullable, then it changes to:
log:boolean|null|undefined;
What I think it should be is:
log?: boolean
Pretty much all of these params are optional. Is there anyway to make that notation? @JsOptional doesn't seem to work on properties (though maybe it should).
There was a problem hiding this comment.
@mattrunyon and I had this chat in slack (you were in the channel too, but I don't recall if you weighed in) - we have 4 cases to handle, potentially:
- not optional, not nullable - required, non-null values
- optional and nullable - not required, and you can explicitly state null
- not optional, but nullable - required, but you can explicitly state null
- optional, but not nullable - not required, but if you specify it, it cannot be null
(note that I'm using "null" as a placeholder for "null or undefined")
If we need all four states, we must have both @JsNullable and a @TsOptional that we can decorate on properties to make this clear. On the other hand, if we really only need the first and last, then in terms of values that are possible, adding @JsNullable to a property means the following:
- it can be a real value
- it can be assigned to null
- it can be assigned to undefined
- it can be left undefined
The last three are all the same in Java, and JS insists on a distinction between the last two. Matt and I decided that the last two really doesnt matter, as long as you aren't asking the object directly if it has the property defined on it, but just reading or writing to the property.
But null vs undefined still matters - if it is allowed to be null but not undefined, then TS will tell you that a foo === null check is correct, when you actually want a foo == null check to normalize out the undefined value - and vice versa. Assuming we support "nullable properties are automatically optional" we can drop the undefined from the union, but we can't/shouldn't drop the null, or nulls could escape.
For Java/GWT's part, there is no difference between a field that hasn't been initialized and one that has been initialized to null, so anywhere we handle a null, we have to have some kind of undefined handling too.
| @JsNullable | ||
| public int colspan; |
There was a problem hiding this comment.
This declares them as:
colspan:number|null|undefined;
I think colspan?:number would be more accurate; is there any way to make that notation?
There was a problem hiding this comment.
Not at this time, we'll have that ready soon though.
| import jsinterop.base.JsPropertyMap; | ||
|
|
||
| import java.util.Map; | ||
|
|
||
| @JsType(name = "SeriesDescriptor", namespace = "dh.plot") | ||
| public class JsSeriesDescriptor { | ||
| // TODO (deephaven-core#3442) change to some kind of String+int union type | ||
| public String plotStyle; | ||
| public String name; |
There was a problem hiding this comment.
Many of the properties on the SeriesDescriptor should be optional.
|
My goal here isn't to suggest that this is complete, but that we've gotten most of the big changes in, and can follow up with optionals, unions, generics, and javadoc. This patch gets merge conflicts at least once per week, if not more - merging it sooner than later will ensure that at least new types are correct, even if we can't switch the UI over to this generated d.ts yet. |
|
Labels indicate documentation is required. Issues for documentation have been opened: How-to: https://github.com/deephaven/deephaven.io/issues/2419 |
There are a few pieces missing to really be happy with these types, but they are mostly accurate and fit the patterns we're expecting.
Follow-ups, ranked in order of priority:
objectoranyreferences. The API actually accepts any object (and will give a runtime error if an unexpected type is used), but for TypeScript we can be more specific.