Gio-2.0.Cancellable.connect is typed incorrectly for GJS

[HeavenVolkoff]

Hello,

Currently, the generated definitions for Gio-2.0 on GJS has incorrect typing for the Cancellable.connect method according to GJS's documentation and directly testing on its REPL:

It seems Gio-2.0.Cancellable doesn't follow the GObject.connect convention and implements a custom connect as far as I could understand.

To solve this I tried to simply implement an injection into Gio-2.0.Cancellable to override the connect method, however that resulted in unresolved conflicts and ultimately all implementations of Gio-2.0.Cancellable.connect ended up commented out of the type definitiondue to conflicts:

I then tried to understand why this was happening in conflict-resolver.ts, and from what I could gather this seems to be a temporary solution:
https://github.com/sammydre/ts-for-gir/blob/bdfd98a5654a8ffcaafe623ae123981ad39429d1/packages/cli/src/type-definition-generator.ts#L545-L548

I don't know if the injection solution and solving the current method conflict shortcoming is the correct path for this particular problem, or if there is some other, simpler, way. Maybe this could be solved by adding an exception on commenting out conflicting injected methods? Any ideas?

[HeavenVolkoff]

I was able to make the injection solution work by adding the exception on commenting out conflicting injected methods, and using an override to the connect method, it doesn't exactly match the runtime behaviour, but at least it does not complain on correct code anymore:
master...HeavenVolkoff:ts-for-gir:fix-gio-cancellable-connect

I don't know if this is a scalable approach to this type of problem. However, considering it originates from Gio.Cancellable basically breaking Liskov substitution principle and Typescript type system being quite strict about it, I couldn't think of any other way that wasn't extremely hacky or ovely rcomplicated.

[JumpLink]

@HeavenVolkoff Maybe we can introduce a new property: ignoreConflict (or more general ignoreTypeError) and if this is true we could generate a // @ts-ignore over the generated type definition, maybe this way we could use the exact connect method type definition. I am not sure right now, but it could be that I have integrated something similar before.

Your approach is right, our own overrides should not be commented out on conflicts.

I'm not at the computer right now (currently I am on the mobile phone) so I can't try it myself right now. During the week I can try this as soon as I have time for that. Feel free to try this yourself if you are faster than me ;)

[HeavenVolkoff]

@JumpLink I don't think the // @ts-ignore solution is going to work.

When removing the overload from the Gio.Cancelled.connect method, Typescript complains that the whole class, and interface, incorrectly extends the base class:

Since the errors are for the whole class, even when you annotate the Gio.Cancelled.connect method with // @ts-ignore the errors are not ignored. The only way to make TS ignore them is to apply the // @ts-ignore annotation top-level to both the class and the interface:

However, I don't personally think this is a good solution, because it can hide errors that spawn from future changes in the generator, and can, possibly, also affect how typescript type checks user projects.

I don't think my current approach is a great one, but for now is the one with fewer drawbacks that I could come up if. I will open a draft PR with it, so you can take a better look and if you think of something better feel free to commit to the PR directly.

[HeavenVolkoff]

A partial solution, that could be implemented in a future init command, would be to use ESLint's no-restricted-syntax custom rules to warn users that calling .connect('cancelled', () => {}) is possibly wrong if the object is a Gio.Cancelled instance. Which is something GJS already does in their ESLint config:
https://gitlab.gnome.org/GNOME/gjs/-/blob/master/.eslintrc.yml#L145-161

[HeavenVolkoff]

Issue resolved by PR #91, with some caveats yet to be addressed, described in #92 and #93

[JumpLink]

@HeavenVolkoff Thank you! I'll look at it next week, this one I was very busy