UCL-VR / ubiq

Other
98 stars 34 forks source link

Upgrade to typescript #37

Closed TheBv closed 9 months ago

TheBv commented 11 months ago

Recently I've been working on adding some custom functionality to the ubiq server for a project and I felt like having typescript support for ubiq would be really nice to more easily develop features and catch issues.

This PR basically does that. I've converted everything besides the modules to typescript and also made the switch to ESM. I guess mainly I want to see if there's also interest from the ubiq team to use typescript.

While working on this I've also noted a couple of issues/mistakes that I've either fixed directly or that I've annotated with a FIXME: comment. I think most of it is related to old functions/code that were removed at some point but weren't completely removed. A statement on them would be much appreciated.

Remaining To-do's

sebjf commented 11 months ago

Hi @TheBv, thank you for this substantial PR!

We've discussed and agree that using typescript for the server is preferable.

The main thing to make sure on our side is that there is no additional friction for new developers. (E.g. investigating nodejs loaders, or ts-node, or similar to avoid an additional build step, making sure the packaging via rollup for the browser library works the same way, etc.)

We are about to embark on a project to improve the experience of managing the server; supporting more visibility into what it's doing, supporting automated monitoring, and containerization options. I will do the typescript conversion as part of this.

I am going start by looking into how to set the project up such that our existing processes are as seamless as they are now, then convert the server using your PR as a starting point, and will fix up the legacy stuff as I go.

TheBv commented 11 months ago

Hi @sebjf!

That's great to hear! We ourselves were planning to implement a REST API to view the current status of the server. E.g to view currently open rooms, maybe even be able to manage them (close rooms, kick players etc.). This isn't our highest priority project right now since we're focusing on other things but if at any point you progress on updating the server I'd be happy to help out if needed!

sebjf commented 9 months ago

Hi @TheBv,

I have just made a PR for the 'finished' TypeScript Upgrade here: https://github.com/UCL-VR/ubiq/pull/39, as I couldn't rebase (the source of) this one. The branch is based off yours though, so it contains your changes and you should still show up as a contributor!

Thanks again for starting this off, and apologies it's taken this long to get to a stable state to merge. The changes as you know are quite significant and between the external samples, Rollup, Jest and the new linter there was a lot of back and forth to make sure all the tooling worked nicely together! Even though there is always more to do (as detailed in that PR), I think its now stable enough to move ahead with.

If you have further comments, esp. on e.g. the status module, please let us know! Hopefully even though it's been a month or two your ideas for the REST API and ours haven't diverged too much, if you still would like to work on this together.

I am going to close this PR now the new one is created but all your comments about the changes would be welcomed on the new one.