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

Draft: Port demos to TypeScript #201

Draft
wants to merge 144 commits into
base: main
Choose a base branch
from
Draft

Conversation

vixalien
Copy link
Contributor

@vixalien vixalien commented Aug 12, 2024

This is my attempt to porting demos from javascript to workbench.

Groundwork

  • All files were renamed from .js to .ts. I will be running the TypeScript compiler (with tsc) to fix demo files as we go.
  • I added a simple README.typescript.md showing basic instructions on how I generated the types
  • I added a tsconfig.json file
  • I compiled and included all the types in workbench-types

Future tasks

Ported demos

  • About Dialog
  • Accessibility
  • Account ⚠️
  • Action Bar
  • Actions
  • Animation
  • Audio
  • Avatar ⚠️
  • Banner
  • Box
  • Boxed Lists
  • Breakpoints
  • Button Row
  • Button
  • CSS Gradients
  • Calendar
  • Camera ⚠️
  • Carousel
  • Checkboxes
  • Clamp
  • Color Dialog ⚠️
  • Color Picker ⚠️
  • Column View ⚠️
  • Context Menu
  • Custom Widget
  • Database ⚠️
  • Dialog
  • Drag and Drop ⚠️
  • Drawing Area ⚠️
  • Drop Down ⚠️
  • Drop Zone ⚠️
  • Email ⚠️
  • Emoji Chooser
  • Event Controllers
  • File Monitor ⚠️
  • Flow Box
  • Font Dialog ⚠️
  • Frame
  • Gamepad
  • Grid View
  • Grid
  • HTTP Image ⚠️
  • HTTP Request ⚠️
  • HTTP Server
  • Image
  • Label
  • Launcher ⚠️
  • Level Bars
  • Link Button
  • List Model
  • List View
  • List View with a Tree
  • List View with Sections
  • Location ⚠️
  • Map
  • Memory Monitor
  • Menu Button
  • Menu
  • Message Dialogs ⚠️
  • Navigation View
  • Network Monitor
  • Notification
  • Open File ⚠️
  • Overlay Split View
  • Overlay
  • Picture
  • Popovers
  • Power Profile Monitor
  • Preferences Dialog
  • Progress Bar
  • Radio Buttons
  • Revealer
  • SVG
  • Save File ⚠️
  • Scale
  • Screencast ⚠️
  • Screenshot
  • Scrolled Window
  • Search
  • Select Folder ⚠️
  • Separator
  • Session Monitor and Inhibit ⚠️
  • Snapshot
  • Source View
  • Spell Checker
  • Spin Button
  • Spinner
  • Stack
  • Styling with CSS
  • Switch
  • Tab View
  • Text Colors
  • Text Fields
  • Text View
  • Toasts
  • Toggle Button
  • Tooltip
  • Video
  • View Switcher
  • Wallpaper ⚠️
  • Web View
  • WebSocket Client
  • Welcome

@vixalien
Copy link
Contributor Author

vixalien commented Aug 20, 2024

To work with this version of the demos while hacking on Workbench, you can bind mount it. For example,

sudo mount --bind /path/to/cloned/demos /path/to/Worbench

@vixalien
Copy link
Contributor Author

vixalien commented Sep 3, 2024

Currently all the demos are ported 🥳. However, some demos have // @ts-expect-error directives where the types generated have some issue or another. I have tried to create relevant issues upstream and link to them, but a longer term strategy may be need.

Now it's time for rebase, and working on other steps.

@vixalien vixalien force-pushed the wip/vixalien/typescript branch from a00b394 to c341532 Compare September 3, 2024 17:25
@UrtsiSantsi
Copy link
Contributor

@vixalien Why do you use get_object<Type>("name") in some places and get_object("name") as Type in others? Shouldn't we use get_object<Type>("name") everywhere?

@vixalien
Copy link
Contributor Author

Yeah we should use that everywhere. I used the former method when I was getting started on this, since I didn't know the latter was possible.

@UrtsiSantsi
Copy link
Contributor

I'll add a commit tomorrow, if you don't beat me to it.

@vixalien vixalien force-pushed the wip/vixalien/typescript branch from c341532 to e41f55e Compare October 31, 2024 11:37
@vixalien
Copy link
Contributor Author

@sonnyp I have been brought to the attention of ts-blank-space which looks like it might be useful for getting reliable stack traces.

@vixalien vixalien force-pushed the wip/vixalien/typescript branch from e41f55e to aeea1bf Compare October 31, 2024 11:46
@vixalien
Copy link
Contributor Author

I'm currently working towards enabling strict mode in TS. This will catch more errors, but also makes the code look a bit more complex, because there are more null assertions.

@JumpLink you might be interested to look at all the ts-expect-errors we have in the Changes tab so as to help improve ts-for-gir.

@UrtsiSantsi
Copy link
Contributor

@vixalien I fixed all the places I could find, that were still not using the generic get_object. But I didn't test it 😅

@vixalien
Copy link
Contributor Author

vixalien commented Nov 1, 2024

I had already pushed the changes and you force pushed the branch 🙈🙊

@UrtsiSantsi
Copy link
Contributor

