-
Notifications
You must be signed in to change notification settings - Fork 6
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
Next version: GNOME Shell 47 #49
Conversation
@JumpLink Can we go back to 3.x typings? These 4.0.0 beta types (presumably generated after the merge with gi.ts?) are still lacking a bunch of promisified overloads, and it just doesn't seem right for a stable typing to depend on beta types which still regress over the previous stable types. See also #39 (comment) |
@swsnr I think it is better, to fix this issue (gjsify/ts-for-gir#171) and don't use older types, since they lack some other improvements. 🤷🏼♂️ |
@JumpLink Now that Gnome 47 is out, how do we proceed here? |
I just checked out https://gjs.guide/extensions/upgrading/gnome-shell-47.html Gnome changed the method signature of --- a/prefs.d.ts
+++ b/prefs.d.ts
@@ -11,10 +11,10 @@ export class ExtensionPreferences extends ExtensionBase {
* Get the single widget that implements
* the extension's preferences.
*
- * @returns {Gtk.Widget}
+ * @returns {Gtk.Widget|Promise<Gtk.Widget>}
* @throws {GObject.NotImplementedError}
*/
- getPreferencesWidget(): Gtk.Widget;
+ getPreferencesWidget(): Gtk.Widget | Promise<Gtk.Widget>;
/**
* Fill the preferences window with preferences.
@@ -24,7 +24,7 @@ export class ExtensionPreferences extends ExtensionBase {
*
* @param {Adw.PreferencesWindow} window - the preferences window
*/
- fillPreferencesWindow(window: Adw.PreferencesWindow): void;
+ fillPreferencesWindow(window: Adw.PreferencesWindow): Promise<void>;
}
|
@schnz Be the change you want to see 😉 We'd appreciate if you could take this branch and finish it (mostly fixing the merge conflicts), and add the above change. I'll review it, and I think I'd also be able to publish a preliminary release for 47 to Npm. 🙂 |
Sure - I can add the changes (in addition to everything else that's mentioned in the referenced CHANGELOG) But I am at work - so I won't be able to do that until tonight or Saturday morning. |
@schnz That'd be very much appreciated 🙏 Take all the time you need, that's still likely a lot faster than any of us can do this 😅 |
I am now updating all generated gir types in ts-for-gir to their latest version so that we can update them here once again |
@JumpLink 🎉 🙏 |
We need to manually update all / or at least those who changed |
I think the aforementioned change to ExtensionPrefs is the only significant change. So let's perhaps get a version with updated dependencies out first? |
@swsnr @Totto16 If you think that you won't be able to implement it so quickly, we can do it that way, otherwise you are welcome to create a PR in this branch / PR. I am currently publishing the updated types on NPM and then updating them here again, I would then be done for now and would hand over the rest to you both? |
I'll do it tomorrow, so IMO we should wait for some testing until merging into main, even if we did merge everything into main with gnome 46, but having a feature branch |
@Totto16 I don't have much time this weekend, so if you can work on these types this weekend, then do it the way that's best for you 👍 We can still discuss this later on. I've released my extensions for 47 already, based on the 46 types here, but I haven't seen any runtime issues so far, so I believe with this release the impact on the typings is really small (unlike, say 45 to 46). |
I have published the current status of this PR on NPM for testing |
@swsnr Oh sorry my mistake, you are right |
I now checked the types extensively with my extension, and they seem to work correctly in all cases. There is only one caveat: I try to support more versions of the extension in one branch, so I use a non async version of For more reference on this see both PRs from above, 😉 We should further discuss this, as I find it good to respect doc strings of the actual implementation, but also be reasonable about it. It isn't required to change, but I'd like to be backwards compatible in at least some way 😓 @schnz and @swsnr what do you think of this, a we / you already discussed this in #51 ? |
Hmm, like I said in #51
The way I see it:
However, allowing I don't have a good solution here to be honest. I could argue for and against both options in one way or another
I, for myself, have choosen option (ii) and the new release of gTile is no longer compatible with Gnome versions below 47, although it would work flawlessly on them. |
As said above, I tend for option ii as well, more so, because I'd really like to look into some kind of automation based on the jsdoc type annotations from upstream (in my dream world GNOME Shell itself would use typescript and we'd get types for free ☁️ 🫠 ) |
I also think it makes sense if we start to port the missing types in GNOME Shell as JSDoc types and build something to read them out. It's best to build something to generate the types first, then see what's not working well and then start improving that in GNOME Shell itself. But then a separate issue / PR / branch makes sense. For now we can publish the types for version 47 as before. |
Automation would be very long term goal. Annotations are still scarce in upstream, and I don't really have much time to work on this any time soon anyway 😅 So let's just continue with our current process. |
Yes I agree, maybe we can also have something like an allow list for files with JSDoc types that are tested and working, so that this runs as a by-product. Also the GNOME Shell developers themselves have often mentioned that they would prefer to maintain the types directly in GNOME Shell, so they would be co-operative. In the best case scenario, this would mean less work for everyone involved in the long term |
I like the idea of automation. I could see myself implementing a prototype in a time-boxed manner (in 1 or 2 months from now) to see if type extraction in a semi-automated way is possible. But this would be no more than a little digging into the topic and a breakdown of steps that would have to be done in order to integrate this to This would imply that we strictly follow JSDoc types from upstream, where available. This would also have the effect that we would have to discuss type widening changes, like the one proposed by @Totto16 here, directly in the gnome-shell repository with the upstream developers. In this instance, I would assume that they are open to widening the return type to avoid an otherwise totally unnecessary breaking change. |
@swsnr Good question! I think we should create an issue on the GNOME Shell GitLab instance and discussing these questions there |
Mark gnome types for 47
I think we can merge this now? |
I think so, yes 👍 |
I would agree too 👍🏼 |
I suggest that we leave this PR open until GNOME Shell 47 is released. Until then, we can publish releases with the
next
tag, I have already released the current state of this PR this way.I hope this helps to prepare your extensions for the next release and fix the types from here along the way ;)
Changes