docs: DOC-1179: JS API - Wrapper class docs#7775
Conversation
elijahpetty
added
NoDocumentationNeeded
NoReleaseNotesNeeded
No docs changes detected for a9a65a9 |
There was a problem hiding this comment.
Pull request overview
Updates JS client API wrapper classes with clearer Javadoc to improve generated documentation and clarify JS-facing conversion helpers.
Changes:
- Added/expanded Javadoc on numeric wrappers (
LongWrapper,BigIntegerWrapper,BigDecimalWrapper) covering parsing, numeric conversion, andvalueOf()/toString(). - Added Javadoc clarifying
DateWrapperepoch-nanos semantics and JSDateconversions. - Added Javadoc to
LocalDateWrapperdescribing field getters and string formatting.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web/client-api/src/main/java/io/deephaven/web/client/api/LongWrapper.java | Adds class/method Javadoc for parsing and JS-friendly conversions (asNumber, valueOf, toString). |
| web/client-api/src/main/java/io/deephaven/web/client/api/LocalDateWrapper.java | Documents valueOf, date component getters, and toString formatting. |
| web/client-api/src/main/java/io/deephaven/web/client/api/DateWrapper.java | Adds class-level and method Javadoc for nanos timestamps and JsDate conversions. |
| web/client-api/src/main/java/io/deephaven/web/client/api/BigIntegerWrapper.java | Documents ofString, conversions, and equality/hash semantics. |
| web/client-api/src/main/java/io/deephaven/web/client/api/BigDecimalWrapper.java | Documents ofString, conversions, and equality/hash semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| * This type extends {@link LongWrapper} and inherits the following conversions: | ||
| * <ul> | ||
| * <li>{@link LongWrapper#asNumber()} for a numeric representation</li> | ||
| * <li>{@link LongWrapper#toString()} and {@link LongWrapper#valueOf()} for string representations</li> | ||
| * <li>{@link LongWrapper#ofString(String)} for parsing</li> | ||
| * </ul> |
There was a problem hiding this comment.
The class Javadoc says DateWrapper “inherits … LongWrapper#ofString(String) for parsing”, but ofString returns a LongWrapper, not a DateWrapper. In JS, calling DateWrapper.ofString(...) would produce an instance without asDate(), which is surprising. Consider either adding a DateWrapper.ofString(String) that returns a DateWrapper, or adjusting the Javadoc to avoid implying a parsing API specific to DateWrapper.
There was a problem hiding this comment.
That isn't how GWT exports static methods on classes - they aren't inherited, so there is no DateWrapper.ofString method.
The typedoc output does seem to believe that the class members are inherited though - but the return type is LongWrapper, anything that expects a DateWrapper will figure it out pretty quickly...
| * Creates a wrapper from an integer string. | ||
| * | ||
| * @param str A string value accepted by {@link BigInteger#BigInteger(String)}. |
There was a problem hiding this comment.
We can't reference BigInteger in a meaningful way from js/ts
| * Creates a wrapper from an integer string. | |
| * | |
| * @param str A string value accepted by {@link BigInteger#BigInteger(String)}. | |
| * Creates a wrapper from an integer string. The provided string may start with {@code +} or {@code -}, and must otherwise only contain the digits 0-9. | |
| * | |
| * @param str A string representation of an integer. |
| /** | ||
| * Returns the wrapped value as a number. | ||
| * | ||
| * @return The {@link BigDecimal} value converted to a {@code double}. |
There was a problem hiding this comment.
double doesnt mean anything in particular in JS
| * @return The {@link BigDecimal} value converted to a {@code double}. | |
| * @return The {@link BigDecimal} value converted to a JS {@code Number}. |
(or just Number).
| * Creates a wrapper from a decimal string. | ||
| * | ||
| * @param value A string value accepted by {@link BigDecimal#BigDecimal(String)}. |
There was a problem hiding this comment.
| * Creates a wrapper from a decimal string. | |
| * | |
| * @param value A string value accepted by {@link BigDecimal#BigDecimal(String)}. | |
| * Creates a wrapper from a decimal string. Strings may optionally start with {@code +}/{@code -}, | |
| * must have a decimal value, and may end with {@code e}/{@code E} followed by an exponent. | |
| * | |
| * @param value A string decimal value with arbitrary precision. |
| * This type extends {@link LongWrapper} and inherits the following conversions: | ||
| * <ul> | ||
| * <li>{@link LongWrapper#asNumber()} for a numeric representation</li> | ||
| * <li>{@link LongWrapper#toString()} and {@link LongWrapper#valueOf()} for string representations</li> | ||
| * <li>{@link LongWrapper#ofString(String)} for parsing</li> | ||
| * </ul> |
There was a problem hiding this comment.
That isn't how GWT exports static methods on classes - they aren't inherited, so there is no DateWrapper.ofString method.
The typedoc output does seem to believe that the class members are inherited though - but the return type is LongWrapper, anything that expects a DateWrapper will figure it out pretty quickly...
No description provided.