romgrk / node-gtk

GTK+ bindings for NodeJS (via GObject introspection)
MIT License
494 stars 41 forks source link

Types for typescript #280

Open TheBlueOompaLoompa opened 3 years ago

TheBlueOompaLoompa commented 3 years ago

Adding types would allow faster development because of intellisense and the typescript compiler would be able to catch errors in the users syntax. This is the tool that could be used for generating the typings: https://github.com/sammydre/ts-for-gjs

I don't understand how typings are added to node modules like this but I will look into how it has been done for other libraries. See discussion here: https://github.com/romgrk/node-gtk/discussions/266#discussioncomment-563161

romgrk commented 3 years ago

From what I've understood we'll need to add a "types" field in package.json to point to our typings. The typings generated by ts-for-gjs are slightly wrong, and will need some work.

romgrk commented 3 years ago

List of things that need to be adjusted/verified:

More details here. @JumpLink also seems to be doing some work on this on their fork.

I'd be fine with including types directly with this module, we could use ts-for-gjs as a dev dependency to pregenerate typings.

TheBlueOompaLoompa commented 3 years ago

This is just running ts-for-gjs without any cleanup for the newest versions of the libraries including Gtk4: https://github.com/TheBlueOompaLoompa/node-gtk

romgrk commented 3 years ago

Looks like a good start, here are the list of things I'd aim to get fixed before merging:

That should get us mostly correct types. Do you have an interest in adjusting the types? If not, I'll surely get around to this but not sure when.

TheBlueOompaLoompa commented 3 years ago

I will try to work on types because right now I'm working on a project with Gtk4. I think I will start at the newest versions and work backwards starting with Gtk4 and then Gtk3. No Gtk2 because anyone still working on a project with it should have upgraded a while ago. Maybe I will have time for the other libraries but Gtk is first.

mildsunrise commented 3 years ago

The decision to add types is a delicate one IMHO... I'm not sure it's a good idea since:

  1. We can't possibly collect all the libraries / .girs that exist and generate types for them.
  2. The user will have different versions of the libraries. If the user's libraries are newer, they'll get errors if they try to use new functions. If they're older, some functions won't be really there and TypeScript could miss bugs.
  3. The types would get outdated pretty quickly → node-gtk will need to release new versions periodically.
  4. It's a huge set of typings; editor performance and package size will suffer. But users only need access to a handful of libraries.

Typings are awesome, but when they have inconsistencies (some libraries silently do not get typechecked, some properties get silently ignored at runtime, etc.) they can be very confusing.

An alternative is to instruct users to place a prepare script in package.json, like so:

 "prepare": "ts-for-gir <libraries they want to use, or *> -o lib/codegen",

Then ignore lib/codegen from git. In the code, they would do:

import { Button } from './lib/codegen/Gtk-1.0'

This solves all problems above. The drawback is the extra setup work. There are plenty of other cases where you need to generate code (Protobuf, gRPC, OpenAPI, etc.) so I don't think we'd be asking for anything nonstandard here.

romgrk commented 3 years ago
  1. Agreed, but if we do we can add typing for the handful libraries that will compose 95% of use cases (Gtk, Gdk, Pango, Cairo, Vte, Gst, ...).
  2. Indeed, that's a real problem.
  3. Yes, and gnome devs are not known for their backwards-compatibility abilities.

It's fine to make it an additional install step, we just need to make sure to make it very clear and easy, the most annoying thing with typings is installing them :] We could add ts-for-gir as a dependency and expose it for our users in some way. We should also add them to the project template by default.

mildsunrise commented 3 years ago

agreed that it should be as easy & straightforward as possible. adding it as a dependency doesn't feel right bc it's only a devDependency, but it's justified I think. maybe we could do it as a post-install step?

romgrk commented 3 years ago

Yes, post-install step is good. I guess we can explore all that and find a good compromise once someone gets started on this. It's on my task list but so are so many other things :|

gavr123456789 commented 3 years ago

Is there any news on how soon the types can be added?

romgrk commented 3 years ago

No sorry :/ You can get what is available up and running by following the instructions on https://github.com/sammydre/ts-for-gjs, but it will require manually setting them up on your part.

JumpLink commented 1 year ago

@romgrk @gavr123456789 @TheBlueOompaLoompa @mildsunrise

I released ts-for-gir v3.0.0 🚀 now. In this release I have focused on introducing NPM packages, this pre-generated NPM packages can be accessed directly on NPM, such as the node-gtk or node-gtk-4.0 package.

I encourage you to explore ts-for-gir / the NPM packages and provide your valuable feedback 🤗️

romgrk commented 1 year ago

Neat packaging :+1: