node-gtk typings issues

[romgrk]

Hey,

node-gtk maintainer here. I've reviewed some of the typings for node-gtk and here are some issues I've found. I'll refer to Gtk-3.0.d.ts as an example. I'll use prefix g- when using glib terms and ts- when using typescript terms.

1. g-interfaces are listed as ts-classes. For example

export class Editable {
     /* Methods of Gtk.Editable */
     copyClipboard(): void
     // ...
}

g-interfaces can be implemented by g-classes, so I don't think ts-class is the correct mapping for g-interfaces. For example, classes Gtk.Entry and Gtk.TextView both implement Gtk.Editable.

2. g-signals don't have a connect_after method

     /* Signals of Gtk.Editable */
     connect(sigName: "changed", callback: (($obj: Editable) => void)): number
     connect_after(sigName: "changed", callback: (($obj: Editable) => void)): number
     emit(sigName: "changed"): void
     on(sigName: "changed", callback: (...args: any[]) => void): NodeJS.EventEmitter
     once(sigName: "changed", callback: (...args: any[]) => void): NodeJS.EventEmitter
     off(sigName: "changed", callback: (...args: any[]) => void): NodeJS.EventEmitter

.on and friends also don't return the instance but I take that as a node-gtk issue as it should respect nodejs semantics, will be fixed soon.

Note however that there is an open PR that will add an optional argument to .on(signalName: string, cb: Function, after?: boolean) (and .once).
Note that I would also don't tend to promote usage of GObject#connect. It's a lower-lever method and all it's use-cases are satisfied by GObject#on. Just saying.

3. GObject#on callbacks arguments are known

connect(sigName: "delete-text", callback: (($obj: Editable, startPos: number, endPos: number) => void)): number
on(sigName: "delete-text", callback: (...args: any[]) => void): NodeJS.EventEmitter

The arguments for the .on callback are the same as for the .connect callback.

Note: node-gtk skips passing the instance as an argument to the callback because it's a leftover of the C paradigm where closures don't exist. So .on('delete-text', (editable, startPos, endPos) => { /* ... */ }) becomes .on('delete-text', (startPos, endPos) => { /* ... */ }).

4. g-vfuncs aren't currently supported :/

5. typings don't seem to use inheritance for g-classes

export class AccelLabel {
     /* Properties of Gtk.AccelLabel */
     accelClosure: Function
     accelWidget: Widget
     /* Properties of Gtk.Label */
     angle: number
     attributes: Pango.AttrList
     // ...
}

Typings seem to repeat all g-classes parents' props/signals/methods in their descendants. I don't know if there is an implementation reason for that but here would be a good moment to make use of extends.

6. overrides

node-gtk overrides certain API:s to expose a more pleasant API surface when it makes sense. For example, Gtk.Widget#getSizeRequest() is normally typed as (): [number, number], but node-gtk exposes it as (): { width: number, height: number }. All required JSDoc annotations to get the types right are present in the override files (eg Gtk.Widget#getSizeRequest) though I understand it might be too much work for you.

That's it for now, I understand if you can't implement everything, just wanted to mention it for the sake of correctness.

/cc @JumpLink

[JumpLink]

Thanks you for your helpful feedback @romgrk 💚
I will look at it as soon as possible, hopefully in a few weeks.

[JumpLink]

@romgrk Unfortunately I have no time at the moment, maybe you can see if you can implement some of these things yourself?

[romgrk]

Sorry, no bandwidth for that :/ I might get around to it but I have more urgent issues to fix in node-gtk first.

[JumpLink]

Any updates in node-gtk @romgrk ?

[romgrk]

Still no time, the project has other bugs that are more important to fix and I've been having trouble getting around to them :/

[JumpLink]

@romgrk I hope I've fixed the points 2, 3, 4 and 6 now. Can you take a look at @types/node-gtk on my fork?

[romgrk]

I've taken a quick look, seems good overall :)

Fixing 5 would be awesome though. E.g. for Gtk, the methods signature usually take a Gtk.Widget, but we want child clasees (Gtk.Button, Gtk.Icon, ...) to fit in there so they need to inherit from Gtk.Widget.

[mildsunrise]

I think it hasn't been mentioned (& I still need to look at it) but APIs like setProperty are incorrectly marked as taking a GValue, the correct type would be any I think?

edit: also, this tool also seems to mark arrays of primitives as any

[romgrk]

Indeed, our overrides here are missing the JSDoc to change the typings (opened #284).

[mildsunrise]

Also to keep it in mind, the typings use Class.$gtype when it should be Class.prototype.__gtype__, but I'm not sure this was meant to be a public API, because it's missing on boxed objects (only available on actual instances).

[romgrk]

Not public, those are used for edge cases where there is not enough info in either GIR or Glib's gtype system about a boxed instance. They might be removed when I find a better way to handle them.

[mildsunrise]

In that case I suppose the best thing would be to add a public API at Class.$gtype to match gjs. It makes more sense than adding it on instances and the user can do instance.constructor.$gtype

[romgrk]

Yes, adding it on the constructor seems like the best place to add a public API for that. I don't really like the $ notation though. But let's open an issue on node-gtk for further discussion on this, it will save @sammydre from hearing our node-gtk internal discussions.

[JumpLink]

Theoretically all points should be solved now, if there are still issues please open a new issue, I am happy about feedback. :)