@vixalien I didn't do a force push. According to GitHub I added the commits - "UrtsiSantsi added 12 commits", "UrtsiSantsi added 28 commits", while when you force pushed earlier it is "vixalien force-pushed the wip/vixalien/typescript branch from e41f55e to aeea1bf".

My changes are on top of your changes - you fixed the case where the pattern get_object("name") as Type; was used, but missed the cases where get_object("name); was used without any type and that is what I fixed. Can you check again?

@vixalien
Copy link
Contributor Author

vixalien commented Nov 1, 2024

but missed the cases where get_object("name); was used without any type and that is what I fixed.

Oh, I see. I actually left out those cases, but I was just being lazy 😆. Thanks!

I will need to rebase them so that we have a singular commit for each file. i.e. "port: Camera", "port: Welcome" etc... but don't worry about it.

I've also gone and enabled strict mode, which now causes a few more errors to pop in. You can run tsc in the root of the repo to see the issues.

@sonnyp
Copy link
Contributor

sonnyp commented Nov 1, 2024

@vixalien don't worry about commit history, I'll squash everything when merging

Individual commits won't be helpful.

@sonnyp
Copy link
Contributor

sonnyp commented Nov 1, 2024

Don't include all the typescript definitions here. They should probably live in another repo. They are only included here so that I can easily check the validity of the demos I have ported so far.

They should be in Workbench since that's the runtime.

Add a simple script that will generate the types for us. Probably similar to https://github.com/vixalien/gi-typescript-definitions/blob/main/generate_types_in_flatpak.sh

Ditto

Ensure CI is run for .ts files

👍

@sonnyp I have been brought to the attention of ts-blank-space which looks like it might be useful for getting reliable stack traces.

This looks interesting for “compiling” ts files before running.

But we still need a solution to compile ts files to js for humans.

@sonnyp
Copy link
Contributor

sonnyp commented Nov 1, 2024

But we still need a solution to compile ts files to js for humans.

Actually we could also use ts-blank-space and simplify process the result with prettier.

@UrtsiSantsi
Copy link
Contributor

I think we should leave java script as a language option:

  • It is shorter and simpler than type script, which is good for a demos
  • Supporting both will require little effort

@vixalien
Copy link
Contributor Author

vixalien commented Nov 2, 2024

They should be in Workbench since that's the runtime.

Add a simple script that will generate the types for us. Probably similar to vixalien/gi-typescript-definitions@main/generate_types_in_flatpak.sh

Ditto

Sure. I'll work on it next.

@sonnyp
Copy link
Contributor

sonnyp commented Nov 12, 2024

@UrtsiSantsi that's the idea 👍

@vixalien
Copy link
Contributor Author

With GJS now supporting sourcemaps, there is no need for ts-blank-space. I will just go through with finishing the port of the demos, then move the typescript file generation script to Workbench.

@JumpLink
Copy link

JumpLink commented Mar 6, 2025

Can you still use any help here?

@vixalien
Copy link
Contributor Author

vixalien commented Mar 8, 2025

Can you still use any help here?

Not much, really. I'm going to rebase this PR and add types for the other demos.

Since you're the types guy, you can check the demos I marked with a warning emoji (⚠️). These files have an issue which required me to use /// @ts-expect-error, and I would like to eliminate all of these type mismatches if possible.

vixalien added 4 commits March 8, 2025 17:28
Done using `find src/ -name "*.js" -exec sh -c 'mv "$0" "${0%.js}.ts"' {} \;`
included in-tree for now. might need to look for long-lasting solution
@vixalien vixalien force-pushed the wip/vixalien/typescript branch from 4555635 to bd68229 Compare March 8, 2025 15:29
@vixalien
Copy link
Contributor Author

vixalien commented Mar 8, 2025

Yeah, I think what's next for this is to figure out how to use these types in Workbench:

@sonnyp
Copy link
Contributor

sonnyp commented Mar 8, 2025

Generate the typescript in Workbench, at build time??

Yes 👍

Ensure CI works for typescript files

I can take care of that if you like

Build the typescript files in Workbench at build time to JavaScript

Did we find a solution to compile to reasonable JavaScript? As in keep variable names and so on
For formatting we can simply pass the files through biome or prettier (I can take care of that)

@vixalien
Copy link
Contributor Author

vixalien commented Mar 8, 2025

Did we find a solution to compile to reasonable JavaScript? As in keep variable names and so on

Not sure. If I remember, you have suggested we use babel in the past. We can also use other build tools such as esbuild, then prettify the output. (esbuild eats empty lines, though)

@sonnyp
Copy link
Contributor

sonnyp commented Mar 10, 2025

https://babeljs.io/docs/babel-plugin-transform-typescript should do the trick 👍

@vixalien
Copy link
Contributor Author

Okay, I would say now the ball is now in your hands

sonnyp added a commit that referenced this pull request Mar 11, 2025
See also #201

---------

Co-authored-by: Angelo Verlain <[email protected]>
@sonnyp
Copy link
Contributor

sonnyp commented Mar 11, 2025

I have partially merged this e4b515e

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

Successfully merging this pull request may close these issues.

4 participants