google / jsinterop-annotations

JsInterop java annotations for J2CL and GWT
Apache License 2.0
18 stars 8 forks source link

Proposed change in behavior for `@JsProperty`/`@JsType` with non-native Record types #6

Open niloc132 opened 10 months ago

niloc132 commented 10 months ago

https://openjdk.java.net/jeps/395 added Records to Java 16 as a finalized feature. In the course of implementing this for GWT 2, we've observed that it might make sense to slightly modify @JsProperty for non-native types in a way that doesn't automatically produce errors. It could also make sense to change how @JsType behaves on non-native records to automatically expose record components as properties rather than methods.

From the JEP:

A record class, and the components in its header, may be decorated with annotations. Any annotations on the record components are propagated to the automatically derived fields, methods, and constructor parameters, according to the set of applicable targets for the annotation. Type annotations on the types of record components are also propagated to the corresponding type uses in the automatically derived members.

This means that if @JsProperty is annotated on a record component, it will propagate to both the accessor method and the private final field. That is,

record Point(@JsProperty int x, @JsProperty int y) {}

would effectively become

record Point(@JsProperty int x, @JsProperty int y) {
  @JsProperty
  private final int x;
  @JsProperty
  private final int y;
  @JsProperty
  public int x() {
    return this.x;
  }
  @JsProperty
  public int y() {
    return this.y;
  }
}

This seems problematic for two reasons:

Possible workarounds:

I propose instead that

Next, decorating a record with @JsType. This is unlikely to reflect the developer's intent, as by default the constructor and accessor methods would be treated as if decorated with their default jsinterop annotation. Instead, I suggest that the methods should be exported as properties. This makes the above example even simpler, offering a simple immutable record/struct to JS:

@JsType
public record Point(int x, int y) {}

As the constructor and accessors are implicit (but can be explicit, and in the case of the constructor, compact), if declared they could have their own jsinterop annotations present (@JsIgnore, for example), and if other methods are added, they would follow the standard rules that JsType follows - if public, they are annotated as expected. As records cannot have non-static fields added explicitly, this effectively means only constructors and methods will be able to be exposed in this way.

One observation: the compact constructor, in conjunction with these rules could simplify records with many components. Instead of the initial example where each component was annotated with @JsProperty, we could instead see

@JsType
public record Book(String title, int pageCount, List<String> authors) {
  @JsIgnore
  Book {
  }
}

It may also make sense to offer new behavior for native records, so a native Java record means something - perhaps a closure struct could make sense here? This would mean that invoking a native Java record's constructor would serve to create an object with the expected components, simple syntactic sugar to replace the interfaces with create() factory method that jsinterop-generator currently produces. This is a little more opinionated though (dealing with immutability, etc), so could be taken up separately.

gkdn commented 10 months ago

Thanks for the write up. I think it generally makes sense though not sure what to do with native types. My quick take is JsType should mean the same thing everywhere and my there is a different annotation if we want to significantly change meaning (like JsFunction, JsEnum etc..).

niloc132 commented 8 months ago

After some reflection, I think that @JsType(isNative=true) on an enum would be fairly similar in its wrongness, so in the same way that marking a Java enum as a native js type doesn't make sense, I think marking a Java record as native js type probably doesn't make sense.

I think I'd go further at this time and suggest that a closure @record is probably incompatible with Java record, even with a hypothetical @JsRecord annotation to paper over details:

Any read I have of it, there is no good analog in closure for an immutable record/struct type - even if you were to find a type already written as

/** @record */
function Book() {};
/** @const {string} */
Book.prototype.title;
/** @const {number} */
Book.prototype.pageCount;
/** @const {string[]} */
Book.prototype.authors;

you still have the "problem"/mismatch that the array in authors is mutable. Maybe we just punt on that, and acknowledge that Java developers will have to be aware that native records won't behave at all like Java records (in contrast to enums which have some compile time limitations, but no cases where values will change in surprising ways).

Given that, is there any way to permit native Java record types, without also applying runtime changes to the objects like calling freeze on them...?


The good news at least is that I don't think this needs to affect non-native records: