Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@record types with no members have no create() method #51

Open
niloc132 opened this issue Jan 27, 2023 · 4 comments
Open

@record types with no members have no create() method #51

niloc132 opened this issue Jan 27, 2023 · 4 comments

Comments

@niloc132
Copy link
Contributor

Steps to reproduce, create a record type with no members.

/**
 * @record
 */
function MyRecord() {}

Expected, a Java interface with a static create() method. Actual output is a Java interface with no contents.

--

While this might seem like a weird edge case, there are at least two ways that this comes up. First, when tsickle generates externs from typescript where a type is re-exported in another file, tsickle will generate a new type that extends the existing type. Second, a record type that extends from two other record types, but doesn't contribute new fields.

/**
 * @record
 */
function ParentRecordA() {}

/**
 * @type {string}
 */
ParentRecordA.prototype.name;

/**
 * @record
 */
function ParentRecordB() {}

/**
 * @type {number}
 */
ParentRecordB.prototype.value;

/**
 * @record
 * @extends {ParentRecordA}
 * @extends {ParentRecordB}
 */
function ChildRecord() {}

Expected Java output,

@JsType(isNative = true, namespace = JsPackage.GLOBAL)
public interface ChildRecord extends ParentRecordA, ParentRecordB {
  @JsOverlay
  static ChildRecord create() {
    return Js.uncheckedCast(JsPropertyMap.of());
  }
}

Actual Java output

@JsType(isNative = true, namespace = JsPackage.GLOBAL)
public interface ChildRecord extends ParentRecordA, ParentRecordB { }

An easy workaround is to just create an instance of a supertype, or just a plain JsObject.create(null) or JsPropertyMap.of(), then wrap in a cast as the usual implementation does. But this isn't expected to be the correct way, since all other records get a generated factory method.

@niloc132
Copy link
Contributor Author

@gkdn
Copy link
Member

gkdn commented Feb 4, 2023

Hmm isDictionaryType decision seems straightforward but looks like TypeReference abstraction doesn't resolve fields so we currently cannot tell if parent has any fields.

@niloc132
Copy link
Contributor Author

niloc132 commented Feb 8, 2023

Does it matter if the parent has fields or not? Even if a record with no supertype has no fields, if the user must create an instance to use some api, it makes more sense to call MyRecord.create() than to manually write out the Js.cast(JsPropertyMap.of()) call - or, if this doesn't make sense, it seems reasonable to remove all such create() method, as the user can just as easily make this call in all places where a record is created?

@gkdn
Copy link
Member

gkdn commented Feb 8, 2023

Even if a record with no supertype has no fields, if the user must create an instance to use some api, it makes more sense to call MyRecord.create() than to manually write out the Js.cast(JsPropertyMap.of()) call

My recollection is; when we emit create methods unconditionally it had some unintended effect (though I don't remember what it was).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants